Add usage metrics for counting entity updates #889
Conversation
lib/data/analytics.js
Outdated
| "recent": 0, | ||
| "total": 0 | ||
| }, | ||
| "num_updated_entities": { |
There was a problem hiding this comment.
I think this name has the potential to be slightly confusing. We're measuring the number of entity updates, where each entity can be counted multiple times (once for each update). However, the name of the metric seems to imply that we're measuring a subset of all entities. Put another way, it's possible for num_updated_entities > num_entities.
@sadiqkhoja, I'll let you decide whether to use the existing name or a different name like num_entity_updates. The name isn't shown in Frontend (text is shown instead), so it's just a question of whether we could end up getting confused internally.
There was a problem hiding this comment.
Do we want to count "Number of Entities Updated" or "Number of Updates across all Entities"?
There was a problem hiding this comment.
I'm not sure actually. getodk/central#410 suggests the latter. "Number of Entities Updated" would never exceed "Number of Entities", but I think it'd be interesting if we saw a very high "Number of Updates across all Entities". It'd be nice to have some sense of how often updates happen.
matthew-white
left a comment
matthew-white
left a comment
There was a problem hiding this comment.
I think @sadiqkhoja did the primary review. I took a quick look at the new query. I also confirmed that num_updated_entities has been renamed to num_entity_updates.
lib/model/query/analytics.js
Outdated
| ) AS errors ON ds.id = errors."datasetId" | ||
| LEFT JOIN ( | ||
| SELECT | ||
| COUNT (a.details -> 'entityId'::TEXT) total, |
There was a problem hiding this comment.
Could this just be COUNT(1)?
|
@sadiqkhoja, I see that you updated the XLSForm as well for the metric rename. Thanks for doing that! I've uploaded it to data.getodk.cloud. |
Backend portion of getodk/central#410
Needs the frontend translation change and the central config version change and the new version of the form to be uploaded.
odk-analytics.xlsx
What has been done to verify that this works as intended?
Integration Test +
Tried sending local analytics to test server.
Why is this the best possible solution? Were any other approaches considered?
NA
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
No
Does this change require updates to the API documentation? If so, please update docs/api.md as part of this PR.
No
Before submitting this PR, please make sure you have:
make test-fulland confirmed all checks still pass OR confirm CircleCI build passes