Skip to content
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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ dependencies = [
# [end] jsonschema format validators
"sentry-arroyo>=2.29.6",
"sentry-forked-email-reply-parser>=0.5.12.post1",
"sentry-kafka-schemas>=2.1.6",
"sentry-kafka-schemas>=2.1.7",
"sentry-ophio>=1.1.3",
"sentry-protos>=0.4.1",
"sentry-redis-tools>=0.5.0",
Expand Down
16 changes: 16 additions & 0 deletions src/sentry/uptime/consumers/results_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from sentry_kafka_schemas.codecs import Codec
from sentry_kafka_schemas.schema_types.snuba_uptime_results_v1 import SnubaUptimeResult
from sentry_kafka_schemas.schema_types.uptime_results_v1 import (
CHECKSTATUS_DISALLOWED_BY_ROBOTS,
CHECKSTATUS_MISSED_WINDOW,
CheckResult,
)
Expand All @@ -34,6 +35,7 @@
)
from sentry.uptime.subscriptions.subscriptions import (
check_and_update_regions,
disable_uptime_detector,
remove_uptime_subscription_if_unused,
)
from sentry.uptime.subscriptions.tasks import (
Expand Down Expand Up @@ -296,6 +298,20 @@ def handle_result(self, subscription: UptimeSubscription | None, result: CheckRe
}
subscription_regions = load_regions_for_uptime_subscription(subscription.id)

if result["status"] == CHECKSTATUS_DISALLOWED_BY_ROBOTS:
try:
detector = get_detector(subscription)
logger.info("disallowed_by_robots", extra=result)
Copy link
Member

Choose a reason for hiding this comment

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

Might also be worth including this as a metrics.incr like we have elsewhere, for example:

            metrics.incr(
                "uptime.result_processor.subscription_not_found",
                sample_rate=1.0,
                tags={"uptime_region": result.get("region", "default")},
            )

It'd be good to have something like that here too

metrics.incr(
"uptime.result_processor.disallowed_by_robots",
sample_rate=1.0,
tags={"uptime_region": result.get("region", "default")},
)
disable_uptime_detector(detector)
except Exception as e:
logger.exception("disallowed_by_robots.error", extra={"error": e, "result": result})
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Improper Exception Handling Causes Cleanup Failures

The new CHECKSTATUS_DISALLOWED_BY_ROBOTS handling catches Detector.DoesNotExist with a broad except Exception. This prevents proper cleanup of orphaned subscriptions and logs an expected condition as a generic error.

Fix in Cursor Fix in Web


# Discard shadow mode region results
if is_shadow_region_result(result, subscription_regions):
metrics.incr(
Expand Down
26 changes: 26 additions & 0 deletions tests/sentry/uptime/consumers/test_results_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from arroyo.types import BrokerValue, Partition, Topic
from django.test import override_settings
from sentry_kafka_schemas.schema_types.uptime_results_v1 import (
CHECKSTATUS_DISALLOWED_BY_ROBOTS,
CHECKSTATUS_FAILURE,
CHECKSTATUS_MISSED_WINDOW,
CHECKSTATUS_SUCCESS,
Expand Down Expand Up @@ -523,6 +524,31 @@ def test_missed(self) -> None:
with pytest.raises(Group.DoesNotExist):
Group.objects.get(grouphash__hash=hashed_fingerprint)

@override_options({"uptime.snuba_uptime_results.enabled": False})
def test_disallowed(self) -> None:
features = [
"organizations:uptime",
"organizations:uptime-create-issues",
]
result = self.create_uptime_result(
self.subscription.subscription_id, status=CHECKSTATUS_DISALLOWED_BY_ROBOTS
)
with (
mock.patch("sentry.uptime.consumers.results_consumer.logger") as logger,
self.feature(features),
):
assert self.detector.enabled

self.send_result(result)

logger.info.assert_any_call(
"disallowed_by_robots",
extra={**result},
)

self.detector.refresh_from_db()
assert not self.detector.enabled

def test_onboarding_failure(self) -> None:
features = [
"organizations:uptime",
Expand Down
6 changes: 3 additions & 3 deletions uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading