Skip to content

feat(tracing-without-performance): Returned orphan errors with trace … #54103

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 3, 2023
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
42 changes: 37 additions & 5 deletions src/sentry/api/endpoints/organization_events_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ class TraceError(TypedDict):
project_slug: str
title: str
level: str
timestamp: str
event_type: str
generation: int


class TracePerformanceIssue(TypedDict):
Expand Down Expand Up @@ -450,6 +453,9 @@ def serialize_error(event: SnubaError) -> TraceError:
"project_slug": event["project"],
"title": event["title"],
"level": event["tags[level]"],
"timestamp": event["timestamp"],
"event_type": "error",
"generation": 0,
}

@staticmethod
Expand Down Expand Up @@ -545,7 +551,15 @@ def get(self, request: HttpRequest, organization: Organization, trace_id: str) -
)

return Response(
self.serialize(transactions, errors, roots, warning_extra, event_id, detailed)
self.serialize(
transactions,
errors,
roots,
warning_extra,
event_id,
detailed,
tracing_without_performance_enabled,
)
)


Expand Down Expand Up @@ -608,6 +622,7 @@ def serialize(
warning_extra: Dict[str, str],
event_id: Optional[str],
detailed: bool = False,
allow_orphan_errors: bool = False,
) -> Sequence[LightResponse]:
"""Because the light endpoint could potentially have gaps between root and event we return a flattened list"""
if event_id is None:
Expand Down Expand Up @@ -730,6 +745,7 @@ def serialize(
warning_extra: Dict[str, str],
event_id: Optional[str],
detailed: bool = False,
allow_orphan_errors: bool = False,
) -> Sequence[FullResponse]:
"""For the full event trace, we return the results as a graph instead of a flattened list

Expand All @@ -754,8 +770,8 @@ def serialize(
results_map[None].append(root_event)
to_check.append(root)

iteration = 0
with sentry_sdk.start_span(op="building.trace", description="full trace"):
iteration = 0
has_orphans = False
while parent_map or to_check:
if len(to_check) == 0:
Expand Down Expand Up @@ -855,6 +871,19 @@ def serialize(
)
break

# We are now left with orphan errors in the error_map,
# that we need to serialize and return with our results.
orphan_errors: List[TraceError] = []
if allow_orphan_errors and iteration <= MAX_TRACE_SIZE:
for errors in error_map.values():
for error in errors:
orphan_errors.append(self.serialize_error(error))
iteration += 1
if iteration > MAX_TRACE_SIZE:
break
if iteration > MAX_TRACE_SIZE:
break

root_traces: List[TraceEvent] = []
orphans: List[TraceEvent] = []
for index, result in enumerate(results_map.values()):
Expand All @@ -867,14 +896,17 @@ def serialize(
# We sort orphans and roots separately because we always want the root(s) as the first element(s)
root_traces.sort(key=child_sort_key)
orphans.sort(key=child_sort_key)
orphan_errors = sorted(orphan_errors, key=lambda k: k["timestamp"])

if len(orphans) > 0:
sentry_sdk.set_tag("discover.trace-view.contains-orphans", "yes")
logger.warning("discover.trace-view.contains-orphans", extra=warning_extra)

return [trace.full_dict(detailed) for trace in root_traces] + [
orphan.full_dict(detailed) for orphan in orphans
]
return (
[trace.full_dict(detailed) for trace in root_traces]
+ [orphan.full_dict(detailed) for orphan in orphans]
+ [orphan for orphan in orphan_errors]
)


@region_silo_endpoint
Expand Down
180 changes: 180 additions & 0 deletions tests/snuba/api/endpoints/test_organization_events_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,9 @@ def test_with_errors(self):
"project_slug": self.gen1_project.slug,
"level": "fatal",
"title": error.title,
"timestamp": error.timestamp,
"generation": 0,
"event_type": "error",
} in gen1_event["errors"]
assert {
"event_id": error1.event_id,
Expand All @@ -1118,8 +1121,182 @@ def test_with_errors(self):
"project_slug": self.gen1_project.slug,
"level": "warning",
"title": error1.title,
"timestamp": error1.timestamp,
"generation": 0,
"event_type": "error",
} in gen1_event["errors"]

def test_with_only_orphan_errors_with_same_span_ids(self):
span_id = uuid4().hex[:16]
start, end = self.get_start_end(10000)

# Error 1
error_data = load_data(
"javascript",
timestamp=end,
)
error_data["contexts"]["trace"] = {
"type": "trace",
"trace_id": self.trace_id,
"span_id": span_id,
}
error_data["level"] = "fatal"
error = self.store_event(error_data, project_id=self.project.id)

# Error 2 before after Error 1
error_data1 = load_data(
"javascript",
timestamp=start,
)
error_data1["level"] = "warning"
error_data1["contexts"]["trace"] = {
"type": "trace",
"trace_id": self.trace_id,
"span_id": span_id,
}
error1 = self.store_event(error_data1, project_id=self.project.id)

with self.feature(
[*self.FEATURES, "organizations:performance-tracing-without-performance"]
):
response = self.client.get(
self.url,
data={"project": -1},
format="json",
)
assert response.status_code == 200, response.content
assert len(response.data) == 2
# Sorting by timestamp puts Error1 after Error2 in the response
assert {
"event_id": error.event_id,
"issue_id": error.group_id,
"span": span_id,
"project_id": self.project.id,
"project_slug": self.project.slug,
"level": "fatal",
"title": error.title,
"timestamp": error.timestamp,
"generation": 0,
"event_type": "error",
} == response.data[1]
assert {
"event_id": error1.event_id,
"issue_id": error1.group_id,
"span": span_id,
"project_id": self.project.id,
"project_slug": self.project.slug,
"level": "warning",
"title": error1.title,
"timestamp": error1.timestamp,
"generation": 0,
"event_type": "error",
} == response.data[0]

def test_with_only_orphan_errors_with_different_span_ids(self):
start, _ = self.get_start_end(1000)
span_id = uuid4().hex[:16]
error_data = load_data(
"javascript",
timestamp=start,
)
error_data["contexts"]["trace"] = {
"type": "trace",
"trace_id": self.trace_id,
"span_id": span_id,
}
error_data["level"] = "fatal"
error = self.store_event(error_data, project_id=self.project.id)
error_data["level"] = "warning"
span_id1 = uuid4().hex[:16]
error_data["contexts"]["trace"] = {
"type": "trace",
"trace_id": self.trace_id,
"span_id": span_id1,
}
error1 = self.store_event(error_data, project_id=self.project.id)

with self.feature(
[*self.FEATURES, "organizations:performance-tracing-without-performance"]
):
response = self.client.get(
self.url,
data={"project": -1},
format="json",
)
assert response.status_code == 200, response.content
assert len(response.data) == 2
assert {
"event_id": error.event_id,
"issue_id": error.group_id,
"span": span_id,
"project_id": self.project.id,
"project_slug": self.project.slug,
"level": "fatal",
"title": error.title,
"timestamp": error.timestamp,
"generation": 0,
"event_type": "error",
} in response.data
assert {
"event_id": error1.event_id,
"issue_id": error1.group_id,
"span": span_id1,
"project_id": self.project.id,
"project_slug": self.project.slug,
"level": "warning",
"title": error1.title,
"timestamp": error1.timestamp,
"generation": 0,
"event_type": "error",
} in response.data

def test_with_mixup_of_orphan_errors_with_simple_trace_data(self):
self.load_trace()
start, _ = self.get_start_end(1000)
span_id = uuid4().hex[:16]
error_data = load_data(
"javascript",
timestamp=start,
)
error_data["contexts"]["trace"] = {
"type": "trace",
"trace_id": self.trace_id,
"span_id": span_id,
}
error_data["level"] = "fatal"
error = self.store_event(error_data, project_id=self.project.id)
error_data["level"] = "warning"
span_id1 = uuid4().hex[:16]
error_data["contexts"]["trace"] = {
"type": "trace",
"trace_id": self.trace_id,
"span_id": span_id1,
}

with self.feature(
[*self.FEATURES, "organizations:performance-tracing-without-performance"]
):
response = self.client.get(
self.url,
data={"project": -1},
format="json",
)
assert response.status_code == 200, response.content
assert len(response.data) == 2
self.assert_trace_data(response.data[0])
assert {
"event_id": error.event_id,
"issue_id": error.group_id,
"span": span_id,
"project_id": self.project.id,
"project_slug": self.project.slug,
"level": "fatal",
"title": error.title,
"timestamp": error.timestamp,
"generation": 0,
"event_type": "error",
} in response.data

def test_with_default(self):
self.load_trace()
start, _ = self.get_start_end(1000)
Expand All @@ -1143,6 +1320,9 @@ def test_with_default(self):
"project_slug": self.gen1_project.slug,
"level": "debug",
"title": "this is a log message",
"timestamp": default_event.timestamp,
"generation": 0,
"event_type": "error",
} in root_event["errors"]

def test_pruning_root(self):
Expand Down