From a8470707227880a2e55b04ccb6dfd446f8abe9d8 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 16 Jan 2025 14:22:02 +0200 Subject: [PATCH 01/11] Add method for checking custom callouts --- corehq/apps/app_manager/models.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 988a73795467..9d1985416d1c 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4468,10 +4468,32 @@ def make_build(self, comment=None, user_id=None): assert copy._id prune_auto_generated_builds.delay(self.domain, self._id) - self.check_build_dependencies(new_build=copy) + self.analyse_app_build(new_build=copy) return copy + def analyse_app_build(self, new_build): + self.check_for_custom_callouts(new_build) + self.check_build_dependencies(new_build) + + def check_for_custom_callouts(self, new_build): + from corehq.apps.app_manager.util import app_callout_templates + + templates = next(app_callout_templates) + template_ids = set([t['id'] for t in templates]) + + def app_has_custom_intents(): + return any( + any(set(form.wrapped_xform().odk_intents) - template_ids) + for form in new_build.get_forms() + ) + + if app_has_custom_intents(): + metrics_counter( + 'commcare.app_build.custom_app_callout', + tags={'domain': new_build.domain}, + ) + def check_build_dependencies(self, new_build): """ Reports whether the app dependencies have been added or removed. From dd8145e90c2cff96f9364cb06b3c448b404a71b1 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 16 Jan 2025 14:26:11 +0200 Subject: [PATCH 02/11] Tag app id --- corehq/apps/app_manager/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 9d1985416d1c..491b7333f205 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4491,7 +4491,7 @@ def app_has_custom_intents(): if app_has_custom_intents(): metrics_counter( 'commcare.app_build.custom_app_callout', - tags={'domain': new_build.domain}, + tags={'domain': new_build.domain, 'app_id': new_build.copy_of}, ) def check_build_dependencies(self, new_build): From 9e400b125d3613c859600bcc7a840db0c228d049 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 17 Jan 2025 10:21:53 +0200 Subject: [PATCH 03/11] Add task for managing metrics for app build --- corehq/apps/app_manager/tasks.py | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/corehq/apps/app_manager/tasks.py b/corehq/apps/app_manager/tasks.py index b439f49986f3..7c9d7601923f 100644 --- a/corehq/apps/app_manager/tasks.py +++ b/corehq/apps/app_manager/tasks.py @@ -15,6 +15,8 @@ from corehq.apps.users.models import CommCareUser, CouchUser from corehq.toggles import USH_USERCASES_FOR_WEB_USERS from corehq.util.decorators import serial_task +from corehq.util.metrics import metrics_counter +from corehq.apps.app_manager.dbaccessors import get_latest_build_doc logger = get_task_logger(__name__) @@ -91,3 +93,50 @@ def update_linked_app_and_notify_task(domain, app_id, master_app_id, user_id, em def load_appcues_template_app(domain, username, app_slug): from corehq.apps.app_manager.views.apps import load_app_from_slug load_app_from_slug(domain, username, app_slug) + + +@task(serializer='pickle', queue='background_queue', ignore_result=True) +def analyse_app_build(new_build): + check_for_custom_callouts(new_build) + check_build_dependencies(new_build) + + +def check_for_custom_callouts(new_build): + from corehq.apps.app_manager.util import app_callout_templates + + templates = next(app_callout_templates) + template_ids = set([t['id'] for t in templates]) + + def app_has_custom_intents(): + return any( + any(set(form.wrapped_xform().odk_intents) - template_ids) + for form in new_build.get_forms() + ) + + if app_has_custom_intents(): + metrics_counter( + 'commcare.app_build.custom_app_callout', + tags={'domain': new_build.domain, 'app_id': new_build.copy_of}, + ) + + +def check_build_dependencies(new_build): + """ + Reports whether the app dependencies have been added or removed. + """ + + def has_dependencies(build): + return bool( + build.profile.get('features', {}).get('dependencies') + ) + + new_build_has_dependencies = has_dependencies(new_build) + + last_build = get_latest_build_doc(new_build.domain, new_build.copy_of) + last_build = new_build.__class__.wrap(last_build) if last_build else None + last_build_has_dependencies = has_dependencies(last_build) if last_build else False + + if not last_build_has_dependencies and new_build_has_dependencies: + metrics_counter('commcare.app_build.dependencies_added') + elif last_build_has_dependencies and not new_build_has_dependencies: + metrics_counter('commcare.app_build.dependencies_removed') From 16745fb1a97eec11d31023a673d3e282e9407563 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 17 Jan 2025 10:22:20 +0200 Subject: [PATCH 04/11] Invoke task after build has saved --- corehq/apps/app_manager/models.py | 46 ----------------------- corehq/apps/app_manager/views/releases.py | 2 + 2 files changed, 2 insertions(+), 46 deletions(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 491b7333f205..0f856798df15 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -181,7 +181,6 @@ from corehq.blobs.mixin import CODES, BlobMixin from corehq.const import USER_DATE_FORMAT, USER_TIME_FORMAT from corehq.util import bitly, view_utils -from corehq.util.metrics import metrics_counter from corehq.util.quickcache import quickcache from corehq.util.timer import TimingContext, time_method from corehq.util.timezones.conversions import ServerTime @@ -4468,53 +4467,8 @@ def make_build(self, comment=None, user_id=None): assert copy._id prune_auto_generated_builds.delay(self.domain, self._id) - self.analyse_app_build(new_build=copy) - return copy - def analyse_app_build(self, new_build): - self.check_for_custom_callouts(new_build) - self.check_build_dependencies(new_build) - - def check_for_custom_callouts(self, new_build): - from corehq.apps.app_manager.util import app_callout_templates - - templates = next(app_callout_templates) - template_ids = set([t['id'] for t in templates]) - - def app_has_custom_intents(): - return any( - any(set(form.wrapped_xform().odk_intents) - template_ids) - for form in new_build.get_forms() - ) - - if app_has_custom_intents(): - metrics_counter( - 'commcare.app_build.custom_app_callout', - tags={'domain': new_build.domain, 'app_id': new_build.copy_of}, - ) - - def check_build_dependencies(self, new_build): - """ - Reports whether the app dependencies have been added or removed. - """ - - def has_dependencies(build): - return bool( - build.profile.get('features', {}).get('dependencies') - ) - - new_build_has_dependencies = has_dependencies(new_build) - - last_build = get_latest_build_doc(self.domain, self.id) - last_build = self.__class__.wrap(last_build) if last_build else None - last_build_has_dependencies = has_dependencies(last_build) if last_build else False - - if not last_build_has_dependencies and new_build_has_dependencies: - metrics_counter('commcare.app_build.dependencies_added') - elif last_build_has_dependencies and not new_build_has_dependencies: - metrics_counter('commcare.app_build.dependencies_removed') - def convert_app_to_build(self, copy_of, user_id, comment=None): self.copy_of = copy_of built_on = datetime.datetime.utcnow() diff --git a/corehq/apps/app_manager/views/releases.py b/corehq/apps/app_manager/views/releases.py index fbf517406a9e..ca2afb7b6016 100644 --- a/corehq/apps/app_manager/views/releases.py +++ b/corehq/apps/app_manager/views/releases.py @@ -67,6 +67,7 @@ ) from corehq.apps.app_manager.tasks import ( create_build_files_for_all_app_profiles, + analyse_app_build, ) from corehq.apps.app_manager.util import ( get_and_assert_practice_user_in_domain, @@ -387,6 +388,7 @@ def make_app_build(app, comment, user_id): user_id=user_id, ) copy.save(increment_version=False) + analyse_app_build.delay(copy) return copy From fb0ff786e23c6c8d1d0d4ec9047ceb51743a028e Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 17 Jan 2025 10:43:27 +0200 Subject: [PATCH 05/11] Make sure to fetch the apps deterministically --- corehq/apps/app_manager/tasks.py | 17 ++++++++++++----- corehq/apps/app_manager/views/releases.py | 4 ++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/corehq/apps/app_manager/tasks.py b/corehq/apps/app_manager/tasks.py index 7c9d7601923f..573390366554 100644 --- a/corehq/apps/app_manager/tasks.py +++ b/corehq/apps/app_manager/tasks.py @@ -7,6 +7,7 @@ get_app, get_auto_generated_built_apps, get_latest_build_id, + get_build_ids, ) from corehq.apps.app_manager.exceptions import ( AppValidationError, @@ -16,7 +17,6 @@ from corehq.toggles import USH_USERCASES_FOR_WEB_USERS from corehq.util.decorators import serial_task from corehq.util.metrics import metrics_counter -from corehq.apps.app_manager.dbaccessors import get_latest_build_doc logger = get_task_logger(__name__) @@ -96,7 +96,10 @@ def load_appcues_template_app(domain, username, app_slug): @task(serializer='pickle', queue='background_queue', ignore_result=True) -def analyse_app_build(new_build): +def analyse_new_app_build(domain, app_id): + new_build_id = get_latest_build_id(domain, app_id) + new_build = get_app(domain, new_build_id) + check_for_custom_callouts(new_build) check_build_dependencies(new_build) @@ -131,10 +134,14 @@ def has_dependencies(build): ) new_build_has_dependencies = has_dependencies(new_build) + app_build_ids = get_build_ids(new_build.domain, new_build.copy_of) + + last_build_has_dependencies = False - last_build = get_latest_build_doc(new_build.domain, new_build.copy_of) - last_build = new_build.__class__.wrap(last_build) if last_build else None - last_build_has_dependencies = has_dependencies(last_build) if last_build else False + if len(app_build_ids) > 1: + previous_build_id = app_build_ids[app_build_ids.index(new_build.id) + 1] + previous_build = get_app(new_build.domain, previous_build_id) + last_build_has_dependencies = has_dependencies(previous_build) if previous_build else False if not last_build_has_dependencies and new_build_has_dependencies: metrics_counter('commcare.app_build.dependencies_added') diff --git a/corehq/apps/app_manager/views/releases.py b/corehq/apps/app_manager/views/releases.py index ca2afb7b6016..27c74a8af5a5 100644 --- a/corehq/apps/app_manager/views/releases.py +++ b/corehq/apps/app_manager/views/releases.py @@ -67,7 +67,7 @@ ) from corehq.apps.app_manager.tasks import ( create_build_files_for_all_app_profiles, - analyse_app_build, + analyse_new_app_build, ) from corehq.apps.app_manager.util import ( get_and_assert_practice_user_in_domain, @@ -388,7 +388,7 @@ def make_app_build(app, comment, user_id): user_id=user_id, ) copy.save(increment_version=False) - analyse_app_build.delay(copy) + analyse_new_app_build.delay(app.domain, app._id) return copy From 2741b7e6b50e680a6344b37c97b332b7ab8d183d Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 17 Jan 2025 10:47:44 +0200 Subject: [PATCH 06/11] Remove pickle serialization --- corehq/apps/app_manager/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/app_manager/tasks.py b/corehq/apps/app_manager/tasks.py index 573390366554..4e08980304ca 100644 --- a/corehq/apps/app_manager/tasks.py +++ b/corehq/apps/app_manager/tasks.py @@ -95,7 +95,7 @@ def load_appcues_template_app(domain, username, app_slug): load_app_from_slug(domain, username, app_slug) -@task(serializer='pickle', queue='background_queue', ignore_result=True) +@task(queue='background_queue', ignore_result=True) def analyse_new_app_build(domain, app_id): new_build_id = get_latest_build_id(domain, app_id) new_build = get_app(domain, new_build_id) From 265a39db1207d74e02bdb9361f8fdf1270ab3ab7 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 17 Jan 2025 11:14:10 +0200 Subject: [PATCH 07/11] Fix test --- corehq/apps/app_manager/tests/test_apps.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/corehq/apps/app_manager/tests/test_apps.py b/corehq/apps/app_manager/tests/test_apps.py index 35f37c8fe656..c0a8ae35ef9d 100644 --- a/corehq/apps/app_manager/tests/test_apps.py +++ b/corehq/apps/app_manager/tests/test_apps.py @@ -39,6 +39,7 @@ from corehq.apps.domain.shortcuts import create_domain from corehq.apps.linked_domain.applications import link_app from corehq.apps.userreports.tests.utils import get_sample_report_config +from corehq.apps.app_manager.views.releases import make_app_build MockRequest = namedtuple('MockRequest', ['status', 'data']) @@ -423,8 +424,8 @@ def test_dependencies_feature_added(self): factory.app.save() self.addCleanup(factory.app.delete) - with patch("corehq.apps.app_manager.models.metrics_counter") as metric_counter_mock: - factory.app.make_build() + with patch("corehq.apps.app_manager.tasks.metrics_counter") as metric_counter_mock: + make_app_build(factory.app, "comment", user_id="user_id") metric_counter_mock.assert_called_with('commcare.app_build.dependencies_added') def test_dependencies_feature_removed(self): @@ -440,8 +441,8 @@ def test_dependencies_feature_removed(self): factory.app.profile = {'features': {'dependencies': []}} factory.app.save() - with patch("corehq.apps.app_manager.models.metrics_counter") as metric_counter_mock: - factory.app.make_build() + with patch("corehq.apps.app_manager.tasks.metrics_counter") as metric_counter_mock: + make_app_build(factory.app, "comment", user_id="user_id") metric_counter_mock.assert_called_with('commcare.app_build.dependencies_removed') def test_dependencies_feature_metrics_not_triggerd(self): @@ -456,6 +457,6 @@ def test_dependencies_feature_metrics_not_triggerd(self): factory.app.save() - with patch("corehq.apps.app_manager.models.metrics_counter") as metric_counter_mock: - factory.app.make_build() + with patch("corehq.apps.app_manager.tasks.metrics_counter") as metric_counter_mock: + make_app_build(factory.app, "comment", user_id="user_id") metric_counter_mock.assert_not_called() From f274054b10d2ee02870c57525167939c9922f5d9 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Mon, 20 Jan 2025 17:49:24 +0200 Subject: [PATCH 08/11] Pass build id instead of app id --- corehq/apps/app_manager/tasks.py | 3 +-- corehq/apps/app_manager/views/releases.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/corehq/apps/app_manager/tasks.py b/corehq/apps/app_manager/tasks.py index 4e08980304ca..327e3942990f 100644 --- a/corehq/apps/app_manager/tasks.py +++ b/corehq/apps/app_manager/tasks.py @@ -96,8 +96,7 @@ def load_appcues_template_app(domain, username, app_slug): @task(queue='background_queue', ignore_result=True) -def analyse_new_app_build(domain, app_id): - new_build_id = get_latest_build_id(domain, app_id) +def analyse_new_app_build(domain, new_build_id): new_build = get_app(domain, new_build_id) check_for_custom_callouts(new_build) diff --git a/corehq/apps/app_manager/views/releases.py b/corehq/apps/app_manager/views/releases.py index 27c74a8af5a5..f9202a34d60f 100644 --- a/corehq/apps/app_manager/views/releases.py +++ b/corehq/apps/app_manager/views/releases.py @@ -388,7 +388,7 @@ def make_app_build(app, comment, user_id): user_id=user_id, ) copy.save(increment_version=False) - analyse_new_app_build.delay(app.domain, app._id) + analyse_new_app_build.delay(app.domain, copy._id) return copy From 8b8e04ccf74780941fe2693674f459884b14dab5 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Mon, 20 Jan 2025 18:01:47 +0200 Subject: [PATCH 09/11] Pull out function to calculate ids and memoize result --- corehq/apps/app_manager/tasks.py | 7 ++----- corehq/apps/app_manager/util.py | 7 +++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/corehq/apps/app_manager/tasks.py b/corehq/apps/app_manager/tasks.py index 327e3942990f..6c941b64ebe2 100644 --- a/corehq/apps/app_manager/tasks.py +++ b/corehq/apps/app_manager/tasks.py @@ -104,14 +104,11 @@ def analyse_new_app_build(domain, new_build_id): def check_for_custom_callouts(new_build): - from corehq.apps.app_manager.util import app_callout_templates - - templates = next(app_callout_templates) - template_ids = set([t['id'] for t in templates]) + from corehq.apps.app_manager.util import app_callout_templates_ids def app_has_custom_intents(): return any( - any(set(form.wrapped_xform().odk_intents) - template_ids) + any(set(form.wrapped_xform().odk_intents) - app_callout_templates_ids) for form in new_build.get_forms() ) diff --git a/corehq/apps/app_manager/util.py b/corehq/apps/app_manager/util.py index 8faec1b827e1..3c206b07c4da 100644 --- a/corehq/apps/app_manager/util.py +++ b/corehq/apps/app_manager/util.py @@ -16,6 +16,7 @@ from couchdbkit import ResourceNotFound from couchdbkit.exceptions import DocTypeError +from memoized import memoized from dimagi.utils.couch import CriticalSection from corehq import toggles @@ -540,7 +541,13 @@ def _app_callout_templates(): yield data +@memoized +def _app_callout_templates_ids(): + return {t['id'] for t in next(_app_callout_templates())} + + app_callout_templates = _app_callout_templates() +app_callout_templates_ids = _app_callout_templates_ids() def purge_report_from_mobile_ucr(report_config): From 6968738f7f50ba9f98bb1e8f9cf7f64606ec7193 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Tue, 21 Jan 2025 09:01:31 +0200 Subject: [PATCH 10/11] Replace memoized -> quickcache --- corehq/apps/app_manager/util.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/corehq/apps/app_manager/util.py b/corehq/apps/app_manager/util.py index 3c206b07c4da..8f0f5308f687 100644 --- a/corehq/apps/app_manager/util.py +++ b/corehq/apps/app_manager/util.py @@ -16,7 +16,6 @@ from couchdbkit import ResourceNotFound from couchdbkit.exceptions import DocTypeError -from memoized import memoized from dimagi.utils.couch import CriticalSection from corehq import toggles @@ -541,7 +540,7 @@ def _app_callout_templates(): yield data -@memoized +@quickcache([], timeout=60 * 60 * 24 * 7) # 1 week def _app_callout_templates_ids(): return {t['id'] for t in next(_app_callout_templates())} From 0886d4ca0ffa25f485b4ade57a9833371e86c232 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 31 Jan 2025 11:44:46 +0200 Subject: [PATCH 11/11] Invoke app_callout_templates_ids directly --- corehq/apps/app_manager/tasks.py | 3 ++- corehq/apps/app_manager/util.py | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/corehq/apps/app_manager/tasks.py b/corehq/apps/app_manager/tasks.py index 6c941b64ebe2..c38b807a96f1 100644 --- a/corehq/apps/app_manager/tasks.py +++ b/corehq/apps/app_manager/tasks.py @@ -105,10 +105,11 @@ def analyse_new_app_build(domain, new_build_id): def check_for_custom_callouts(new_build): from corehq.apps.app_manager.util import app_callout_templates_ids + template_ids = app_callout_templates_ids() def app_has_custom_intents(): return any( - any(set(form.wrapped_xform().odk_intents) - app_callout_templates_ids) + any(set(form.wrapped_xform().odk_intents) - template_ids) for form in new_build.get_forms() ) diff --git a/corehq/apps/app_manager/util.py b/corehq/apps/app_manager/util.py index 8f0f5308f687..0507a45ee069 100644 --- a/corehq/apps/app_manager/util.py +++ b/corehq/apps/app_manager/util.py @@ -541,12 +541,11 @@ def _app_callout_templates(): @quickcache([], timeout=60 * 60 * 24 * 7) # 1 week -def _app_callout_templates_ids(): +def app_callout_templates_ids(): return {t['id'] for t in next(_app_callout_templates())} app_callout_templates = _app_callout_templates() -app_callout_templates_ids = _app_callout_templates_ids() def purge_report_from_mobile_ucr(report_config):