-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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(grouping): Add metrics for issue merging and unmerging #52919
Conversation
4bc4ce9
to
fae89c2
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #52919 +/- ##
=======================================
Coverage 79.37% 79.37%
=======================================
Files 4934 4934
Lines 207215 207223 +8
Branches 35403 35403
=======================================
+ Hits 164481 164493 +12
+ Misses 37704 37701 -3
+ Partials 5030 5029 -1
|
fae89c2
to
4c1d292
Compare
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.
Always such an excellent job at testing!
@@ -61,6 +62,13 @@ def delete(self, request: Request, group) -> Response: | |||
if not hash_list: | |||
return Response() | |||
|
|||
metrics.incr( | |||
"grouping.unmerge_issues", |
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'm really interested on knowing how many people actually unmerge.
This adds DataDog metrics for instances of issues being merged and unmerged. Both metrics include the issues' platform, and the merge metric additionally includes referer (since you can merge from either the issue stream or the Similar Issues tab). A few missing merge-related tests were also added.
Note: I was originally going to include the number of issues being merged as extra data for the merging metric, but it occurred to me that people are more likely to merge in new issues as they appear rather than wait and merge all the issues at once, so I removed it. We can revisit this later if necessary.
Ref: #52920
Ref: #52922