WIP: Big ole DetectorSettings refactor by kcons · Pull Request #112888 · getsentry/sentry

kcons · 2026-04-13T23:52:14Z

Goal is simple: GroupType use shouldn't result in importing anything substantial from workflow engine, let alone specific validator implementations.

We break the chain with an explicit DetectorType thing that should exist for every GroupType we have meaningful Detector implementations for.

sentry-warden · 2026-04-14T00:01:53Z

src/sentry/preprod/size_analysis/detectorconfig.py

+                return self._extract_head(data_packet) - self._extract_base(data_packet)
+            case "relative_diff":
+                base = self._extract_base(data_packet)
+                return ((self._extract_head(data_packet) - base) / base) * 100


Division by zero in extract_value when base size is zero

When threshold_type is relative_diff, the code divides by base without checking if it's zero. If base_install_size_bytes or base_download_size_bytes is 0 in the data packet, self._extract_base() returns 0 and the expression ((self._extract_head(data_packet) - base) / base) * 100 raises ZeroDivisionError. This crashes the detector evaluation, preventing issue detection for that data packet.

Verification

Traced extract_value method at line 239-250 in detectorconfig.py. Confirmed _extract_base returns int from data_packet.packet.get('base_install_size_bytes') or base_download_size_bytes. The TypedDict in grouptype.py defines these as int | None, meaning 0 is a valid value. Division occurs at line 248 without zero-check. The helper function _build_evidence_text at line 131 correctly guards against this with if base_bytes else 0.

Identified by Warden sentry-backend-bugs · 8X5-S26

sentry-warden · 2026-04-14T00:01:54Z

src/sentry/uptime/detectorconfig.py

+        recovery_threshold = self.detector.config["recovery_threshold"]
+        downtime_threshold = self.detector.config["downtime_threshold"]


KeyError accessing threshold config keys before backfill migration completes

The code accesses self.detector.config["recovery_threshold"] and self.detector.config["downtime_threshold"] with direct bracket notation at lines 97-98 and 208. The backfill migration 0045_backfill_detector_thresholds.py that adds these keys is marked is_post_deployment = True, meaning it runs after code deployment. During this window, existing uptime detectors will lack these keys, causing KeyError when the handler processes check results.

Verification

Verified by reading the migration at src/sentry/uptime/migrations/0045_backfill_detector_thresholds.py which shows is_post_deployment = True at line 51. Traced code path from consumer at results_consumer.py:468 calling process_detectors() which invokes the detector handler and accesses the thresholds property. Also verified line 208 has the same issue.

Suggested fix: Use .get() with default values to handle missing keys gracefully during the migration window

Suggested change

recovery_threshold = self.detector.config["recovery_threshold"]

downtime_threshold = self.detector.config["downtime_threshold"]

from sentry.uptime.types import DEFAULT_DOWNTIME_THRESHOLD, DEFAULT_RECOVERY_THRESHOLD

recovery_threshold = self.detector.config.get("recovery_threshold", DEFAULT_RECOVERY_THRESHOLD)

downtime_threshold = self.detector.config.get("downtime_threshold", DEFAULT_DOWNTIME_THRESHOLD)

from sentry.uptime.types import DEFAULT_DOWNTIME_THRESHOLD

threshold = self.detector.config.get("downtime_threshold", DEFAULT_DOWNTIME_THRESHOLD)

Identified by Warden sentry-backend-bugs · A4S-9GJ

github-actions · 2026-04-14T00:06:05Z

Backend Test Failures

Failures on 630131f in this run:

tests/sentry/workflow_engine/endpoints/test_organization_project_detector_index.py::OrganizationProjectDetectorIndexPostTest::test_incompatible_group_type

— log

