-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(related_issues): Support passing an event_id to the endpoint #70878
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
This enables showing trace connected issues from the issue details page for the event we're currently viewing.
"event_id": request.query_params.get("event_id"), | ||
"project_id": request.query_params.get("project_id"), | ||
} | ||
data, meta = RELATED_ISSUES_ALGORITHMS[related_type](group, extra_args) |
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.
One of the two algorithms will now use the extra two query parameters.
data, meta = RELATED_ISSUES_ALGORITHMS[related_type](group) | ||
extra_args = { | ||
"event_id": request.query_params.get("event_id"), | ||
"project_id": request.query_params.get("project_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.
project_id
is a required parameter for querying an event from Snuba.
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.
if it's required, shouldn't this validate that it is not none?
meta: dict[str, str] = {} | ||
event_id = extra_args.get("event_id") | ||
project_id = extra_args.get("project_id") | ||
if event_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.
The new approach we want to support.
assert event.group_id == group.id | ||
else: | ||
# If we drop trace connected issues from similar issues we can remove this | ||
event = group.get_recommended_event_for_environments() |
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 old approach.
def load_errors( | ||
self, | ||
project: Project, | ||
span_id: str | None = None, |
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.
This is an unrelated change that makes calling this code less weird.
@@ -47,20 +45,43 @@ def test_same_root_related_issues(self) -> None: | |||
assert response.json() == {"type": "same_root_cause", "data": [5], "meta": {}} | |||
|
|||
def test_trace_connected_errors(self) -> None: | |||
error_event, _, another_proj_event = self.load_errors(self.project, uuid4().hex[:16]) | |||
error_event, _, another_proj_event = self.load_errors(self.project) |
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.
No need to pass the span_id
anymore.
recommended_event = group.get_recommended_event_for_environments() # type: ignore[union-attr] | ||
assert recommended_event is not None # It helps with typing | ||
|
||
assert error_event.group_id != another_proj_event.group_id | ||
assert error_event.project.id != another_proj_event.project.id | ||
assert error_event.trace_id == another_proj_event.trace_id | ||
|
||
# This sets the group_id to the one we want to query about | ||
self.group_id = error_event.group_id # type: ignore[assignment] |
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.
Moving this line closer to the call to the API since it is related.
qs_params={ | ||
"type": "trace_connected", | ||
"event_id": another_proj_event.event_id, | ||
"project_id": another_proj_event.project_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.
We test in here that we pass event_id
and project_id
unlike the previous test.
assert project_id is not None | ||
event = eventstore.backend.get_event_by_id(project_id, event_id, group_id=group.id) | ||
# If we are requesting an specific event, we want to be notified with an error | ||
assert event is not None | ||
# This ensures that the event is actually part of the group and we are notified | ||
assert event.group_id == group.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.
AssertionError produced here would bubble up as generic 500 Internal Server Error
, but it should be caught in the front end and return 400 Bad Request
except AssertionError: | ||
return Response({}, 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.
ideally it would say which field, but this is fine.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
This enables showing trace connected issues from the issue details page for the event we're currently viewing.