Skip to content

fix(crons): Don't drop invalid schema when writing checkin rows #97945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/sentry/monitors/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,11 @@ def update_config(self, config_payload, validated_config):
def get_validated_config(self):
try:
jsonschema.validate(self.config, MONITOR_CONFIG)
return self.config
except jsonschema.ValidationError:
logging.exception("Monitor: %s invalid config: %s", self.id, self.config)
logging.warning("Monitor: %s invalid config: %s", self.id, self.config, exc_info=True)
# We should always return the config here - just log an error if we detect that it doesn't
# match the schema
Comment on lines +375 to +376
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To some extent I wonder if this validation is even useful at this step... I guess it's good to know if somehow invalid configs are getting into the system so we can fix them

return self.config

def get_issue_alert_rule(self):
issue_alert_rule_id = self.config.get("alert_rule_id")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
SnubaQuery,
SnubaQueryEventType,
)
from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error
from sentry.workflow_engine.models import DataCondition, DataConditionGroup, DataSource, Detector
from sentry.workflow_engine.models.data_condition import Condition
from sentry.workflow_engine.registry import data_source_type_registry
Expand Down Expand Up @@ -320,7 +321,8 @@ def test_invalid_detector_type(self) -> None:
assert not validator.is_valid()
assert validator.errors.get("type") == [
ErrorDetail(
string="Unknown detector type 'invalid_type'. Must be one of: error", code="invalid"
string=get_unknown_detector_type_error("invalid_type", self.organization),
code="invalid",
)
]

Expand Down
28 changes: 18 additions & 10 deletions tests/sentry/monitors/test_models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
from datetime import datetime, timezone
from unittest import mock

Expand Down Expand Up @@ -254,26 +255,33 @@ def test_update_config(self) -> None:
}

def test_config_validator(self) -> None:
config = {
"checkin_margin": None,
"max_runtime": None,
"schedule": [1, "month"],
"schedule_type": ScheduleType.INTERVAL,
"alert_rule_id": 1,
}
monitor = Monitor.objects.create(
organization_id=self.organization.id,
project_id=self.project.id,
name="Unicron",
slug="unicron",
config={
"checkin_margin": None,
"max_runtime": None,
"schedule": [1, "month"],
"schedule_type": ScheduleType.INTERVAL,
"alert_rule_id": 1,
},
config=config,
)
validated_config = monitor.get_validated_config()
assert validated_config is not None
assert validated_config == config

# Check to make sure bad config fails validation
validated_config["bad_key"] = 100
monitor.config = validated_config
assert monitor.get_validated_config() is None

with self.assertLogs(logger="root", level=logging.WARNING) as cm:
bad_config = monitor.get_validated_config()
assert bad_config == validated_config
assert bad_config["bad_key"] == 100

assert len(cm.records) == 1
assert "invalid config" in cm.records[0].message


class CronMonitorDataSourceHandlerTest(TestCase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from sentry.uptime.grouptype import UptimeDomainCheckFailure
from sentry.uptime.types import DATA_SOURCE_UPTIME_SUBSCRIPTION
from sentry.workflow_engine.endpoints.organization_detector_index import convert_assignee_values
from sentry.workflow_engine.endpoints.validators.utils import get_unknown_detector_type_error
from sentry.workflow_engine.models import DataCondition, DataConditionGroup, DataSource, Detector
from sentry.workflow_engine.models.data_condition import Condition
from sentry.workflow_engine.models.detector_group import DetectorGroup
Expand Down Expand Up @@ -682,7 +683,7 @@ def test_invalid_group_type(self) -> None:
status_code=400,
)
assert response.data == {
"type": ["Unknown detector type 'invalid_type'. Must be one of: error"]
"type": [get_unknown_detector_type_error("invalid_type", self.organization)]
}

def test_incompatible_group_type(self) -> None:
Expand Down
Loading