feat(related_issues): Trace connected errors #69237

armenzg · 2024-04-18T19:29:34Z

Given a group, we look for the recommended event and search for any other errors in the trace.

These trace-connected groups will be shown under the Related Issues tab. See the current look (more UI changes will be needed).

codecov · 2024-04-18T20:05:32Z

Codecov Report

Attention: Patch coverage is 92.10526% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 79.48%. Comparing base (85558cb) to head (16914f5).
Report is 22 commits behind head on master.

❗ Current head 16914f5 differs from pull request most recent head 9371263. Consider uploading reports for the commit 9371263 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #69237      +/-   ##
==========================================
- Coverage   79.65%   79.48%   -0.17%     
==========================================
  Files        6476     6477       +1     
  Lines      287490   287527      +37     
  Branches    49547    49554       +7     
==========================================
- Hits       228990   228549     -441     
- Misses      58132    58610     +478     
  Partials      368      368

Files	Coverage Δ
src/sentry/eventstore/models.py	`96.73% <100.00%> (+0.04%)`	⬆️
src/sentry/issues/related/__init__.py	`100.00% <100.00%> (ø)`
src/sentry/issues/related/same_root_cause.py	`100.00% <100.00%> (ø)`
src/sentry/search/events/builder/discover.py	`96.31% <100.00%> (+<0.01%)`	⬆️
src/sentry/snuba/referrer.py	`100.00% <100.00%> (ø)`
src/sentry/testutils/cases.py	`89.43% <100.00%> (+0.07%)`	⬆️
src/sentry/testutils/factories.py	`96.00% <100.00%> (+<0.01%)`	⬆️
.../sentry/api/endpoints/organization_events_trace.py	`24.68% <85.71%> (-65.91%)`	⬇️
src/sentry/issues/related/trace_connected.py	`85.71% <85.71%> (ø)`

... and 9 files with indirect coverage changes

Given an issue, if we can find an event with a trace, we may be able to find other issue in its trace. This can allow the customer to act on them together.

armenzg · 2024-04-23T19:43:17Z

src/sentry/api/endpoints/organization_events_trace.py

-            auto_fields=False,
-        ),
-    )
+    error_query = find_errors_for_trace_id(params, trace_id, limit)


I'm going to call find_errors_for_trace_id from my endpoint, thus, making it its own function.

I'd prefer you re-implement this function rather than re-use here so that I can still make changes here without potentially breaking your endpoint.

Especially because the query pattern here is very dependent on feeding the latter augment_transactions_with_spans

armenzg · 2024-04-23T19:44:54Z

src/sentry/issues/related/same_root_cause.py

