Skip to content

Commit c7050ec

Browse files
committed
feat(alerts): Allow arithmetic in alert validation
- This updates the validators to allow arithmetic - Unsure how to test the actual arithmetic works but this no longer errors haha
1 parent 94daa28 commit c7050ec

File tree

3 files changed

+99
-27
lines changed

3 files changed

+99
-27
lines changed

src/sentry/incidents/logic.py

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from copy import deepcopy
77
from dataclasses import dataclass, replace
88
from datetime import datetime, timedelta, timezone
9+
from re import Match
910
from typing import Any, TypedDict
1011
from uuid import UUID, uuid4
1112

@@ -24,6 +25,7 @@
2425
from sentry.db.models import Model
2526
from sentry.db.models.manager.base_query_set import BaseQuerySet
2627
from sentry.deletions.models.scheduleddeletion import CellScheduledDeletion
28+
from sentry.discover.arithmetic import is_equation, parse_arithmetic, strip_equation
2729
from sentry.incidents import tasks
2830
from sentry.incidents.events import IncidentCreatedEvent, IncidentStatusUpdatedEvent
2931
from sentry.incidents.models.alert_rule import (
@@ -1848,12 +1850,15 @@ def get_opsgenie_teams(organization_id: int, integration_id: int) -> list[tuple[
18481850

18491851

18501852
def get_column_from_aggregate(
1851-
aggregate: str, allow_mri: bool, allow_eap: bool = False
1853+
aggregate: str,
1854+
allow_mri: bool,
1855+
allow_eap: bool = False,
1856+
match: Match[str] | None = None,
18521857
) -> str | None:
18531858
# These functions exist as SnQLFunction definitions and are not supported in the older
18541859
# logic for resolving functions. We parse these using `fields.is_function`, otherwise
18551860
# they will fail using the old resolve_field logic.
1856-
match = is_function(aggregate)
1861+
match = is_function(aggregate) if match is None else match
18571862
if match and (
18581863
match.group("function") in SPANS_METRICS_FUNCTIONS
18591864
or match.group("function") in METRICS_LAYER_UNSUPPORTED_TRANSACTION_METRICS_FUNCTIONS
@@ -1900,21 +1905,29 @@ def check_aggregate_column_support(
19001905
aggregate: str, allow_mri: bool = False, allow_eap: bool = False
19011906
) -> bool:
19021907
# TODO(ddm): remove `allow_mri` once the experimental feature flag is removed.
1903-
column = get_column_from_aggregate(aggregate, allow_mri, allow_eap)
1904-
match = is_function(aggregate)
1905-
function = match.group("function") if match else None
1906-
return (
1907-
column is None
1908-
or is_measurement(column)
1909-
or column in SUPPORTED_COLUMNS
1910-
or column in TRANSLATABLE_COLUMNS
1911-
or (is_mri(column) and allow_mri)
1912-
or (
1913-
isinstance(function, str)
1914-
and column in INSIGHTS_FUNCTION_VALID_ARGS_MAP.get(function, [])
1915-
)
1916-
or allow_eap
1917-
)
1908+
if is_equation(aggregate):
1909+
_, _, terms = parse_arithmetic(strip_equation(aggregate))
1910+
else:
1911+
terms = [aggregate]
1912+
1913+
for term in terms:
1914+
match = is_function(term)
1915+
column = get_column_from_aggregate(term, allow_mri, allow_eap, match)
1916+
function = match.group("function") if match else None
1917+
if not (
1918+
column is None
1919+
or is_measurement(column)
1920+
or column in SUPPORTED_COLUMNS
1921+
or column in TRANSLATABLE_COLUMNS
1922+
or (is_mri(column) and allow_mri)
1923+
or (
1924+
isinstance(function, str)
1925+
and column in INSIGHTS_FUNCTION_VALID_ARGS_MAP.get(function, [])
1926+
)
1927+
or allow_eap
1928+
):
1929+
return False
1930+
return True
19181931

19191932

19201933
def translate_aggregate_field(

src/sentry/search/eap/trace_metrics/validator.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
from rest_framework import serializers
55

6+
from sentry.discover.arithmetic import is_equation, parse_arithmetic, strip_equation
67
from sentry.exceptions import InvalidSearchQuery
78
from sentry.search.eap.resolver import SearchResolver
89
from sentry.search.eap.trace_metrics.config import TraceMetricsSearchResolverConfig
@@ -58,14 +59,20 @@ def validate_trace_metrics_aggregate(aggregate: str) -> None:
5859
Raises:
5960
serializers.ValidationError: If the aggregate is invalid
6061
"""
61-
try:
62-
trace_metric = extract_trace_metric_from_aggregate(aggregate)
63-
if trace_metric is None:
64-
raise InvalidSearchQuery(
65-
f"Trace metrics aggregate {aggregate} must specify metric name, type, and unit"
62+
if is_equation(aggregate):
63+
_, _, terms = parse_arithmetic(strip_equation(aggregate))
64+
else:
65+
terms = [aggregate]
66+
67+
for term in terms:
68+
try:
69+
trace_metric = extract_trace_metric_from_aggregate(term)
70+
if trace_metric is None:
71+
raise InvalidSearchQuery(
72+
f"Trace metrics aggregate {term} must specify metric name, type, and unit"
73+
)
74+
except InvalidSearchQuery as e:
75+
logger.exception(f"Invalid trace metrics aggregate: {term} {e}")
76+
raise serializers.ValidationError(
77+
{"aggregate": f"Invalid trace metrics aggregate: {term}"}
6678
)
67-
except InvalidSearchQuery as e:
68-
logger.exception("Invalid trace metrics aggregate: %s %s", aggregate, e)
69-
raise serializers.ValidationError(
70-
{"aggregate": f"Invalid trace metrics aggregate: {aggregate}"}
71-
)

tests/sentry/workflow_engine/endpoints/test_organization_project_detector_index.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,58 @@ def test_use_transactions_instead_of_generic_metrics_dataset(self) -> None:
426426
assert query_sub.snuba_query.aggregate == "count()"
427427
assert query_sub.snuba_query.event_types == [SnubaQueryEventType.EventType.TRANSACTION]
428428

429+
@with_feature(
430+
[
431+
"organizations:tracemetrics-alerts",
432+
"organizations:tracemetrics-enabled",
433+
]
434+
)
435+
def test_use_metrics_equation_as_aggregate(self) -> None:
436+
data = {**self.valid_data}
437+
equation = 'equation|count_if(`agent_name:"Agent Run"`,value,request_duration,distribution,none) * 2'
438+
data["dataSources"] = [
439+
{
440+
"queryType": SnubaQuery.Type.PERFORMANCE.value,
441+
"dataset": Dataset.EventsAnalyticsPlatform.value,
442+
"query": "",
443+
"aggregate": equation,
444+
"timeWindow": 300,
445+
"environment": self.environment.name,
446+
"eventTypes": [SnubaQueryEventType.EventType.TRACE_ITEM_METRIC.name.lower()],
447+
}
448+
]
449+
450+
with self.tasks():
451+
response = self.get_success_response(
452+
self.organization.slug,
453+
self.project.slug,
454+
**data,
455+
status_code=201,
456+
)
457+
458+
assert (
459+
response.data["dataSources"][0]["queryObj"]["snubaQuery"]["dataset"]
460+
== Dataset.EventsAnalyticsPlatform.value
461+
)
462+
assert response.data["dataSources"][0]["queryObj"]["snubaQuery"]["query"] == ""
463+
assert response.data["dataSources"][0]["queryObj"]["snubaQuery"]["aggregate"] == equation
464+
465+
detector = Detector.objects.get(id=response.data["id"])
466+
data_source = DataSource.objects.get(detector=detector)
467+
assert data_source.type == data_source_type_registry.get_key(
468+
QuerySubscriptionDataSourceHandler
469+
)
470+
assert data_source.organization_id == self.organization.id
471+
query_sub = QuerySubscription.objects.get(id=int(data_source.source_id))
472+
assert query_sub.project == self.project
473+
assert query_sub.snuba_query.type == SnubaQuery.Type.PERFORMANCE.value
474+
assert query_sub.snuba_query.dataset == Dataset.EventsAnalyticsPlatform.value
475+
assert query_sub.snuba_query.query == ""
476+
assert query_sub.snuba_query.aggregate == equation
477+
assert query_sub.snuba_query.event_types == [
478+
SnubaQueryEventType.EventType.TRACE_ITEM_METRIC
479+
]
480+
429481

430482
@cell_silo_test
431483
class OrganizationProjectDetectorIndexMonitorPostTest(APITestCase):

0 commit comments

Comments
 (0)