Skip to content

Commit 1ea07c2

Browse files
authored
fix(uptime): Fix value we return for event_id in the eap endpoints (#97446)
This should be a unique id per item, and should also be the `sentry.item_id` <!-- Describe your PR here. -->
1 parent 1f6b5dc commit 1ea07c2

File tree

6 files changed

+54
-84
lines changed

6 files changed

+54
-84
lines changed

src/sentry/search/eap/uptime_results/attributes.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,21 @@
66
column.public_alias: column
77
for column in COMMON_COLUMNS
88
+ [
9+
ResolvedAttribute(
10+
public_alias="id",
11+
internal_name="sentry.item_id",
12+
search_type="string",
13+
),
914
ResolvedAttribute(
1015
public_alias="trace",
1116
internal_name="sentry.trace_id",
1217
search_type="string",
1318
),
19+
ResolvedAttribute(
20+
public_alias="span_id",
21+
internal_name="span_id",
22+
search_type="string",
23+
),
1424
ResolvedAttribute(
1525
public_alias="environment",
1626
internal_name="environment",

src/sentry/snuba/trace.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,6 @@ def _serialize_columnar_uptime_item(
354354
columns_by_name = {col.internal_name: col for col in UPTIME_ATTRIBUTE_DEFINITIONS.values()}
355355
common_column_names = {col.internal_name for col in COMMON_COLUMNS}
356356

357-
guid = row_dict["guid"].val_str
358357
trace_id = row_dict["sentry.trace_id"].val_str
359358
check_status = row_dict["check_status"].val_str
360359
http_status_code = (
@@ -368,8 +367,10 @@ def _serialize_columnar_uptime_item(
368367
project_id = row_dict["sentry.project_id"].val_int
369368
project_slug = project_slugs[project_id]
370369

370+
item_id_str = row_dict["sentry.item_id"].val_str
371+
371372
span = {
372-
"event_id": guid,
373+
"event_id": item_id_str,
373374
"project_id": project_id,
374375
"project_slug": project_slug,
375376
"transaction_id": trace_id,

src/sentry/uptime/consumers/eap_converter.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"""
99

1010
import logging
11+
import uuid
1112
from collections.abc import MutableMapping
1213

1314
from google.protobuf.timestamp_pb2 import Timestamp
@@ -25,6 +26,8 @@
2526

2627
logger = logging.getLogger(__name__)
2728

29+
UPTIME_NAMESPACE = uuid.UUID("f8d7a4e2-5b3c-4a9d-8e1f-3c2b1a0d9f8e")
30+
2831

2932
def _anyvalue(value: bool | str | int | float) -> AnyValue:
3033
if isinstance(value, bool):
@@ -98,6 +101,7 @@ def convert_uptime_request_to_trace_item(
98101
attributes["check_id"] = _anyvalue(result["guid"])
99102
attributes["request_sequence"] = _anyvalue(request_sequence)
100103
attributes["incident_status"] = _anyvalue(incident_status.value)
104+
attributes["span_id"] = _anyvalue(result["span_id"])
101105

102106
if request_info is not None:
103107
attributes["request_type"] = _anyvalue(request_info["request_type"])
@@ -162,11 +166,11 @@ def convert_uptime_result_to_trace_items(
162166

163167
for sequence, request_info in enumerate(request_info_list):
164168
if sequence == 0:
165-
# First request uses the span_id directly
166-
item_id = result["span_id"].encode("utf-8")[:16].ljust(16, b"\x00")
169+
name = result["span_id"]
167170
else:
168-
request_id = f"{result['span_id']}_req_{sequence}"
169-
item_id = request_id.encode("utf-8")[:16].ljust(16, b"\x00")
171+
name = f"{result['span_id']}_req_{sequence}"
172+
173+
item_id = int(uuid.uuid5(UPTIME_NAMESPACE, name).hex, 16).to_bytes(16, "little")
170174

171175
request_item = convert_uptime_request_to_trace_item(
172176
project, result, request_info, sequence, item_id, incident_status

tests/sentry/uptime/endpoints/test_base.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def create_eap_uptime_result(
5050
response_body_size_bytes=None,
5151
status_reason_type=None,
5252
status_reason_description=None,
53+
span_id=None,
5354
) -> TraceItem:
5455
if organization is None:
5556
organization = self.organization
@@ -63,6 +64,8 @@ def create_eap_uptime_result(
6364
guid = uuid4().hex
6465
if subscription_id is None:
6566
subscription_id = f"sub-{uuid4().hex[:8]}"
67+
if span_id is None:
68+
span_id = uuid4().hex[:16]
6669

6770
attributes_data = {
6871
"guid": guid,
@@ -75,6 +78,7 @@ def create_eap_uptime_result(
7578
"request_sequence": request_sequence,
7679
"check_duration_us": check_duration_us,
7780
"request_duration_us": request_duration_us,
81+
"span_id": span_id,
7882
}
7983

8084
if check_id is not None:
@@ -143,3 +147,8 @@ def store_uptime_results(self, uptime_results):
143147
files=files,
144148
)
145149
assert response.status_code == 200
150+
151+
for result in uptime_results:
152+
# Reverse the ids here since these are stored in little endian in Clickhouse
153+
# and end up reversed.
154+
result.item_id = result.item_id[::-1]

tests/snuba/api/endpoints/test_organization_events_uptime_results.py

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def setUp(self):
1919
}
2020

2121
def build_expected_result(self, **kwargs):
22-
return {"id": None, "project.name": None, **kwargs}
22+
return {"project.name": None, **kwargs}
2323

2424
@pytest.mark.querybuilder
2525
def test_simple_uptime_query(self) -> None:
@@ -55,10 +55,16 @@ def test_simple_uptime_query(self) -> None:
5555

5656
assert data == [
5757
self.build_expected_result(
58-
check_status="success", http_status_code=200, region="us-east-1"
58+
id=results[0].item_id.hex(),
59+
check_status="success",
60+
http_status_code=200,
61+
region="us-east-1",
5962
),
6063
self.build_expected_result(
61-
check_status="failure", http_status_code=500, region="us-west-2"
64+
id=results[1].item_id.hex(),
65+
check_status="failure",
66+
http_status_code=500,
67+
region="us-west-2",
6268
),
6369
]
6470

@@ -97,8 +103,12 @@ def test_status_filter_query(self) -> None:
97103
data = response.data["data"]
98104

99105
assert data == [
100-
self.build_expected_result(check_status="success", http_status_code=200),
101-
self.build_expected_result(check_status="success", http_status_code=201),
106+
self.build_expected_result(
107+
id=results[0].item_id.hex(), check_status="success", http_status_code=200
108+
),
109+
self.build_expected_result(
110+
id=results[2].item_id.hex(), check_status="success", http_status_code=201
111+
),
102112
]
103113

104114
@pytest.mark.querybuilder
@@ -144,13 +154,15 @@ def test_timing_fields_query(self) -> None:
144154

145155
assert data == [
146156
self.build_expected_result(
157+
id=results[0].item_id.hex(),
147158
check_status="success",
148159
check_duration_us=150000,
149160
request_duration_us=125000,
150161
dns_lookup_duration_us=25000,
151162
tcp_connection_duration_us=15000,
152163
),
153164
self.build_expected_result(
165+
id=results[1].item_id.hex(),
154166
check_status="failure",
155167
check_duration_us=30000000,
156168
request_duration_us=30000000,
@@ -201,6 +213,7 @@ def test_cross_level_filter_query(self) -> None:
201213

202214
assert data == [
203215
self.build_expected_result(
216+
id=results[1].item_id.hex(),
204217
check_status="failure",
205218
http_status_code=504,
206219
dns_lookup_duration_us=150000,
@@ -259,6 +272,7 @@ def test_redirect_sequence_query(self) -> None:
259272

260273
assert data == [
261274
self.build_expected_result(
275+
id=results[1].item_id.hex(),
262276
check_id=check_id,
263277
request_sequence=1,
264278
http_status_code=200,
@@ -311,6 +325,7 @@ def test_region_and_status_combination(self) -> None:
311325

312326
assert data == [
313327
self.build_expected_result(
328+
id=results[1].item_id.hex(),
314329
check_status="failure",
315330
region="us-east-1",
316331
http_status_code=500,

tests/snuba/api/endpoints/test_organization_trace.py

Lines changed: 4 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ def setUp(self):
3434
def test_basic_uptime_item_serialization(self):
3535
"""Test basic serialization with all required fields."""
3636
row_dict = {
37+
"sentry.item_id": ProtoAttributeValue(val_str="check-123"),
3738
"sentry.project_id": ProtoAttributeValue(val_int=1),
3839
"guid": ProtoAttributeValue(val_str="check-123"),
3940
"sentry.trace_id": ProtoAttributeValue(val_str="a" * 32),
@@ -78,6 +79,7 @@ def test_basic_uptime_item_serialization(self):
7879
def test_redirect_chain_serialization(self):
7980
"""Test serialization of redirect chain with different URLs."""
8081
row_dict = {
82+
"sentry.item_id": ProtoAttributeValue(val_str="check-789"),
8183
"sentry.project_id": ProtoAttributeValue(val_int=1),
8284
"guid": ProtoAttributeValue(val_str="check-789"),
8385
"sentry.trace_id": ProtoAttributeValue(val_str="b" * 32),
@@ -101,6 +103,7 @@ def test_redirect_chain_serialization(self):
101103
def test_null_and_missing_fields(self):
102104
"""Test handling of null and missing optional fields."""
103105
row_dict = {
106+
"sentry.item_id": ProtoAttributeValue(val_str="check-null"),
104107
"sentry.project_id": ProtoAttributeValue(val_int=1),
105108
"guid": ProtoAttributeValue(val_str="check-null"),
106109
"sentry.trace_id": ProtoAttributeValue(val_str="c" * 32),
@@ -514,6 +517,7 @@ def _trace_item_to_api_span(self, trace_item: TraceItem, children=None) -> dict:
514517
elif attr_value.HasField("bool_value"):
515518
row_dict[attr_name] = ProtoAttributeValue(val_bool=attr_value.bool_value)
516519

520+
row_dict["sentry.item_id"] = ProtoAttributeValue(val_str=trace_item.item_id.hex())
517521
row_dict["sentry.project_id"] = ProtoAttributeValue(val_int=trace_item.project_id)
518522
row_dict["sentry.organization_id"] = ProtoAttributeValue(val_int=trace_item.organization_id)
519523
row_dict["sentry.trace_id"] = ProtoAttributeValue(val_str=trace_item.trace_id)
@@ -702,76 +706,3 @@ def test_uptime_root_tree_without_orphans(self):
702706
data = response.data
703707

704708
self.assert_expected_results(data, [uptime_result], expected_children_ids=["root"])
705-
706-
def test_uptime_root_tree_multiple_checks(self):
707-
"""Test handling of multiple uptime checks with only the last one becoming parent"""
708-
self.load_trace(is_eap=True)
709-
710-
self.create_event(
711-
trace_id=self.trace_id,
712-
transaction="/transaction/orphan",
713-
spans=[],
714-
project_id=self.project.id,
715-
parent_span_id=uuid4().hex[:16],
716-
milliseconds=500,
717-
is_eap=True,
718-
)
719-
check1_result = self._create_uptime_result_with_original_url(
720-
organization=self.organization,
721-
project=self.project,
722-
trace_id=self.trace_id,
723-
guid="check-111",
724-
check_status="success",
725-
http_status_code=200,
726-
request_sequence=0,
727-
request_url="https://first.com",
728-
scheduled_check_time=self.day_ago,
729-
check_duration_us=100000,
730-
)
731-
732-
check2_redirect = self._create_uptime_result_with_original_url(
733-
organization=self.organization,
734-
project=self.project,
735-
trace_id=self.trace_id,
736-
guid="check-222",
737-
check_status="success",
738-
http_status_code=301,
739-
request_sequence=0,
740-
request_url="https://second.com",
741-
scheduled_check_time=self.day_ago,
742-
check_duration_us=200000,
743-
)
744-
745-
check2_final = self._create_uptime_result_with_original_url(
746-
organization=self.organization,
747-
project=self.project,
748-
trace_id=self.trace_id,
749-
guid="check-222",
750-
check_status="success",
751-
http_status_code=200,
752-
request_sequence=1,
753-
request_url="https://www.second.com",
754-
scheduled_check_time=self.day_ago,
755-
check_duration_us=300000,
756-
)
757-
758-
features = self.FEATURES + [
759-
"organizations:uptime-eap-enabled",
760-
"organizations:uptime-eap-uptime-results-query",
761-
]
762-
763-
self.store_uptime_results([check1_result, check2_redirect, check2_final])
764-
765-
with self.feature(features):
766-
response = self.client_get(
767-
data={"timestamp": self.day_ago, "include_uptime": "1"},
768-
)
769-
770-
assert response.status_code == 200, response.content
771-
data = response.data
772-
773-
self.assert_expected_results(
774-
data,
775-
[check1_result, check2_redirect, check2_final],
776-
expected_children_ids=["/transaction/orphan", "root"],
777-
)

0 commit comments

Comments
 (0)