@@ -11,7 +11,10 @@ def same_root_cause_analysis(group: Group) -> list[int]:
    """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
-    project_groups = RangeQuerySetWrapper(Group.objects.filter(project=group.project_id), limit=100)
+    project_groups = RangeQuerySetWrapper(
+        Group.objects.filter(project=group.project_id).exclude(id=group.id),


Unrelated change to exclude the group ID we're querying about.

This is the related test change for it:

src/sentry/search/events/builder/discover.py

src/sentry/testutils/factories.py

armenzg · 2024-04-23T19:48:39Z

tests/sentry/api/endpoints/issues/test_related_issues.py

    endpoint = "sentry-api-0-issues-related-issues"
+    FEATURES: list[str] = []


This is needed in one of the test cases. Ideally, I fix it up there.

armenzg · 2024-04-23T19:48:59Z

tests/sentry/api/endpoints/issues/test_related_issues.py


    def setUp(self) -> None:
        super().setUp()
        self.login_as(user=self.user)
        self.organization = self.create_organization(owner=self.user)
-        self.error_type = "ApiTimeoutError"
-        self.error_value = "Timed out attempting to reach host: api.github.com"


Unrelated change. I could move it into its own PR.

vartec · 2024-04-23T20:08:40Z

src/sentry/issues/related/trace_connected.py

+    query = QueryBuilder(
+        Dataset.Events,
+        {"start": start, "end": end, "organization_id": org_id, "project_id": project_ids},
+        query=f"trace:{event.trace_id}",
+        selected_columns=["id", "issue.id"],
+        # Don't add timestamp to this orderby as snuba will have to split the time range up and make multiple queries
+        orderby=["id"],
+        limit=100,
+        config=QueryBuilderConfig(auto_fields=False),
+    )


this seems like potentially rather heavy query, maybe add some read-through cache?

wmak · 2024-04-23T22:07:00Z

src/sentry/issues/related/trace_connected.py

+    org_id = group.project.organization_id
+    # XXX: Test without a list and validate the data type
+    project_ids = list(Project.objects.filter(organization_id=org_id).values_list("id", flat=True))
+    start, end = default_start_end_dates()  # Today to 90 days back


+1 to Bartek's comment here, this is a giant query if its spanning multiple projects and ninety days, I'm not familiar with what you're building, but do you really need to scan 90d of data? Traces rarely span more than a few minutes, and even more rarely span multiple days.

That is to say, you have an event do you know that events timestamp? can you use that?

I will follow up.
We can just use the timestamp of the event as the start date.

Agree that a day in either direction seems like it would cover 99% of useful cases.

You'd want to see the trace connected issues for downstream issues as well (e.g. the second issue in a trace). I think you could limit this safely to several hours either direction of the occurrence.

src/sentry/testutils/cases.py

Given a group, we look for the recommended event and search for any other errors in the trace. These trace-connected groups will be shown under the Related Issues tab. See the current look (more UI changes will be needed). <img width="933" alt="image" src="https://github.com/getsentry/sentry/assets/44410/b3f774d7-dc71-40b9-b144-65f143b6e981">

Follow up to #69237

sentry-io · 2024-04-25T17:49:23Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ AssertionError: assert 0 == 3 pytest.runtest.protocol tests/snuba/api/endpoin... View Issue

_{Did you find this useful? React with a 👍 or 👎}

**NOTE:** This is not the final UI but a step towards it. This shows the recently added trace-connected errors. This is a follow-up to the backend work: #69237 ![image](https://github.com/getsentry/sentry/assets/44410/a354acca-f6b4-4f9d-9ea6-9c0212f478c9)

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 18, 2024

vercel bot deployed to Preview April 18, 2024 19:33 View deployment

Base automatically changed from ref/trace_test_case/armenzg to master April 22, 2024 14:37

armenzg added 7 commits April 22, 2024 10:41

feat(related_issues): Return trace connected issues

2a9d203

Given an issue, if we can find an event with a trace, we may be able to find other issue in its trace. This can allow the customer to act on them together.

WIP

bf43555

Working on tests

a75931d

Do not return itself

5723d27

Fix trace loading. Next Snuba query

8bdd284

WIP: Testing trace connected errors

9bd9c92

Typing changes

887a548

armenzg force-pushed the feat/trace_connected_errors/armenzg branch from 05a39bd to 61497e4 Compare April 22, 2024 16:17

vercel bot deployed to Preview April 22, 2024 16:20 View deployment

armenzg added 6 commits April 22, 2024 13:39

Event typing changes

8cd88a3

WIP

5ed8c66

Merge branch 'master' into feat/trace_connected_errors/armenzg

41fb124

Minor changes

2a0b622

feat(discover): Prevent calling Snuba with an empty list of projects

7400e74

Merge branch 'master' into feat/trace_connected_errors/armenzg

16914f5

armenzg force-pushed the feat/trace_connected_errors/armenzg branch from 61497e4 to 16914f5 Compare April 23, 2024 17:58

vercel bot deployed to Preview April 23, 2024 18:00 View deployment

armenzg added 2 commits April 23, 2024 14:39

Geneate another project

ea134f1

Test is working

9371263

vercel bot deployed to Preview April 23, 2024 19:36 View deployment

Remove change

29fe3c1

armenzg commented Apr 23, 2024

View reviewed changes

armenzg added 2 commits April 23, 2024 15:51

Undo typing changes

434ad9d

Remove this

19542d6

armenzg requested a review from a team April 23, 2024 19:52

armenzg changed the title ~~WIP feat(related_issues): Trace connected errors~~ feat(related_issues): Trace connected errors Apr 23, 2024

armenzg marked this pull request as ready for review April 23, 2024 19:53

armenzg requested a review from a team as a code owner April 23, 2024 19:53

vercel bot deployed to Preview April 23, 2024 19:54 View deployment

armenzg self-assigned this Apr 23, 2024

Re-implement

46149c1

vercel bot deployed to Preview April 23, 2024 20:06 View deployment

vartec approved these changes Apr 23, 2024

View reviewed changes

armenzg requested a review from wmak April 23, 2024 21:31

wmak reviewed Apr 23, 2024

View reviewed changes

src/sentry/testutils/cases.py Show resolved Hide resolved

Adjust tests

ea2aebb

vercel bot deployed to Preview April 24, 2024 11:39 View deployment

Merge branch 'master' into feat/trace_connected_errors/armenzg

5e63c11

vercel bot deployed to Preview April 24, 2024 11:43 View deployment

armenzg enabled auto-merge (squash) April 24, 2024 11:57

armenzg merged commit 34d9222 into master Apr 24, 2024
48 checks passed

armenzg deleted the feat/trace_connected_errors/armenzg branch April 24, 2024 12:22

armenzg added this to the Related Issues - V1 milestone Apr 25, 2024

armenzg mentioned this pull request Apr 25, 2024

BE - ⏫ Trace-connected related issues #69615

Closed

2 tasks

armenzg linked an issue Apr 25, 2024 that may be closed by this pull request

BE - ⏫ Trace-connected related issues #69615

Closed

2 tasks

armenzg added a commit that referenced this pull request Apr 25, 2024

feat(related_issues): Display trace connected errors

b684923

Follow up to #69237

armenzg mentioned this pull request Apr 25, 2024

feat(related_issues): Display trace connected errors #69670

Merged

armenzg mentioned this pull request Apr 25, 2024

Related issues - MVP #69616

Closed

5 tasks

github-actions bot locked and limited conversation to collaborators May 14, 2024

		endpoint = "sentry-api-0-issues-related-issues"
		FEATURES: list[str] = []

Uh oh!

feat(related_issues): Trace connected errors #69237

feat(related_issues): Trace connected errors #69237

Uh oh!

Conversation

armenzg commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sentry-io bot commented Apr 25, 2024

Suspect Issues

Uh oh!

Uh oh!

armenzg commented Apr 18, 2024 •

edited

Loading

codecov bot commented Apr 18, 2024 •

edited

Loading