ref(alerts): Update Snuba queries to match events-stats more closely #77755

ceorourke · 2024-09-18T23:26:29Z

When a user creates an anomaly detection alert we need to query snuba for 28 days worth of historical data to send to Seer to calculate the anomalies. Originally (#74614) I'd tried to pull out the relevant parts of the events-stats endpoint to mimic the data we see populated in metric alert preview charts (but for a larger time period, and it's happening after the rule is saved so I can't use any of the request object stuff) but I think I missed some things, so this PR aims to make that data be the same.

Closes https://getsentry.atlassian.net/browse/ALRT-288 (hopefully)

TODO

Double check each metric alert type's events-stats SQL output against anomaly detection's
Try to put crash rate alerts back in there

ceorourke · 2024-09-18T23:46:57Z

src/sentry/api/bases/organization_events.py

@@ -42,6 +42,27 @@
 from sentry.utils.snuba import MAX_FIELDS, SnubaTSResult


+def get_query_columns(columns, rollup):


I moved this to be reused by anomaly detection

ceorourke · 2024-09-18T23:48:00Z

src/sentry/seer/anomaly_detection/utils.py

    """
+    serializer = SnubaTSResultSerializer(organization=organization, lookup=None, user=None)


I'm using the same serializer the events-stats endpoint uses and just pulling that data off to format into a list of TimeSeriesPoints for Seer's API. I clicked through every alert type and it always has the timestamp and count

ceorourke · 2024-09-18T23:49:56Z

src/sentry/seer/anomaly_detection/utils.py

+        data,
+        resolve_axis_column(query_columns[0]),
+        allow_partial_buckets=False,
+        zerofill_results=False,


I was getting strange results in tests with this set to True, and for our purposes I think it doesn't matter that much since we default to sending Seer a 0 if we don't find a count anyway

By "strange" I mean it was hitting this line and overwriting data with a count I had in a test as an empty array.

codecov · 2024-09-19T00:17:41Z

❌ 3 Tests Failed:

Tests completed	Failed	Passed	Skipped
21625	3	21622	207

View the top 3 failed tests by shortest run time

tests.sentry.incidents.endpoints.test_organization_alert_rule_anomalies.AlertRuleAnomalyEndpointTest test_seer_error

Stack Traces | 8.74s run time

#x1B[1m#x1B[.../incidents/endpoints/test_organization_alert_rule_anomalies.py#x1B[0m:276: in test_seer_error
    resp = self.get_error_response(
#x1B[1m#x1B[.../sentry/testutils/cases.py#x1B[0m:793: in get_error_response
    assert_status_code(response, status_code)
#x1B[1m#x1B[.../sentry/testutils/asserts.py#x1B[0m:39: in assert_status_code
    assert minimum &lt;= response.status_code &lt; maximum, (
#x1B[1m#x1B[31mE   AssertionError: (200, b'[]')#x1B[0m
#x1B[1m#x1B[31mE   assert 400 &lt;= 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 200 = &lt;Response status_code=200, "application/json"&gt;.status_code#x1B[0m

tests.sentry.incidents.endpoints.test_organization_alert_rule_anomalies.AlertRuleAnomalyEndpointTest test_simple

Stack Traces | 8.98s run time

#x1B[1m#x1B[.../incidents/endpoints/test_organization_alert_rule_anomalies.py#x1B[0m:115: in test_simple
    assert mock_seer_request.call_count == 1
#x1B[1m#x1B[31mE   AssertionError: assert 0 == 1#x1B[0m
#x1B[1m#x1B[31mE    +  where 0 = &lt;MagicMock name='urlopen' id='139907010458256'&gt;.call_count#x1B[0m

tests.sentry.incidents.endpoints.test_organization_alert_rule_anomalies.AlertRuleAnomalyEndpointTest test_timeout

Stack Traces | 9.07s run time

#x1B[1m#x1B[.../incidents/endpoints/test_organization_alert_rule_anomalies.py#x1B[0m:204: in test_timeout
    resp = self.get_error_response(
#x1B[1m#x1B[.../sentry/testutils/cases.py#x1B[0m:793: in get_error_response
    assert_status_code(response, status_code)
#x1B[1m#x1B[.../sentry/testutils/asserts.py#x1B[0m:39: in assert_status_code
    assert minimum &lt;= response.status_code &lt; maximum, (
#x1B[1m#x1B[31mE   AssertionError: (200, b'[]')#x1B[0m
#x1B[1m#x1B[31mE   assert 400 &lt;= 200#x1B[0m
#x1B[1m#x1B[31mE    +  where 200 = &lt;Response status_code=200, "application/json"&gt;.status_code#x1B[0m

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

ceorourke · 2024-09-19T21:45:33Z

src/sentry/seer/anomaly_detection/utils.py

+        stats_period=None,
+        environments=environments,
+    )
+    snuba_query_string = get_snuba_query_string(snuba_query)


This is one of the key changes here - the front end constructs a stringified query based on snuba_query.query AND snuba_query.event_types. This adds a join to the table for things like errors count with the is:unresolved query, or when you're using the dropdown to select "errors", "default", or "errors OR default" event types

ceorourke · 2024-09-20T00:21:45Z

The users experiencing errors query is selecting data as a different name but it's otherwise the same, I don't know if that makes a difference to the outcome?
events-stats:

SELECT (events._snuba_events.time AS _snuba_events.time), (uniq((events._snuba_events.tags[sentry:user] AS _snuba_events.tags[sentry:user])) AS _snuba_count_unique_user)

anomaly detection:
SELECT (events._snuba_events.time AS _snuba_events.time), (uniq((events._snuba_events.tags[sentry:user] AS _snuba_events.tags[sentry:user])) AS _snuba_count_unique_tags_sentry_user)

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

vercel bot deployed to Preview September 18, 2024 23:27 View deployment

vercel bot deployed to Preview September 18, 2024 23:35 View deployment

vercel bot deployed to Preview September 18, 2024 23:44 View deployment

ceorourke commented Sep 18, 2024

View reviewed changes

vercel bot deployed to Preview September 18, 2024 23:55 View deployment

vercel bot deployed to Preview September 19, 2024 00:35 View deployment

ceorourke added 8 commits September 19, 2024 10:50

fix snuba query

0fc6cfa

use SnubaTSResultSerializer

bdab736

update some tests

0bf63c7

formatting and update a missing org param

a95aefb

typing

eea294f

be safer, don't need to put ts in a var

f17c46e

oops fix count

7e8170f

fix environments param

4dfe836

ceorourke force-pushed the ceorourke/anomaly-detection-no-none-values branch from 634ed2c to 4dfe836 Compare September 19, 2024 18:00

vercel bot deployed to Preview September 19, 2024 18:04 View deployment

add snuba query event type to query

a00fb6f

vercel bot deployed to Preview September 19, 2024 21:44 View deployment

ceorourke commented Sep 19, 2024

View reviewed changes

make sure it works with and w/o is:unresolved queries

6357580

vercel bot deployed to Preview September 19, 2024 22:30 View deployment

realize I only thought I needed that because the event type didn't match

136d058

vercel bot deployed to Preview September 20, 2024 00:21 View deployment

ceorourke requested a review from wedamija September 20, 2024 00:22

ceorourke mentioned this pull request Sep 20, 2024

feat(anomaly detection):preview chart proxy api endpoint #77813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ref(alerts): Update Snuba queries to match events-stats more closely #77755

ref(alerts): Update Snuba queries to match events-stats more closely #77755

ceorourke commented Sep 18, 2024 •

edited

Loading

ceorourke Sep 18, 2024

ceorourke Sep 18, 2024 •

edited

Loading

ceorourke Sep 18, 2024 •

edited

Loading

ceorourke Sep 19, 2024

codecov bot commented Sep 19, 2024 •

edited

Loading

ceorourke Sep 19, 2024

ceorourke commented Sep 20, 2024

		@@ -42,6 +42,27 @@
		from sentry.utils.snuba import MAX_FIELDS, SnubaTSResult


		def get_query_columns(columns, rollup):

		"""
		serializer = SnubaTSResultSerializer(organization=organization, lookup=None, user=None)

ref(alerts): Update Snuba queries to match events-stats more closely #77755

Are you sure you want to change the base?

ref(alerts): Update Snuba queries to match events-stats more closely #77755

Conversation

ceorourke commented Sep 18, 2024 • edited Loading

ceorourke Sep 18, 2024

Choose a reason for hiding this comment

ceorourke Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

ceorourke Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

ceorourke Sep 19, 2024

Choose a reason for hiding this comment

codecov bot commented Sep 19, 2024 • edited Loading

❌ 3 Tests Failed:

ceorourke Sep 19, 2024

Choose a reason for hiding this comment

ceorourke commented Sep 20, 2024

ceorourke commented Sep 18, 2024 •

edited

Loading

ceorourke Sep 18, 2024 •

edited

Loading

ceorourke Sep 18, 2024 •

edited

Loading

codecov bot commented Sep 19, 2024 •

edited

Loading