Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
10 changes: 7 additions & 3 deletions sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -1848,9 +1848,13 @@
telemetry["trace_id"] = (
trace_id or "00000000-0000-0000-0000-000000000000"
)
span_id = trace_context.get("span_id")
if telemetry.get("span_id") is None and span_id:
telemetry["span_id"] = span_id

# 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
span = sentry_sdk.get_current_span()
if span:
telemetry["span_id"] = span.span_id

Check warning on line 1857 in sentry_sdk/scope.py

View check run for this annotation

@sentry/warden / warden: code-review

apply_to_telemetry uses global current span instead of self's span

The new code calls sentry_sdk.get_current_span() which internally invokes sentry_sdk.get_current_scope() rather than using self. The previous implementation derived span_id from self.get_trace_context(). If apply_to_telemetry is ever invoked on a scope other than the global current scope (e.g. an isolation/merged scope passed in by the client), the attached span_id will reflect the globally active span rather than the scope's own span. This is a subtle behavioral/side-effect change that may attach the wrong span_id to logs/metrics in nested or forked scope contexts.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated
Comment thread
sentrivana marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated

self._apply_scope_attributes_to_telemetry(telemetry)
self._apply_user_attributes_to_telemetry(telemetry)
Expand Down
2 changes: 1 addition & 1 deletion tests/integrations/loguru/test_loguru.py
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def test_logger_with_all_attributes(
logs = envelopes_to_logs(envelopes)

assert "span_id" in logs[0]
assert isinstance(logs[0]["span_id"], str)
assert logs[0]["span_id"] is None

attributes = logs[0]["attributes"]

Expand Down
20 changes: 19 additions & 1 deletion tests/test_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def envelopes_to_logs(envelopes: List[Envelope]) -> List[Log]:
"attributes": otel_attributes_to_dict(log_json["attributes"]),
"time_unix_nano": int(float(log_json["timestamp"]) * 1e9),
"trace_id": log_json["trace_id"],
"span_id": log_json["span_id"],
"span_id": log_json.get("span_id"),
}
res.append(log)
return res
Expand Down Expand Up @@ -324,6 +324,24 @@ def test_logs_tied_to_transactions(sentry_init, capture_envelopes):
assert logs[0]["span_id"] == trx.span_id


@minimum_python_37
def test_logs_no_span_id_without_active_span(sentry_init, capture_envelopes):
"""
Per the metrics spec, span_id is only attached when a span is active
when the telemetry is emitted. The propagation context's synthesized
span_id must not be used as a fallback.
"""
sentry_init(enable_logs=True)
envelopes = capture_envelopes()

sentry_sdk.logger.warning("This is a log without an active span")

get_client().flush()
logs = envelopes_to_logs(envelopes)
assert logs[0]["trace_id"] is not None
assert logs[0]["span_id"] is None


@minimum_python_37
def test_logs_tied_to_spans(sentry_init, capture_envelopes):
"""
Expand Down
6 changes: 4 additions & 2 deletions tests/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,10 @@ def test_metrics_tracing_without_performance(sentry_init, capture_envelopes):
propagation_context = isolation_scope._propagation_context
assert propagation_context is not None
assert metrics[0]["trace_id"] == propagation_context.trace_id
assert metrics[0]["span_id"] == propagation_context.span_id
# Per the metrics spec, span_id is only attached when a span is
# active when the metric is emitted. The propagation context's
# synthesized span_id must not be used as a fallback.
assert metrics[0]["span_id"] is None


def test_metrics_before_send(sentry_init, capture_envelopes):
Expand Down Expand Up @@ -294,7 +297,6 @@ def test_transport_format(sentry_init, capture_envelopes):
"value": 1,
"timestamp": mock.ANY,
"trace_id": mock.ANY,
"span_id": mock.ANY,
"attributes": {
"sentry.environment": {
"type": "string",
Expand Down
Loading