-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: add extra sentry metrics tagged with repoid #528
Conversation
Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io>
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #528 +/- ##
========================================
Coverage 97.47% 97.48%
========================================
Files 418 418
Lines 34900 35009 +109
========================================
+ Hits 34020 34128 +108
- Misses 880 881 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #528 +/- ##
========================================
Coverage 97.47% 97.48%
========================================
Files 418 418
Lines 34900 35009 +109
========================================
+ Hits 34020 34128 +108
- Misses 880 881 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #528 +/- ##
========================================
Coverage 97.47% 97.48%
========================================
Files 418 418
Lines 34900 35009 +109
========================================
+ Hits 34020 34128 +108
- Misses 880 881 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Changes have been made to critical files, which contain lines commonly executed in production. Learn more ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
==========================================
+ Coverage 97.50% 97.60% +0.09%
==========================================
Files 449 449
Lines 35623 37173 +1550
==========================================
+ Hits 34733 36281 +1548
- Misses 890 892 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 10 files with indirect coverage changes This change has been scanned for critical changes. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question on removal criteria for the metrics and how long we're expecting to be collecting them.
@@ -140,6 +140,17 @@ def process_impl_within_lock( | |||
checkpoints = checkpoints_from_kwargs(TestResultsFlow, kwargs) | |||
|
|||
checkpoints.log(TestResultsFlow.TEST_RESULTS_FINISHER_BEGIN) | |||
|
|||
# TODO: remove this later, we can do this now because there aren't many users using this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have more details about when we should remove this? Should we have a ticket to remind ourselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it depends on the number of unique repos that end up triggering the metrics, the reason we don't want to keep emitting these metrics with the repo tag for too long is because as we get more and more unique repos using this feature the higher the cardinality of the metric will be. At some point it seems the tags will be automatically dropped if their cardinality is too high so this metric will be obsoleted anyways, so we can drop it then.
No description provided.