Skip to content

Commit ec1b0d1

Browse files
committed
timedelta
1 parent 0cb5c5a commit ec1b0d1

File tree

6 files changed

+261
-23
lines changed

6 files changed

+261
-23
lines changed

src/sentry/incidents/logic.py

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,8 @@ class AlertRuleNameAlreadyUsedError(Exception):
482482

483483
# Default values for `SnubaQuery.resolution`, in minutes.
484484
DEFAULT_ALERT_RULE_RESOLUTION = 1
485+
# Comparison alerts query twice (current + comparison window), so we scale
486+
# resolution down to compensate for the increased query load.
485487
DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER = 2
486488
DEFAULT_ALERT_RULE_WINDOW_TO_RESOLUTION = {
487489
30: 2,
@@ -505,13 +507,25 @@ class AlertRuleNameAlreadyUsedError(Exception):
505507
}
506508

507509

508-
def get_alert_resolution(time_window: int, organization: Organization) -> int:
510+
def get_alert_resolution(time_window: int, organization: Organization) -> timedelta:
511+
"""
512+
Return the Snuba subscription evaluation interval for a given alert time window.
513+
514+
Larger time windows don't need fine-grained resolution, so we map them to
515+
coarser buckets to reduce query load. See DEFAULT_ALERT_RULE_WINDOW_TO_RESOLUTION.
516+
517+
:param time_window: The alert's aggregation window, in minutes.
518+
:param organization: The organization (reserved for future per-org overrides).
519+
:return: The evaluation interval as a timedelta.
520+
"""
509521
index = bisect.bisect_right(SORTED_TIMEWINDOWS, time_window)
510522

511523
if index == 0:
512-
return DEFAULT_ALERT_RULE_RESOLUTION
524+
minutes = DEFAULT_ALERT_RULE_RESOLUTION
525+
else:
526+
minutes = DEFAULT_ALERT_RULE_WINDOW_TO_RESOLUTION[SORTED_TIMEWINDOWS[index - 1]]
513527

514-
return DEFAULT_ALERT_RULE_WINDOW_TO_RESOLUTION[SORTED_TIMEWINDOWS[index - 1]]
528+
return timedelta(minutes=minutes)
515529

516530

