Skip to content

Commit 7d3fe86

Browse files
wedamijaandrewshie-sentry
authored andcommitted
fix(uptime): Fix bug with the uptime_checks dataset in the events endpoint (#91824)
A refactor broke this, and we had no tests to verify that it works so it was missed. Fixed and added a test
1 parent 0856edb commit 7d3fe86

File tree

4 files changed

+140
-11
lines changed

4 files changed

+140
-11
lines changed

src/sentry/snuba/rpc_dataset_common.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ def run_table_query(
280280
rpc_response = snuba_rpc.table_rpc([rpc_request])[0]
281281
sentry_sdk.set_tag("query.storage_meta.tier", rpc_response.meta.downsampled_storage_meta.tier)
282282

283-
return process_table_response(rpc_response, table_request)
283+
return process_table_response(rpc_response, table_request, debug=debug)
284284

285285

286286
def process_table_response(

src/sentry/snuba/uptime_checks.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ def query(
4949
dataset: Dataset = Dataset.Discover,
5050
fallback_to_transactions: bool = False,
5151
query_source: QuerySource | None = None,
52+
debug: bool = False,
5253
) -> EventsResponse:
5354
return run_table_query(
5455
TableQuery(
@@ -66,5 +67,6 @@ def query(
6667
use_aggregate_conditions=use_aggregate_conditions,
6768
),
6869
),
69-
)
70+
),
71+
debug=debug,
7072
)

src/sentry/testutils/cases.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2280,19 +2280,30 @@ def store_snuba_uptime_check(
22802280
incident_status: IncidentStatus | None = None,
22812281
scheduled_check_time: datetime | None = None,
22822282
http_status: int | None | NotSet = NOT_SET,
2283+
actual_check_time: datetime | None = None,
2284+
duration_ms: int | None = None,
2285+
check_status_reason: CheckStatusReason | None = None,
2286+
region: str = "default",
2287+
environment: str = "production",
2288+
trace_id: UUID | None = None,
22832289
):
22842290
if scheduled_check_time is None:
22852291
scheduled_check_time = datetime.now() - timedelta(minutes=5)
22862292
if incident_status is None:
22872293
incident_status = IncidentStatus.NO_INCIDENT
22882294
if check_id is None:
22892295
check_id = uuid.uuid4()
2296+
if trace_id is None:
2297+
trace_id = uuid.uuid4()
22902298

2291-
check_status_reason: CheckStatusReason | None = None
2292-
if check_status == "failure":
2299+
if check_status == "failure" and check_status_reason is None:
22932300
check_status_reason = {"type": "failure", "description": "Mock failure"}
22942301

2295-
timestamp = scheduled_check_time + timedelta(seconds=1)
2302+
if not actual_check_time:
2303+
actual_check_time = scheduled_check_time + timedelta(seconds=1)
2304+
2305+
if duration_ms is None:
2306+
duration_ms = random.randint(1, 1000)
22962307

22972308
http_status = default_if_not_set(
22982309
200 if check_status == "success" else random.choice([408, 500, 502, 503, 504]),
@@ -2304,16 +2315,16 @@ def store_snuba_uptime_check(
23042315
"organization_id": self.organization.id,
23052316
"project_id": self.project.id,
23062317
"retention_days": 30,
2307-
"region": "default",
2308-
"environment": "production",
2318+
"region": region,
2319+
"environment": environment,
23092320
"subscription_id": subscription_id,
23102321
"guid": str(check_id),
23112322
"scheduled_check_time_ms": int(scheduled_check_time.timestamp() * 1000),
2312-
"actual_check_time_ms": int(timestamp.timestamp() * 1000),
2313-
"duration_ms": random.randint(1, 1000),
2323+
"actual_check_time_ms": int(actual_check_time.timestamp() * 1000),
2324+
"duration_ms": duration_ms,
23142325
"status": check_status,
23152326
"status_reason": check_status_reason,
2316-
"trace_id": str(uuid.uuid4()),
2327+
"trace_id": str(trace_id),
23172328
"incident_status": incident_status.value,
23182329
"request_info": {
23192330
"http_status_code": http_status,

tests/snuba/api/endpoints/test_organization_events.py

Lines changed: 117 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import math
22
import uuid
3-
from datetime import timedelta
3+
from datetime import UTC, timedelta
44
from typing import Any
55
from unittest import mock
66

77
import pytest
8+
from dateutil import parser
89
from django.test import override_settings
910
from django.urls import reverse
1011
from django.utils import timezone
12+
from rest_framework.response import Response
13+
from sentry_kafka_schemas.schema_types.uptime_results_v1 import CheckStatus, CheckStatusReason
1114
from snuba_sdk.column import Column
1215
from snuba_sdk.function import Function
1316

@@ -35,11 +38,13 @@
3538
ProfilesSnubaTestCase,
3639
SnubaTestCase,
3740
SpanTestCase,
41+
UptimeCheckSnubaTestCase,
3842
)
3943
from sentry.testutils.helpers import parse_link_header
4044
from sentry.testutils.helpers.datetime import before_now, freeze_time
4145
from sentry.testutils.helpers.discover import user_misery_formula
4246
from sentry.types.group import GroupSubStatus
47+
from sentry.uptime.types import IncidentStatus
4348
from sentry.utils import json
4449
from sentry.utils.samples import load_data
4550
from tests.sentry.issues.test_utils import SearchIssueTestMixin
@@ -6870,3 +6875,114 @@ def test_remapping(self):
68706875
assert meta["fields"]["transaction.duration"] == "duration"
68716876
assert meta["units"]["span.duration"] == "millisecond"
68726877
assert meta["units"]["transaction.duration"] == "millisecond"
6878+
6879+
6880+
class OrganizationEventsUptimeDatasetEndpointTest(
6881+
OrganizationEventsEndpointTestBase, UptimeCheckSnubaTestCase
6882+
):
6883+
def coerce_response(self, response: Response) -> None:
6884+
for item in response.data["data"]:
6885+
for field in ("uptime_subscription_id", "uptime_check_id", "trace_id"):
6886+
if field in item:
6887+
item[field] = uuid.UUID(item[field])
6888+
6889+
for field in ("timestamp", "scheduled_check_time"):
6890+
if field in item:
6891+
item[field] = parser.parse(item[field]).replace(tzinfo=UTC)
6892+
6893+
for field in ("duration_ms", "http_status_code"):
6894+
if field in item:
6895+
item[field] = int(item[field])
6896+
6897+
def test_basic(self):
6898+
subscription_id = uuid.uuid4().hex
6899+
check_id = uuid.uuid4()
6900+
self.store_snuba_uptime_check(
6901+
subscription_id=subscription_id, check_status="success", check_id=check_id
6902+
)
6903+
query = {
6904+
"field": ["uptime_subscription_id", "uptime_check_id"],
6905+
"statsPeriod": "2h",
6906+
"query": "",
6907+
"dataset": "uptimeChecks",
6908+
"orderby": ["uptime_subscription_id"],
6909+
}
6910+
6911+
response = self.do_request(query)
6912+
self.coerce_response(response)
6913+
assert response.status_code == 200, response.content
6914+
assert response.data["data"] == [
6915+
{
6916+
"uptime_subscription_id": uuid.UUID(subscription_id),
6917+
"uptime_check_id": check_id,
6918+
}
6919+
]
6920+
6921+
def test_all_fields(self):
6922+
subscription_id = uuid.uuid4().hex
6923+
check_id = uuid.uuid4()
6924+
scheduled_check_time = before_now(minutes=5)
6925+
actual_check_time = before_now(minutes=2)
6926+
duration_ms = 100
6927+
region = "us"
6928+
check_status: CheckStatus = "failure"
6929+
check_status_reason: CheckStatusReason = {
6930+
"type": "timeout",
6931+
"description": "Request timed out",
6932+
}
6933+
http_status = 200
6934+
trace_id = uuid.uuid4()
6935+
environment = "test"
6936+
self.store_snuba_uptime_check(
6937+
environment=environment,
6938+
subscription_id=subscription_id,
6939+
check_status=check_status,
6940+
check_id=check_id,
6941+
incident_status=IncidentStatus.NO_INCIDENT,
6942+
scheduled_check_time=scheduled_check_time,
6943+
http_status=http_status,
6944+
actual_check_time=actual_check_time,
6945+
duration_ms=duration_ms,
6946+
region=region,
6947+
check_status_reason=check_status_reason,
6948+
trace_id=trace_id,
6949+
)
6950+
6951+
query = {
6952+
"field": [
6953+
"environment",
6954+
"uptime_subscription_id",
6955+
"uptime_check_id",
6956+
"scheduled_check_time",
6957+
"timestamp",
6958+
"duration_ms",
6959+
"region",
6960+
"check_status",
6961+
"check_status_reason",
6962+
"http_status_code",
6963+
"trace_id",
6964+
],
6965+
"statsPeriod": "1h",
6966+
"query": "",
6967+
"dataset": "uptimeChecks",
6968+
}
6969+
6970+
response = self.do_request(query)
6971+
self.coerce_response(response)
6972+
assert response.status_code == 200, response.content
6973+
assert response.data["data"] == [
6974+
{
6975+
"environment": environment,
6976+
"uptime_subscription_id": uuid.UUID(subscription_id),
6977+
"uptime_check_id": check_id,
6978+
"environment": environment,
6979+
"scheduled_check_time": scheduled_check_time.replace(microsecond=0),
6980+
"timestamp": actual_check_time,
6981+
"duration_ms": duration_ms,
6982+
"region": region,
6983+
"check_status": check_status,
6984+
"check_status_reason": check_status_reason["type"],
6985+
"http_status_code": http_status,
6986+
"trace_id": trace_id,
6987+
}
6988+
]

0 commit comments

Comments
 (0)