-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(sentry apps): Add SLO for updating servicehooks #94489
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #94489 +/- ##
==========================================
+ Coverage 87.93% 87.97% +0.03%
==========================================
Files 10370 10394 +24
Lines 599908 602459 +2551
Branches 23350 23350
==========================================
+ Hits 527524 530005 +2481
- Misses 71909 71979 +70
Partials 475 475 |
{ | ||
"installations": installations.values_list("id", flat=True), | ||
"sentry_app_id": sentry_app_id, | ||
"events": events, |
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.
what would the cardinality of events
be?
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.
the cardinality would be ~15 - corresponding to each of the webhook types. Though does cardinality matter in extras
?
installations = SentryAppInstallation.objects.filter(sentry_app_id=sentry_app_id) | ||
lifecycle.add_extras( | ||
{ | ||
"installations": installations.values_list("id", flat=True), |
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.
can installations theoretically be extremely large for apps like linear
? - if so it make make logs expensive and maybe something we can lookup based on id?
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.
ok yeah after looking it up its like 16000 installs 💫 . The failure case I'm trying to target here is when an update fails for a specific installation.. hm I guess I can move the lifecycle to start within the while loop. This is a pretty uncommon task so it shouldn't accrue too many metrics
We've had some problems with servicehooks not updating previously so would like an SLO on them