Skip to content
Open
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
8 changes: 6 additions & 2 deletions sentry_sdk/_log_batcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,19 @@ def _to_transport_format(item: "Log") -> "Any":

res = {
"timestamp": int(item["time_unix_nano"]) / 1.0e9,
"trace_id": item.get("trace_id", "00000000-0000-0000-0000-000000000000"),
"span_id": item.get("span_id"),
"level": str(item["severity_text"]),
"body": str(item["body"]),
"attributes": {
k: serialize_attribute(v) for (k, v) in item["attributes"].items()
},
}

if item.get("trace_id") is not None:
res["trace_id"] = item["trace_id"]

if item.get("span_id") is not None:
res["span_id"] = item["span_id"]

return res

def _record_lost(self, item: "Log") -> None:
Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/_metrics_batcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class MetricsBatcher(Batcher["Metric"]):
def _to_transport_format(item: "Metric") -> "Any":
res = {
"timestamp": item["timestamp"],
"trace_id": item["trace_id"],
"name": item["name"],
"type": item["type"],
"value": item["value"],
Expand All @@ -30,6 +29,9 @@ def _to_transport_format(item: "Metric") -> "Any":
},
}

if item.get("trace_id") is not None:
res["trace_id"] = item["trace_id"]

if item.get("span_id") is not None:
res["span_id"] = item["span_id"]

Expand Down
12 changes: 5 additions & 7 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ def get_trace_context(self) -> "Dict[str, Any]":
if (
has_tracing_enabled(self.get_client().options)
and self._span is not None
and not isinstance(self._span, NoOpStreamedSpan)
and not isinstance(self._span, (NoOpStreamedSpan, NoOpSpan))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

generalized these instance checks just in case

):
return self._span._get_trace_context()

Expand Down Expand Up @@ -693,7 +693,7 @@ def iter_trace_propagation_headers(
if (
has_tracing_enabled(client.options)
and span is not None
and not isinstance(span, NoOpStreamedSpan)
and not isinstance(span, (NoOpStreamedSpan, NoOpSpan))
):
for header in span._iter_headers():
yield header
Expand Down Expand Up @@ -1859,17 +1859,15 @@ def apply_to_telemetry(self, telemetry: "Union[Log, Metric, StreamedSpan]") -> N
if not isinstance(telemetry, StreamedSpan):
trace_context = self.get_trace_context()
trace_id = trace_context.get("trace_id")
if telemetry.get("trace_id") is None:
telemetry["trace_id"] = (
trace_id or "00000000-0000-0000-0000-000000000000"
)
if telemetry.get("trace_id") is None and trace_id is not None:
telemetry["trace_id"] = trace_id
Comment thread
sl0thentr0py marked this conversation as resolved.

# span_id should only be populated if there's an active span. We can't
# use the trace_context here because it synthesizes a span_id if there
# isn't one
if telemetry.get("span_id") is None:
if self._span is not None and not isinstance(
self._span, NoOpStreamedSpan
self._span, (NoOpStreamedSpan, NoOpSpan)
):
telemetry["span_id"] = self._span.span_id
else:
Comment on lines +1863 to 1873

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The test test_logs_before_send_log will fail because it incorrectly asserts the presence of span_id in log records created without an active span.
Severity: MEDIUM

Suggested Fix

Update the assertion in the test_logs_before_send_log test to no longer expect span_id to be present in the log record's keys, aligning it with the new behavior where span_id is optional.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry_sdk/scope.py#L1859-L1873

Potential issue: The `apply_to_telemetry` method was modified to only add `span_id` to
log records when an active span is present. However, the test
`test_logs_before_send_log` was not updated to reflect this change. This test creates
logs without an active span but still contains an assertion that expects `span_id` to be
present in the log record's keys. As a result, the assertion will fail with an
`AssertionError` because the `span_id` key will be missing, causing a test regression.

Also affects:

  • tests/test_logs.py

Expand Down
3 changes: 1 addition & 2 deletions tests/integrations/logging/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,7 @@ def test_logger_with_all_attributes(sentry_init, capture_items):

logs = [item.payload for item in items]

assert "span_id" in logs[0]
assert logs[0]["span_id"] is None
assert "span_id" not in logs[0]

attributes = logs[0]["attributes"]

Expand Down
4 changes: 1 addition & 3 deletions tests/test_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def test_logs_no_span_id_without_active_span(sentry_init, capture_items):
get_client().flush()
logs = [item.payload for item in items]
assert logs[0]["trace_id"] is not None
assert logs[0]["span_id"] is None
assert "span_id" not in logs[0]


@minimum_python_37
Expand Down Expand Up @@ -473,7 +473,6 @@ def test_transport_format(sentry_init, capture_envelopes):
"level": "warn",
"timestamp": mock.ANY,
"trace_id": mock.ANY,
"span_id": mock.ANY,
"attributes": {
"sentry.environment": {
"type": "string",
Expand Down Expand Up @@ -552,7 +551,6 @@ def record_lost_event(reason, data_category=None, item=None, *, quantity=1):
"level": "info",
"timestamp": mock.ANY,
"trace_id": mock.ANY,
"span_id": mock.ANY,
"attributes": {
"sentry.environment": {
"type": "string",
Expand Down
Loading