Skip to content

Commit 99f380b

Browse files
ceorourkeandrewshie-sentry
authored andcommitted
chore(ACI): Add logs for triggers to compare both systems (#98758)
Add logs on both the subscription processor when a trigger's threshold is met or resolved and in workflow engine when the workflow's trigger conditions are met so that we can compare the two systems. Both logs have the detector ID so we can match them up and are behind a flag with a small subset of organizations flagged in so it won't be too much data.
1 parent 7be1981 commit 99f380b

File tree

3 files changed

+74
-15
lines changed

3 files changed

+74
-15
lines changed

src/sentry/incidents/subscription_processor.py

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,13 @@ def handle_trigger_alerts(
325325
aggregation_value: float,
326326
fired_incident_triggers: list[IncidentTrigger],
327327
metrics_incremented: bool,
328+
detector: Detector | None,
328329
) -> tuple[list[IncidentTrigger], bool]:
329330
# OVER/UNDER value trigger
331+
has_dual_processing_flag = features.has(
332+
"organizations:workflow-engine-metric-alert-dual-processing-logs",
333+
self.subscription.project.organization,
334+
)
330335
alert_operator, resolve_operator = self.THRESHOLD_TYPE_OPERATORS[
331336
AlertRuleThresholdType(self.alert_rule.threshold_type)
332337
]
@@ -339,15 +344,20 @@ def handle_trigger_alerts(
339344
"incidents.alert_rules.threshold.alert",
340345
tags={"detection_type": self.alert_rule.detection_type},
341346
)
342-
if (
343-
features.has(
344-
"organizations:workflow-engine-metric-alert-dual-processing-logs",
345-
self.subscription.project.organization,
346-
)
347-
and not metrics_incremented
348-
):
349-
metrics.incr("dual_processing.alert_rules.fire")
350-
metrics_incremented = True
347+
if has_dual_processing_flag:
348+
if detector is not None:
349+
logger.info(
350+
"subscription_processor.alert_triggered",
351+
extra={
352+
"rule_id": self.alert_rule.id,
353+
"detector_id": detector.id,
354+
"organization_id": self.subscription.project.organization.id,
355+
"project_id": self.subscription.project.id,
356+
},
357+
)
358+
if not metrics_incremented:
359+
metrics.incr("dual_processing.alert_rules.fire")
360+
metrics_incremented = True
351361
# triggering a threshold will create an incident and set the status to active
352362
incident_trigger = self.trigger_alert_threshold(trigger, aggregation_value)
353363
if incident_trigger is not None:
@@ -364,10 +374,17 @@ def handle_trigger_alerts(
364374
"incidents.alert_rules.threshold.resolve",
365375
tags={"detection_type": self.alert_rule.detection_type},
366376
)
367-
if features.has(
368-
"organizations:workflow-engine-metric-alert-dual-processing-logs",
369-
self.subscription.project.organization,
370-
):
377+
if has_dual_processing_flag:
378+
if detector is not None:
379+
logger.info(
380+
"subscription_processor.alert_triggered",
381+
extra={
382+
"rule_id": self.alert_rule.id,
383+
"detector_id": detector.id,
384+
"organization_id": self.subscription.project.organization.id,
385+
"project_id": self.subscription.project.id,
386+
},
387+
)
371388
metrics.incr("dual_processing.alert_rules.resolve")
372389
incident_trigger = self.trigger_resolve_threshold(trigger, aggregation_value)
373390

@@ -615,7 +632,11 @@ def process_update(self, subscription_update: QuerySubscriptionUpdate) -> None:
615632
return
616633

617634
fired_incident_triggers, metrics_incremented = self.handle_trigger_alerts(
618-
trigger, aggregation_value, fired_incident_triggers, metrics_incremented
635+
trigger,
636+
aggregation_value,
637+
fired_incident_triggers,
638+
metrics_incremented,
639+
detector,
619640
)
620641

621642
if fired_incident_triggers:

src/sentry/workflow_engine/processors/workflow.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@
1414
from sentry.services.eventstore.models import GroupEvent
1515
from sentry.utils import json
1616
from sentry.workflow_engine import buffer
17-
from sentry.workflow_engine.models import Action, DataConditionGroup, Detector, Workflow
17+
from sentry.workflow_engine.models import (
18+
Action,
19+
DataConditionGroup,
20+
Detector,
21+
DetectorWorkflow,
22+
Workflow,
23+
)
1824
from sentry.workflow_engine.models.workflow_data_condition_group import WorkflowDataConditionGroup
1925
from sentry.workflow_engine.processors.contexts.workflow_event_context import (
2026
WorkflowEventContext,
@@ -183,6 +189,23 @@ def evaluate_workflow_triggers(
183189
else:
184190
if evaluation:
185191
triggered_workflows.add(workflow)
192+
if features.has(
193+
"organizations:workflow-engine-metric-alert-dual-processing-logs",
194+
workflow.organization,
195+
):
196+
try:
197+
detector_workflow = DetectorWorkflow.objects.get(workflow_id=workflow.id)
198+
logger.info(
199+
"workflow_engine.process_workflows.workflow_triggered",
200+
extra={
201+
"workflow_id": workflow.id,
202+
"detector_id": detector_workflow.detector_id,
203+
"organization_id": workflow.organization.id,
204+
"project_id": event_data.group.project.id,
205+
},
206+
)
207+
except DetectorWorkflow.DoesNotExist:
208+
continue
186209

187210
metrics_incr(
188211
"process_workflows.triggered_workflows",

tests/sentry/workflow_engine/processors/test_workflow.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from sentry.services.eventstore.models import GroupEvent
1212
from sentry.testutils.factories import Factories
1313
from sentry.testutils.helpers.datetime import before_now, freeze_time
14+
from sentry.testutils.helpers.features import with_feature
1415
from sentry.testutils.helpers.options import override_options
1516
from sentry.testutils.helpers.redis import mock_redis_buffer
1617
from sentry.testutils.pytest.fixtures import django_db_all
@@ -337,6 +338,20 @@ def test_workflow_trigger(self) -> None:
337338
)
338339
assert triggered_workflows == {self.workflow}
339340

341+
@with_feature("organizations:workflow-engine-metric-alert-dual-processing-logs")
342+
@patch("sentry.workflow_engine.processors.workflow.logger")
343+
def test_logs_triggered_workflows(self, mock_logger: MagicMock) -> None:
344+
evaluate_workflow_triggers({self.workflow}, self.event_data, self.event_start_time)
345+
mock_logger.info.assert_called_once_with(
346+
"workflow_engine.process_workflows.workflow_triggered",
347+
extra={
348+
"workflow_id": self.workflow.id,
349+
"detector_id": self.detector.id,
350+
"organization_id": self.workflow.organization.id,
351+
"project_id": self.event_data.group.project.id,
352+
},
353+
)
354+
340355
def test_workflow_trigger__no_conditions(self) -> None:
341356
assert self.workflow.when_condition_group
342357
self.workflow.when_condition_group.conditions.all().delete()

0 commit comments

Comments
 (0)