-
-
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(related_issues): Include event and trace IDs for trace-connected errors #69693
Conversation
…ed errors This information can be used by the UI to show what trace or event were used to determine the trace connected errors.
for key, func in RELATED_ISSUES_ALGORITHMS.items(): | ||
related_issues.append({"type": key, "data": func(group)}) | ||
data, meta = func(group) |
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 function now returns a tuple with some meta data.
"data": [another_proj_event.group_id], | ||
"meta": { | ||
"event_id": error_event.event_id, | ||
"trace_id": error_event.trace_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.
These two pieces of information can now be used in the UI.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #69693 +/- ##
===========================================
+ Coverage 51.80% 79.82% +28.01%
===========================================
Files 6453 6478 +25
Lines 286715 287906 +1191
Branches 49438 49624 +186
===========================================
+ Hits 148540 229814 +81274
+ Misses 137779 57700 -80079
+ Partials 396 392 -4
|
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.
Looks good! Have some typing suggestions but they aren't blocking, code wise it's great and I appreciate adding the missing doc string!
@@ -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]]]: |
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.
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!
@@ -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]]: |
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 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.
I will follow up on the typing suggestions in the next PR! |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
For trace-connected errors, we look for the recommended event for a group and we look for a trace ID. Based on that trace ID, we determine what issues happened during the related trace.
Exposing this data in the API will help the customer visit the trace used to relate the issues.