From 4b2ddfdc0b5f453fd08148b6f363228fd0904a96 Mon Sep 17 00:00:00 2001 From: Dan Fuller Date: Fri, 16 May 2025 13:22:52 -0700 Subject: [PATCH] fix(uptime): Fix bug with the uptime_checks dataset in the events endpoint A refactor broke this, and we had no tests to verify that it works so it was missed. Fixed and added a test --- src/sentry/snuba/rpc_dataset_common.py | 2 +- src/sentry/snuba/uptime_checks.py | 4 +- src/sentry/testutils/cases.py | 27 ++-- .../api/endpoints/test_organization_events.py | 118 +++++++++++++++++- 4 files changed, 140 insertions(+), 11 deletions(-) diff --git a/src/sentry/snuba/rpc_dataset_common.py b/src/sentry/snuba/rpc_dataset_common.py index a0f72f1df24ea7..503f49a26243fd 100644 --- a/src/sentry/snuba/rpc_dataset_common.py +++ b/src/sentry/snuba/rpc_dataset_common.py @@ -280,7 +280,7 @@ def run_table_query( rpc_response = snuba_rpc.table_rpc([rpc_request])[0] sentry_sdk.set_tag("query.storage_meta.tier", rpc_response.meta.downsampled_storage_meta.tier) - return process_table_response(rpc_response, table_request) + return process_table_response(rpc_response, table_request, debug=debug) def process_table_response( diff --git a/src/sentry/snuba/uptime_checks.py b/src/sentry/snuba/uptime_checks.py index b496a757989bf6..eea57fb5013645 100644 --- a/src/sentry/snuba/uptime_checks.py +++ b/src/sentry/snuba/uptime_checks.py @@ -49,6 +49,7 @@ def query( dataset: Dataset = Dataset.Discover, fallback_to_transactions: bool = False, query_source: QuerySource | None = None, + debug: bool = False, ) -> EventsResponse: return run_table_query( TableQuery( @@ -66,5 +67,6 @@ def query( use_aggregate_conditions=use_aggregate_conditions, ), ), - ) + ), + debug=debug, ) diff --git a/src/sentry/testutils/cases.py b/src/sentry/testutils/cases.py index 9d414947775c76..a0cc451505626c 100644 --- a/src/sentry/testutils/cases.py +++ b/src/sentry/testutils/cases.py @@ -2280,6 +2280,12 @@ def store_snuba_uptime_check( incident_status: IncidentStatus | None = None, scheduled_check_time: datetime | None = None, http_status: int | None | NotSet = NOT_SET, + actual_check_time: datetime | None = None, + duration_ms: int | None = None, + check_status_reason: CheckStatusReason | None = None, + region: str = "default", + environment: str = "production", + trace_id: UUID | None = None, ): if scheduled_check_time is None: scheduled_check_time = datetime.now() - timedelta(minutes=5) @@ -2287,12 +2293,17 @@ def store_snuba_uptime_check( incident_status = IncidentStatus.NO_INCIDENT if check_id is None: check_id = uuid.uuid4() + if trace_id is None: + trace_id = uuid.uuid4() - check_status_reason: CheckStatusReason | None = None - if check_status == "failure": + if check_status == "failure" and check_status_reason is None: check_status_reason = {"type": "failure", "description": "Mock failure"} - timestamp = scheduled_check_time + timedelta(seconds=1) + if not actual_check_time: + actual_check_time = scheduled_check_time + timedelta(seconds=1) + + if duration_ms is None: + duration_ms = random.randint(1, 1000) http_status = default_if_not_set( 200 if check_status == "success" else random.choice([408, 500, 502, 503, 504]), @@ -2304,16 +2315,16 @@ def store_snuba_uptime_check( "organization_id": self.organization.id, "project_id": self.project.id, "retention_days": 30, - "region": "default", - "environment": "production", + "region": region, + "environment": environment, "subscription_id": subscription_id, "guid": str(check_id), "scheduled_check_time_ms": int(scheduled_check_time.timestamp() * 1000), - "actual_check_time_ms": int(timestamp.timestamp() * 1000), - "duration_ms": random.randint(1, 1000), + "actual_check_time_ms": int(actual_check_time.timestamp() * 1000), + "duration_ms": duration_ms, "status": check_status, "status_reason": check_status_reason, - "trace_id": str(uuid.uuid4()), + "trace_id": str(trace_id), "incident_status": incident_status.value, "request_info": { "http_status_code": http_status, diff --git a/tests/snuba/api/endpoints/test_organization_events.py b/tests/snuba/api/endpoints/test_organization_events.py index a825885e3865d5..4e681e61dcb8d4 100644 --- a/tests/snuba/api/endpoints/test_organization_events.py +++ b/tests/snuba/api/endpoints/test_organization_events.py @@ -1,13 +1,16 @@ import math import uuid -from datetime import timedelta +from datetime import UTC, timedelta from typing import Any from unittest import mock import pytest +from dateutil import parser from django.test import override_settings from django.urls import reverse from django.utils import timezone +from rest_framework.response import Response +from sentry_kafka_schemas.schema_types.uptime_results_v1 import CheckStatus, CheckStatusReason from snuba_sdk.column import Column from snuba_sdk.function import Function @@ -35,11 +38,13 @@ ProfilesSnubaTestCase, SnubaTestCase, SpanTestCase, + UptimeCheckSnubaTestCase, ) from sentry.testutils.helpers import parse_link_header from sentry.testutils.helpers.datetime import before_now, freeze_time from sentry.testutils.helpers.discover import user_misery_formula from sentry.types.group import GroupSubStatus +from sentry.uptime.types import IncidentStatus from sentry.utils import json from sentry.utils.samples import load_data from tests.sentry.issues.test_utils import SearchIssueTestMixin @@ -6870,3 +6875,114 @@ def test_remapping(self): assert meta["fields"]["transaction.duration"] == "duration" assert meta["units"]["span.duration"] == "millisecond" assert meta["units"]["transaction.duration"] == "millisecond" + + +class OrganizationEventsUptimeDatasetEndpointTest( + OrganizationEventsEndpointTestBase, UptimeCheckSnubaTestCase +): + def coerce_response(self, response: Response) -> None: + for item in response.data["data"]: + for field in ("uptime_subscription_id", "uptime_check_id", "trace_id"): + if field in item: + item[field] = uuid.UUID(item[field]) + + for field in ("timestamp", "scheduled_check_time"): + if field in item: + item[field] = parser.parse(item[field]).replace(tzinfo=UTC) + + for field in ("duration_ms", "http_status_code"): + if field in item: + item[field] = int(item[field]) + + def test_basic(self): + subscription_id = uuid.uuid4().hex + check_id = uuid.uuid4() + self.store_snuba_uptime_check( + subscription_id=subscription_id, check_status="success", check_id=check_id + ) + query = { + "field": ["uptime_subscription_id", "uptime_check_id"], + "statsPeriod": "2h", + "query": "", + "dataset": "uptimeChecks", + "orderby": ["uptime_subscription_id"], + } + + response = self.do_request(query) + self.coerce_response(response) + assert response.status_code == 200, response.content + assert response.data["data"] == [ + { + "uptime_subscription_id": uuid.UUID(subscription_id), + "uptime_check_id": check_id, + } + ] + + def test_all_fields(self): + subscription_id = uuid.uuid4().hex + check_id = uuid.uuid4() + scheduled_check_time = before_now(minutes=5) + actual_check_time = before_now(minutes=2) + duration_ms = 100 + region = "us" + check_status: CheckStatus = "failure" + check_status_reason: CheckStatusReason = { + "type": "timeout", + "description": "Request timed out", + } + http_status = 200 + trace_id = uuid.uuid4() + environment = "test" + self.store_snuba_uptime_check( + environment=environment, + subscription_id=subscription_id, + check_status=check_status, + check_id=check_id, + incident_status=IncidentStatus.NO_INCIDENT, + scheduled_check_time=scheduled_check_time, + http_status=http_status, + actual_check_time=actual_check_time, + duration_ms=duration_ms, + region=region, + check_status_reason=check_status_reason, + trace_id=trace_id, + ) + + query = { + "field": [ + "environment", + "uptime_subscription_id", + "uptime_check_id", + "scheduled_check_time", + "timestamp", + "duration_ms", + "region", + "check_status", + "check_status_reason", + "http_status_code", + "trace_id", + ], + "statsPeriod": "1h", + "query": "", + "dataset": "uptimeChecks", + } + + response = self.do_request(query) + self.coerce_response(response) + assert response.status_code == 200, response.content + assert response.data["data"] == [ + { + "environment": environment, + "uptime_subscription_id": uuid.UUID(subscription_id), + "uptime_check_id": check_id, + "environment": environment, + "scheduled_check_time": scheduled_check_time.replace(microsecond=0), + "timestamp": actual_check_time, + "duration_ms": duration_ms, + "region": region, + "check_status": check_status, + "check_status_reason": check_status_reason["type"], + "http_status_code": http_status, + "trace_id": trace_id, + } + ]