517531
class _OwnerKwargs(TypedDict):
@@ -585,7 +599,7 @@ def create_alert_rule(
585599
raise ResourceDoesNotExist("Your organization does not have access to this feature.")
586600

587601
if detection_type == AlertRuleDetectionType.DYNAMIC:
588-
resolution = time_window
602+
resolution = timedelta(minutes=time_window)
589603
# NOTE: we hardcode seasonality for EA
590604
seasonality = AlertRuleSeasonality.AUTO
591605
if not sensitivity:
@@ -617,8 +631,7 @@ def create_alert_rule(
617631
raise ValidationError("Comparison delta is not a valid field for this alert type")
618632

619633
if comparison_delta is not None:
620-
# Since comparison alerts make twice as many queries, run the queries less frequently.
621-
resolution = resolution * DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER
634+
resolution *= DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER
622635
comparison_delta = int(timedelta(minutes=comparison_delta).total_seconds())
623636

624637
with transaction.atomic(router.db_for_write(SnubaQuery)):
@@ -629,7 +642,7 @@ def create_alert_rule(
629642
query=query,
630643
aggregate=aggregate,
631644
time_window=timedelta(minutes=time_window),
632-
resolution=timedelta(minutes=resolution),
645+
resolution=resolution,
633646
environment=environment,
634647
event_types=event_types,
635648
extrapolation_mode=extrapolation_mode,
@@ -900,11 +913,9 @@ def update_alert_rule(
900913
)
901914

902915
if resolution_comparison_delta is not None:
903-
updated_query_fields["resolution"] = timedelta(
904-
minutes=(resolution * DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER)
905-
)
906-
else:
907-
updated_query_fields["resolution"] = timedelta(minutes=resolution)
916+
resolution *= DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER
917+
918+
updated_query_fields["resolution"] = resolution
908919

909920
if detection_type:
910921
updated_fields["detection_type"] = detection_type
@@ -918,8 +929,10 @@ def update_alert_rule(
918929
updated_fields["seasonality"] = None
919930
elif detection_type == AlertRuleDetectionType.DYNAMIC:
920931
# NOTE: we set seasonality for EA
921-
updated_query_fields["resolution"] = timedelta(
922-
minutes=time_window if time_window is not None else snuba_query.time_window
932+
updated_query_fields["resolution"] = (
933+
timedelta(minutes=time_window)
934+
if time_window is not None
935+
else timedelta(seconds=snuba_query.time_window)
923936
)
924937
updated_fields["seasonality"] = AlertRuleSeasonality.AUTO
925938
updated_fields["comparison_delta"] = None

src/sentry/incidents/metric_issue_detector.py

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@
88

99
from sentry import features, quotas
1010
from sentry.constants import ObjectStatus
11-
from sentry.incidents.logic import enable_disable_subscriptions
11+
from sentry.incidents.logic import (
12+
DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER,
13+
enable_disable_subscriptions,
14+
get_alert_resolution,
15+
)
1216
from sentry.incidents.models.alert_rule import AlertRuleDetectionType
1317
from sentry.relay.config.metric_extraction import on_demand_metrics_feature_flags
1418
from sentry.search.eap.trace_metrics.validator import validate_trace_metrics_aggregate
@@ -239,6 +243,24 @@ def _validate_extrapolation_mode(self, extrapolation_mode: ExtrapolationMode) ->
239243
f"{extrapolation_mode.name.lower()} extrapolation mode is not supported for new detectors. Allowed modes are: client_and_server_weighted, unknown."
240244
)
241245

246+
def _get_resolution_for_window(
247+
self, time_window_seconds: int, detection_type: str | None, comparison_delta: int | None
248+
) -> timedelta:
249+
"""
250+
Compute the appropriate SnubaQuery resolution for a given time window,
251+
mirroring the logic in create_alert_rule / update_alert_rule.
252+
"""
253+
organization = self.context["organization"]
254+
255+
if detection_type == AlertRuleDetectionType.DYNAMIC:
256+
resolution = timedelta(seconds=time_window_seconds)
257+
else:
258+
resolution = get_alert_resolution(time_window_seconds // 60, organization)
259+
if comparison_delta is not None:
260+
resolution *= DEFAULT_CMP_ALERT_RULE_RESOLUTION_MULTIPLIER
261+
262+
return resolution
263+
242264
def get_quota(self) -> DetectorQuota:
243265
organization = self.context.get("organization")
244266
request = self.context.get("request")
@@ -336,14 +358,21 @@ def update_data_source(
336358
data_source.get("extrapolation_mode", snuba_query.extrapolation_mode)
337359
)
338360

361+
new_time_window = data_source.get("time_window", snuba_query.time_window)
362+
resolution = self._get_resolution_for_window(
363+
new_time_window,
364+
instance.config.get("detection_type"),
365+
instance.config.get("comparison_delta"),
366+
)
367+
339368
update_snuba_query(
340369
snuba_query=snuba_query,
341370
query_type=data_source.get("query_type", snuba_query.type),
342371
dataset=data_source.get("dataset", snuba_query.dataset),
343372
query=data_source.get("query", snuba_query.query),
344373
aggregate=data_source.get("aggregate", snuba_query.aggregate),
345-
time_window=timedelta(seconds=data_source.get("time_window", snuba_query.time_window)),
346-
resolution=timedelta(seconds=data_source.get("resolution", snuba_query.resolution)),
374+
time_window=timedelta(seconds=new_time_window),
375+
resolution=resolution,
347376
environment=data_source.get("environment", snuba_query.environment),
348377
event_types=data_source.get("event_types", [event_type for event_type in event_types]),
349378
extrapolation_mode=extrapolation_mode,
@@ -407,6 +436,10 @@ def update(self, instance: Detector, validated_data: dict[str, Any]):
407436

408437
if data_source is not None:
409438
self.update_data_source(instance, data_source, seer_updated)
439+
elif "config" in validated_data:
440+
# Config changed (e.g. detection_type or comparison_delta) without a
441+
# data_source update — recalculate resolution to match the new config.
442+
self.update_data_source(instance, {}, seer_updated)
410443

411444
instance.save()
412445

@@ -415,10 +448,20 @@ def update(self, instance: Detector, validated_data: dict[str, Any]):
415448

416449
def create(self, validated_data: dict[str, Any]):
417450
if "data_sources" in validated_data:
451+
config = validated_data.get("config", {})
452+
detection_type = config.get("detection_type")
453+
comparison_delta = config.get("comparison_delta")
454+
418455
for validated_data_source in validated_data["data_sources"]:
419456
self._validate_transaction_dataset_deprecation(validated_data_source.get("dataset"))
420457
self._validate_extrapolation_mode(validated_data_source.get("extrapolation_mode"))
421458

459+
time_window = validated_data_source.get("time_window")
460+
if time_window is not None:
461+
validated_data_source["resolution"] = self._get_resolution_for_window(
462+
time_window, detection_type, comparison_delta
463+
)
464+
422465
detector = super().create(validated_data)
423466

424467
if detector.config.get("detection_type") == AlertRuleDetectionType.DYNAMIC.value:

src/sentry/snuba/models.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ class Type(Enum):
7272
size=100,
7373
)
7474
aggregate = models.TextField()
75+
# The aggregation window for the query, in seconds.
7576
time_window = models.IntegerField()
77+
# How often the subscription query is evaluated, in seconds. Scaled up for
78+
# larger time windows and comparison alerts to reduce query load.
7679
resolution = models.IntegerField()
7780
extrapolation_mode = models.IntegerField(
7881
choices=ExtrapolationMode.as_choices(),

src/sentry/snuba/snuba_query_validator.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ def create_source(self, validated_data: dict[str, Any]) -> QuerySubscription:
483483
query=validated_data["query"],
484484
aggregate=validated_data["aggregate"],
485485
time_window=timedelta(seconds=validated_data["time_window"]),
486-
resolution=timedelta(minutes=1),
486+
resolution=validated_data.get("resolution", timedelta(minutes=1)),
487487
environment=validated_data["environment"],
488488
event_types=validated_data["event_types"],
489489
group_by=validated_data.get("group_by"),

0 commit comments

Comments
 (0)