Skip to content
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(related_issues): Include event and trace IDs for trace-connected errors #69693

Merged
merged 3 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/sentry/api/endpoints/issues/related_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,4 @@ def get(self, _: Request, group: Group) -> Response:
:pparam string group_id: the ID of the issue
"""
related_issues = find_related_issues(group)
return Response(
{
"data": [
{"type": related_set["type"], "data": related_set["data"]}
for related_set in related_issues
]
}
)
return Response({"data": [related_set for related_set in related_issues]})
7 changes: 4 additions & 3 deletions src/sentry/issues/related/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
}


def find_related_issues(group: Group) -> list[dict[str, list[int] | str]]:
related_issues: list[dict[str, list[int] | str]] = []
def find_related_issues(group: Group) -> list[dict[str, str | list[int] | dict[str, str]]]:
Copy link
Member

@leeandher leeandher Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For here it looks like we can use a typed dict:

class RelatedIssueItem(TypedDict):
  meta: something
  type: str
  data: something
  
def find_related_issues(group: Group) -> list[RelatedIssueItem]

But I'll leave the specific types to you!

related_issues: list[dict[str, str | list[int] | dict[str, str]]] = []
leeandher marked this conversation as resolved.
Show resolved Hide resolved
for key, func in RELATED_ISSUES_ALGORITHMS.items():
related_issues.append({"type": key, "data": func(group)})
data, meta = func(group)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function now returns a tuple with some meta data.

related_issues.append({"type": key, "data": data, "meta": meta})

return related_issues
4 changes: 2 additions & 2 deletions src/sentry/issues/related/same_root_cause.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from sentry.utils.query import RangeQuerySetWrapper


def same_root_cause_analysis(group: Group) -> list[int]:
def same_root_cause_analysis(group: Group) -> tuple[list[int], dict[str, str]]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally like dict returns since they have to name each return value, but either way I think we should extract this to a type alias and reuse it for both algos, maybe even typing RELATED_ISSUES_ALGORITHMS explicitly so that new entries to that dict have a blueprint.

"""Analyze and create a group set if the group was caused by the same root cause."""
# Querying the data field (which is a GzippedDictField) cannot be done via
# Django's ORM, thus, we do so via compare_groups
Expand All @@ -16,7 +16,7 @@ def same_root_cause_analysis(group: Group) -> list[int]:
limit=100,
)
same_error_type_groups = [g.id for g in project_groups if compare_groups(g, group)]
return same_error_type_groups or []
return same_error_type_groups or [], {}


def compare_groups(groupA: Group, groupB: Group) -> bool:
Expand Down
7 changes: 4 additions & 3 deletions src/sentry/issues/related/trace_connected.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
from sentry.utils.snuba import bulk_snuba_queries


def trace_connected_analysis(group: Group) -> list[int]:
def trace_connected_analysis(group: Group) -> tuple[list[int], dict[str, str]]:
"""Determine if the group has a trace connected to it and return other issues that were part of it."""
event = group.get_recommended_event_for_environments()
if not event or event.trace_id is None:
return []
return [], {}

org_id = group.project.organization_id
# XXX: Test without a list and validate the data type
Expand All @@ -40,4 +41,4 @@ def trace_connected_analysis(group: Group) -> list[int]:
if datum["issue.id"] != group.id # Exclude itself
}
)
return transformed_results
return transformed_results, {"event_id": event.event_id, "trace_id": event.trace_id}
20 changes: 16 additions & 4 deletions tests/sentry/api/endpoints/issues/test_related_issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,34 @@ def test_same_root_related_issues(self) -> None:
# https://us.sentry.io/api/0/organizations/sentry/issues-stats/?groups=4741828952&groups=4489703641&statsPeriod=24h
assert response.json() == {
"data": [
{"type": "same_root_cause", "data": [5]},
{"type": "trace_connected", "data": []},
{"type": "same_root_cause", "data": [5], "meta": {}},
{"type": "trace_connected", "data": [], "meta": {}},
],
}

def test_trace_connected_errors(self) -> None:
error_event, _, another_proj_event = self.load_errors(self.project, uuid4().hex[:16])
group = error_event.group
self.group_id = error_event.group_id # type: ignore[assignment]
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

response = self.get_success_response()
assert response.json() == {
"data": [
{"type": "same_root_cause", "data": []},
{"type": "trace_connected", "data": [another_proj_event.group_id]},
{"type": "same_root_cause", "data": [], "meta": {}},
{
"type": "trace_connected",
# This is the other issue in the trace that it is not itself
"data": [another_proj_event.group_id],
"meta": {
"event_id": recommended_event.event_id,
"trace_id": error_event.trace_id,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two pieces of information can now be used in the UI.

},
},
]
}
Loading