-
-
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
fix(ecosystem): Track metrics for issue detail ticket creation #82436
fix(ecosystem): Track metrics for issue detail ticket creation #82436
Conversation
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.
lgtm 👀
return Response(exc.field_errors, status=400) | ||
except IntegrationError as e: | ||
lifecycle.record_failure(e) | ||
return Response({"non_field_errors": [str(e)]}, status=400) |
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.
Q: What are examples of IntegrationErrors
, like is that a failure on our side ?
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.
IntegrationErrors are any errors from a provider that our integrations themselves can't classify as a "field" error.
We have some internal handling in the base IntegrationInstallation
class that checks any ApiInvalidRequestError
responses our integration methods raise:
sentry/src/sentry/integrations/base.py
Line 484 in 6209d6f
def raise_error(self, exc: Exception, identity: Identity | None = None) -> NoReturn: |
Which call into the implementing integration class's error_fields_from_json
method:
def error_fields_from_json(self, data): |
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 #82436 +/- ##
==========================================
+ Coverage 79.44% 80.44% +1.00%
==========================================
Files 7299 7307 +8
Lines 321843 322300 +457
Branches 21011 21011
==========================================
+ Hits 255678 259268 +3590
+ Misses 65760 62627 -3133
Partials 405 405 |
) | ||
|
||
try: | ||
data = installation.create_issue(request.data) |
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.
might be worth tracking with a different slug b/c i have a feeling we have different levels of success based on where the ticket is created from.
also wondering if we should have a SLO around the endpoint that allows the user to search for a ticket in the 3p in the issue details page.
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.
Yeah I think you're right 🤔 I'll add that now. As for search SLOs, let's discuss this during office hours today.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Adds metric gathering and logging to ticket creation on the issue details page. These may need to be recategorized in the future as a different form of issue creation, but for now I think it's fine to track this along with our issue alert rule metrics.