feat(workflow_engine): Add evaluation logs to detectors - #121908
feat(workflow_engine): Add evaluation logs to detectors#121908saponifi3d wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
Scope: Backend
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| from sentry.workflow_engine.models.detector_group import DetectorGroup | ||
| from sentry.workflow_engine.processors import DetectorEvaluation | ||
| from sentry.workflow_engine.processors import DetectorEvaluation, ProcessDetectorsResult | ||
| from sentry.workflow_engine.processors.evaluation_logging import emit_detector_evaluation_logs |
There was a problem hiding this comment.
Detector batch processing aborts on stale organization reference
The new emit_detector_evaluation_logs call in process_detectors uses _get_detector_organization without exception handling, causing the detector loop to abort on stale references or invalid config.
Evidence
- Line 26 imports emit_detector_evaluation_logs, enabling the new logging path in process_detectors.
- At line 310, emit_detector_evaluation_logs receives _get_detector_organization(detector) as its organization argument.
- _get_detector_organization (line 276) accesses detector.linked_project.organization, which raises Project.DoesNotExist if the project was deleted.
- For organization-scoped detectors it calls Organization.objects.get_from_cache(id=organization_id) (line 283), which raises Organization.DoesNotExist when the organization was deleted.
- It also explicitly raises ValueError at line 282 when organization_id is missing or not an integer.
- Because these exceptions are unhandled inside the for detector in detectors: loop, a single bad detector causes process_detectors to abort, skipping all remaining detectors and never creating issue platform payloads for them.
Also found at 1 additional location
src/sentry/workflow_engine/processors/detector.py:282
Identified by Warden · sentry-backend-bugs · DQ2-XJ4
saponifi3d
left a comment
saponifi3d
left a comment
There was a problem hiding this comment.
quick review / notes for the end of the day.
| event_data: dict[str, Any] | None # TODO - improve this typing, for now migrating | ||
|
|
||
|
|
||
| class DetectorEvaluationOutcome(StrEnum): |
There was a problem hiding this comment.
ERROR also seems like a valid outcome to this evaluation.
|
|
||
| @property | ||
| def outcome(self) -> DetectorEvaluationOutcome: | ||
| if self.evaluations: |
There was a problem hiding this comment.
if the evaluation has an error in it, we should proxy that error up to this status as well.
| def to_artifact(self) -> dict[str, object]: | ||
| return { | ||
| "detector_id": self.detector_id, | ||
| "detector_type": self.detector_type, | ||
| "project_id": self.project_id, | ||
| "outcome": self.outcome, | ||
| } |
There was a problem hiding this comment.
this should use artifact_data and then invoke to_artifact to layer in the errors as well.
|
|
||
| organization_id = detector.config.get("organization_id") | ||
| if not isinstance(organization_id, int): | ||
| raise ValueError("Organization-scoped detector is missing organization_id") |
There was a problem hiding this comment.
We should raise a DetectorProcessing exception here instead of a value error, then catch it in process_detectors. the except block should then create an Error evaluation for it, and leave the message as the error message.
1 participant
Description
Use the
DetectorEvaluationto log the results of each detector. This was previously only done for cases when we had a next step (trigger or resolve the detector), now we log every evaluation.