Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SubjectSet completeness metrics #3649

Merged
merged 15 commits into from
Aug 4, 2021
Merged

Conversation

camallen
Copy link
Contributor

@camallen camallen commented Jul 30, 2021

closes #3450

TODO

  • Add the flipper feature flag subject_set_completeness_from_read_replica to read the counts from the replica DB

This PR adds the ability to calculate the completeness of a subject set in the context of a workflow and record these completeness metrics on the SubjectSet resource in a json blob that is keyed by the workflow id.

As a subject set can belong to many workflows in a project we need to record the completeness metrics for the SubjectSet in the context of these workflows. Specifically we need to calculate the ratio of the number of retired subjects in a set divided by the the number of subjects in a set, formula completeness = total_retired_subjects_for_a_workflow_in_a_set / total_subjects_in_a_set.

Finally we clamp the completeness metric to a sensible range, 0.0 (0%) to 1.0 (100%) to ensure we display sensible numbers and avoid issues with incorrect numerators and denominators in the calculation (described in #2155)

These completeness metrics stores this per workflow metric a subject_set.completeness jsonb column in the database and uses the atomic jsonb update operators (available since our upgrade to pg v11) to avoid clobbering data already in the jsonb column and/or by concurrent updates.

Review checklist

  • First, the most important one: is this PR small enough that you can actually review it? Feel free to just reject a branch if the changes are hard to review due to the length of the diff.
  • If there are any migrations, will they the previous version of the app work correctly after they've been run (e.g. the don't remove columns still known about by ActiveRecord).
  • If anything changed with regards to the public API, are those changes also documented in the apiary.apib file?
  • Are all the changes covered by tests? Think about any possible edge cases that might be left untested.

@camallen camallen requested a review from yuenmichelle1 July 30, 2021 10:46
Copy link
Collaborator

@yuenmichelle1 yuenmichelle1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems fine. I had a couple minor questions about syntax, nothing deal breaking.

One thing: how does this work with un-retirement? Do we need to call this SubjectSetCompletenessWorker in this case as well?

@camallen
Copy link
Contributor Author

camallen commented Aug 4, 2021

Everything seems fine. I had a couple minor questions about syntax, nothing deal breaking.

One thing: how does this work with un-retirement? Do we need to call this SubjectSetCompletenessWorker in this case as well?

Good catch about unretirement events - i'll update the PR to handle that event :)

@camallen camallen requested a review from yuenmichelle1 August 4, 2021 15:21
Copy link
Collaborator

@yuenmichelle1 yuenmichelle1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 👍

@github-actions github-actions bot added the approved Approved pull request label Aug 4, 2021
@camallen camallen merged commit 85af7c8 into master Aug 4, 2021
@camallen camallen deleted the subject-set-completeness branch August 4, 2021 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add completeness counts to subject sets
2 participants