Add management command for updating Data Dog metric #35473

Charl1996 · 2024-12-04T06:38:53Z

Related to this PR.

Technical Summary

This PR adds a management command for updating a counter metric on Data Dog. The idea is that this command will only run once. As such, it would probably be run as part of a limited release (I only thought of running it as part of a limited release after I've completed most of this PR).

What does the command do?

The command counts for how many applications the app dependencies feature was enabled and disabled again at some point.

The command will iterate through all application builds on all domains and check if a certain feature was disabled at any point after it was enabled for a particular application. If it was, a datadog counter will be increased by 1 and the particular application builds iteration will move on to the next one.

Knowing this information will give us a sense of feature dropout.

How is the command implemented?

The management command is built on top of the AppMigrationCommandBase. Well, technically only certain parts of the AppMigrationCommandBase, which was pulled out into a super class (see this comment; maybe because I'm planning to run it as a limited release this wasn't strictly necessary, I could simply have copied and modified the class). The super class manages the processed domains and the progress of the command by persisting it to the disk (and cleans up the temp files afterwards).

Please note: this command will probably put some load on the system, so I'll be testing it first on staging to see how it performs before moving forward.

Feature Flag

No specific feature flag

Safety Assurance

Safety story

To test on staging. Tested locally

Automated test coverage

No automated testing

QA Plan

No QA required

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

Charl1996 · 2024-12-04T07:57:02Z

corehq/apps/app_manager/_design/views/saved_app/map.js

@@ -24,6 +24,7 @@ function(doc){
            vellum_case_management: !!doc.vellum_case_management,
            target_commcare_flavor: doc.target_commcare_flavor,
            family_id: doc.family_id,
+            profile: doc.profile,


If this is changed for the limited release, I presume this will actually make the change to the couch view? I need to quickly find out what will happen when the release expires.

kaapstorm · 2024-12-05T12:18:18Z

I haven't gone through this in detail, but I think there is a way to speed this up significantly, considering that we are looking for the dropoff count for a feature flag:

Instead of iterating all domains, we can just iterate the revisions of that one toggle. A toggle is a Couch document, and CouchDB stores the revisions of a document. Fetch all the revisions of the document, loop through the revisions, and get the domains from those.

You can find out how to get revisions by looking at get_deleted_doc() here. Maybe something like ...

def iter_revisions(toggle):
    db = Toggle.get_db()
    result = db.get(toggle._id, revs=True)
    for rev_id in result['_revisions']['ids']:
        res = db.get(toggle._id, rev=rev_id)
        yield Toggle.wrap(res)

Charl1996 · 2024-12-05T13:14:41Z

@kaapstorm

I haven't gone through this in detail, but I think there is a way to speed this up significantly, considering that we are looking for the dropoff count for a feature flag:

The FF has been deprecated 2 months back and the feature is now generally available. I see two options here:

To get the most accurate number we need to iterate over all domains (actually, non-test domains <<-- need to add this)
Assume the uptake wasn't significant in the last 2 months and just iterate over the domains that had the FF enabled.

Any thoughts?

Charl1996 · 2024-12-06T07:26:33Z

The FF has been deprecated 2 months back and the feature is now generally available. I see two options here:

Confirmed that it's OK to only consider the domains that had the feature flag enabled. Will update

Charl1996 · 2024-12-10T07:07:57Z

If folks don't have any concerns I'm happy to start testing this on staging.

mkangia · 2024-12-10T08:31:39Z

corehq/apps/app_manager/management/commands/top_up_app_dependencies_dropout.py

+
+    def run(self, domains, domain_list_position):
+        for domain in domains:
+            app_ids = self.get_app_ids(domain)


this would get delete app ids as well? do we need them? or those are ignored later based on properties set in the class like include_deleted_apps?

Confirming that this here will just get the app but not the builds?

or those are ignored later based on properties set in the class like include_deleted_apps

Correct.

Confirming that this here will just get the app but not the builds?

Yes, the builds are also being excluded with the include_builds = False flag as implemented in the get_all_app_ids method.

Thanks!

I look forward to your test results from staging.

mkangia

Good to test this on staging.

We won't be merging this PR right?

gherceg

Pair reviewed with @jingcheng16 and we just have a question about the use of datadog for this.

gherceg · 2024-12-10T18:56:15Z

corehq/apps/app_manager/management/commands/top_up_app_dependencies_dropout.py

+
+        for build_doc in app_builds[1:]:
+            if self._has_app_dependencies(build_doc['value']):
+                metrics_counter('commcare.app_build.dependencies_removed')


Given this is only going to be run once, and the output is basically 2 numbers (dependencies_added and dependencies_removed), is it easier to just output this in the command? It seems unnecessary to send this one time metric to datadog.

Hey @gherceg
this is meant to bump up the counter to current usage,
the feature uptake and dropout will continue to be tracked further with this other PR.

fyi @jingcheng16

That make sense! Thank you for explanation!

Charl1996 · 2024-12-11T10:49:49Z

We won't be merging this PR right?

No, I'd rather do it through limited release, which I think is now much safer now.

Charl1996 · 2024-12-11T12:36:45Z

Update:
I've checked out of curiosity how many domains this command will run over, and it turns out none now that we only iterate previously feature flagged domains that are not test domains.

I've checked with the necessary people and have concluded that we can close this PR as this step have effectively become redundant.

mkangia · 2024-12-13T08:46:09Z

Nice, good move @Charl1996

Charl1996 requested review from kaapstorm, mkangia and ajeety4 December 4, 2024 06:38

Charl1996 requested a review from orangejenny as a code owner December 4, 2024 06:38

dimagimon added the reindex/migration Reindex or migration will be required during or before deploy label Dec 4, 2024

Charl1996 mentioned this pull request Dec 4, 2024

[SC-4053] DataDog metric for historic app dependency feature usage #35445

Closed

3 tasks

Charl1996 added the product/invisible Change has no end-user visible impact label Dec 4, 2024

Charl1996 commented Dec 4, 2024

View reviewed changes

Charl1996 added the Open for review: do not merge A work in progress label Dec 5, 2024

Charl1996 added 3 commits December 5, 2024 08:10

Add very simple test for command

e10299b

Pull out domains applications updates into superclass

8a47d55

Add management command to update counter

467eaab

Charl1996 force-pushed the cs/SC-4053-datadog-metric-for-feature-historic-usage-management-cmd branch from b17ae91 to ff6f63e Compare December 5, 2024 06:14

Charl1996 added 2 commits December 5, 2024 08:15

Report current usage

9c1bb0b

Set include_docs to True

8d44508

Charl1996 force-pushed the cs/SC-4053-datadog-metric-for-feature-historic-usage-management-cmd branch from ff6f63e to 8d44508 Compare December 5, 2024 06:15

dimagimon removed the reindex/migration Reindex or migration will be required during or before deploy label Dec 5, 2024

Charl1996 mentioned this pull request Dec 5, 2024

Report app dependency addition/removal to DataDog #35455

Merged

3 tasks

Charl1996 added 2 commits December 10, 2024 08:57

Consider only previously FF domains

ae15b1f

Exclude test domains

703a8ac

mkangia reviewed Dec 10, 2024

View reviewed changes

gherceg reviewed Dec 10, 2024

View reviewed changes

Charl1996 closed this Dec 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add management command for updating Data Dog metric #35473

Add management command for updating Data Dog metric #35473

Charl1996 commented Dec 4, 2024 •

edited

Loading

Charl1996 Dec 4, 2024

kaapstorm commented Dec 5, 2024 •

edited

Loading

Charl1996 commented Dec 5, 2024

Charl1996 commented Dec 6, 2024

Charl1996 commented Dec 10, 2024

mkangia Dec 10, 2024

Charl1996 Dec 10, 2024

mkangia Dec 10, 2024

mkangia left a comment

gherceg left a comment •

edited

Loading

gherceg Dec 10, 2024

mkangia Dec 10, 2024

jingcheng16 Dec 10, 2024

Charl1996 commented Dec 11, 2024

Charl1996 commented Dec 11, 2024

mkangia commented Dec 13, 2024

Add management command for updating Data Dog metric #35473

Add management command for updating Data Dog metric #35473

Conversation

Charl1996 commented Dec 4, 2024 • edited Loading

Technical Summary

What does the command do?

How is the command implemented?

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Charl1996 Dec 4, 2024

Choose a reason for hiding this comment

kaapstorm commented Dec 5, 2024 • edited Loading

Charl1996 commented Dec 5, 2024

Charl1996 commented Dec 6, 2024

Charl1996 commented Dec 10, 2024

mkangia Dec 10, 2024

Choose a reason for hiding this comment

Charl1996 Dec 10, 2024

Choose a reason for hiding this comment

mkangia Dec 10, 2024

Choose a reason for hiding this comment

mkangia left a comment

Choose a reason for hiding this comment

gherceg left a comment • edited Loading

Choose a reason for hiding this comment

gherceg Dec 10, 2024

Choose a reason for hiding this comment

mkangia Dec 10, 2024

Choose a reason for hiding this comment

jingcheng16 Dec 10, 2024

Choose a reason for hiding this comment

Charl1996 commented Dec 11, 2024

Charl1996 commented Dec 11, 2024

mkangia commented Dec 13, 2024

Charl1996 commented Dec 4, 2024 •

edited

Loading

kaapstorm commented Dec 5, 2024 •

edited

Loading

gherceg left a comment •

edited

Loading