[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/workflow_engine/endpoints/test_organization_project_detector_index.py:144: in test_incompatible_group_type
    response = self.get_error_response(
src/sentry/testutils/cases.py:663: in get_error_response
    assert_status_code(response, status_code)
src/sentry/testutils/asserts.py:46: in assert_status_code
    assert minimum <= response.status_code < maximum, response
E   AssertionError: <Response status_code=201, "application/json">
E   assert 400 <= 201
E    +  where 201 = <Response status_code=201, "application/json">.status_code

tests/sentry/api/serializers/test_rule.py::WorkflowRuleSerializerTest::test_every_filter — log

[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/api/serializers/test_rule.py:444: in test_every_filter
    self.assert_equal_serializers(issue_alert)
tests/sentry/api/serializers/test_rule.py:164: in assert_equal_serializers
    assert filter in workflow_filters
E   assert {'id': 'sentry.rules.filters.issue_type.IssueTypeFilter', 'name': "The issue's type is equal to ", 'value': 'error'} in [{'id': 'sentry.rules.filters.tagged_event.TaggedEventFilter', 'key': 'environment', 'match': 'is', 'name': "The event...sentry.rules.filters.issue_type.IssueTypeFilter', 'name': "The issue's type is equal to Error", 'value': 'error'}, ...]

tests/sentry/preprod/size_analysis/test_grouptype.py::PreprodSizeAnalysisDetectorQueryFilterTest::test_invalid_query_skips_evaluation_and_logs_exception

— log

[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/preprod/size_analysis/test_grouptype.py:526: in test_invalid_query_skips_evaluation_and_logs_exception
    mock_logger.exception.assert_called_once_with(
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:988: in assert_called_once_with
    raise AssertionError(msg)
E   AssertionError: Expected 'exception' to be called once. Called 0 times.

tests/sentry/workflow_engine/endpoints/test_validators.py::DetectorValidatorTest::test_validate_type_incompatible — log

[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/workflow_engine/endpoints/test_validators.py:303: in test_validate_type_incompatible
    assert not validator.is_valid()
.venv/lib/python3.13/site-packages/rest_framework/serializers.py:225: in is_valid
    self._validated_data = self.run_validation(self.initial_data)
.venv/lib/python3.13/site-packages/rest_framework/serializers.py:444: in run_validation
    value = self.to_internal_value(data)
.venv/lib/python3.13/site-packages/rest_framework/serializers.py:503: in to_internal_value
    validated_value = validate_method(validated_value)
src/sentry/workflow_engine/endpoints/validators/base/detector.py:128: in validate_type
    ds = get_detector_settings(type)
src/sentry/workflow_engine/types.py:470: in get_detector_settings
    f"DetectorType {group_type.detector_type!r} on {group_type.__name__} "
/opt/hostedtoolcache/Python/3.13.1/x64/lib/python3.13/unittest/mock.py:690: in __getattr__
    raise AttributeError(name)
E   AttributeError: __name__

saponifi3d

high level, i really like this approach and simplifying the DetectorSettings on the group; i also really like the way this means that we don't pull in grouptype information into the detector models.

starting to be more nit-picky, i think we might be over-loading the types file a bit. maybe we could pull some of that code out and organize it consistently with the other models / registries internally in the module (i'd also def be open to discussing how we're organizing that stuff, i mostly just would like it to be consistent in the workflow_engine module)

saponifi3d · 2026-04-14T16:43:20Z

src/sentry/incidents/detectorconfig.py

+        )
+
+
+detector_settings_registry.register(


🤔 - i really like how this pulls out the handlers and makes it a lot easier to access

saponifi3d · 2026-04-14T16:59:57Z

src/sentry/workflow_engine/models/detector.py

    )

    # maps to registry (sentry.issues.grouptype.registry) entries for GroupType.slug in sentry.issues.grouptype.GroupType
    type = models.CharField(max_length=200)


should this enforce the types now that there's a consistent list?

saponifi3d · 2026-04-14T17:00:03Z

src/sentry/workflow_engine/types.py

    event_types: list[SnubaQueryEventType.EventType]


+class DetectorType(StrEnum):


one of the goals for using the group types info was so that folks wouldn't need to modify the workflow_engine module to create a new detector. This is moves us towards having a list of detectors to maintain internally -- any thoughts on ways we could get the convenience of this, but the flexibility of being able to register detectors?

My other nit would be that on the other models we've nested the Type class in the parent;

class Detector(): class Type(): ...

should we be consistent? either moving everything into a nested type directory or move this to be nested in the detector?

saponifi3d · 2026-04-14T17:08:33Z

src/sentry/workflow_engine/types.py

    filter: Q | None = None
+
+
+class DetectorSettingsRegistry:


can we move this into the registries directory and use the consistent base class so the interactions are the same?

saponifi3d · 2026-04-14T17:09:35Z

src/sentry/workflow_engine/types.py

+detector_settings_registry = DetectorSettingsRegistry()
+
+
+def get_detector_settings(group_type: type[GroupType]) -> DetectorSettings | None:


🤔 thoughts on this just being a util instead of in types?

kcons added 4 commits April 13, 2026 15:53

ref(workflows): Break dependency chain

a56dc9f

decouple

e366ea8

grouptype.py

eafe68a

big move

75d5c75

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 13, 2026

sentry-warden bot reviewed Apr 14, 2026

View reviewed changes

saponifi3d reviewed Apr 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WIP: Big ole DetectorSettings refactor#112888

WIP: Big ole DetectorSettings refactor#112888
kcons wants to merge 4 commits intomasterfrom
kcons/layerfun

kcons commented Apr 13, 2026

Uh oh!

sentry-warden bot Apr 14, 2026

Uh oh!

sentry-warden bot Apr 14, 2026

Uh oh!

github-actions bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

saponifi3d left a comment

Uh oh!

saponifi3d Apr 14, 2026

Uh oh!

saponifi3d Apr 14, 2026

Uh oh!

saponifi3d Apr 14, 2026

Uh oh!

saponifi3d Apr 14, 2026

Uh oh!

saponifi3d Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		recovery_threshold = self.detector.config["recovery_threshold"]
		downtime_threshold = self.detector.config["downtime_threshold"]

-        recovery_threshold = self.detector.config["recovery_threshold"]
-        downtime_threshold = self.detector.config["downtime_threshold"]
+        from sentry.uptime.types import DEFAULT_DOWNTIME_THRESHOLD, DEFAULT_RECOVERY_THRESHOLD
+        recovery_threshold = self.detector.config.get("recovery_threshold", DEFAULT_RECOVERY_THRESHOLD)
+        downtime_threshold = self.detector.config.get("downtime_threshold", DEFAULT_DOWNTIME_THRESHOLD)
+        from sentry.uptime.types import DEFAULT_DOWNTIME_THRESHOLD
+        threshold = self.detector.config.get("downtime_threshold", DEFAULT_DOWNTIME_THRESHOLD)

		event_types: list[SnubaQueryEventType.EventType]


		class DetectorType(StrEnum):

		detector_settings_registry = DetectorSettingsRegistry()


		def get_detector_settings(group_type: type[GroupType]) -> DetectorSettings \| None:

		)


		detector_settings_registry.register(

Uh oh!

Conversation

kcons commented Apr 13, 2026

Uh oh!

sentry-warden bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

sentry-warden bot Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backend Test Failures

Uh oh!

saponifi3d left a comment

Choose a reason for hiding this comment

Uh oh!

saponifi3d Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

saponifi3d Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

saponifi3d Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

saponifi3d Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

saponifi3d Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 14, 2026 •

edited

Loading