Skip to content
Draft
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
49 changes: 49 additions & 0 deletions tests/tracing/test_http_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import pytest

import sentry_sdk
from sentry_sdk.traces import StreamedSpan
from sentry_sdk.tracing import Transaction
from sentry_sdk.tracing_utils import extract_sentrytrace_data

Expand All @@ -26,6 +28,32 @@
assert parts[2] == "1" if sampled is True else "0" # sampled


@pytest.mark.parametrize("traces_sample_rate", [1.0, 0.0, None])
def test_to_traceparent_span_streaming(sentry_init, traces_sample_rate):
sentry_init(
traces_sample_rate=traces_sample_rate,
_experiments={
"trace_lifecycle": "stream",
},
)

with sentry_sdk.traces.start_span(name="/interactions/other-dogs/new-dog") as span:
traceparent = sentry_sdk.get_traceparent()

parts = traceparent.split("-")
if traces_sample_rate is None:
propagation_context = (
sentry_sdk.get_current_scope().get_active_propagation_context()
)
assert parts[0] == propagation_context.trace_id # trace_id
assert parts[1] == propagation_context.span_id # parent_span_id
assert len(parts) == 2
else:
assert parts[0] == span.trace_id # trace_id
assert parts[1] == span.span_id # parent_span_id
assert parts[2] == "1" if traces_sample_rate == 1.0 else "0" # sampled

Check warning on line 54 in tests/tracing/test_http_headers.py

View check run for this annotation

@sentry/warden / warden: find-bugs

Operator precedence bug causes sampled-false assertion to always pass

Due to Python operator precedence, `assert parts[2] == "1" if traces_sample_rate == 1.0 else "0"` parses as `assert (parts[2] == "1") if traces_sample_rate == 1.0 else "0"` — when `traces_sample_rate == 0.0` it asserts the truthy string `"0"` instead of `parts[2] == "0"`, so the `sampled=false` case is never actually verified. Fix: `assert parts[2] == ("1" if traces_sample_rate == 1.0 else "0")`.

Check warning on line 55 in tests/tracing/test_http_headers.py

View check run for this annotation

@sentry/warden / warden: code-review

Assertion for `traces_sample_rate == 0.0` is always truthy and validates nothing

Due to Python operator precedence, `assert parts[2] == "1" if traces_sample_rate == 1.0 else "0"` is parsed as `assert (parts[2] == "1") if traces_sample_rate == 1.0 else "0"` — so when `traces_sample_rate == 0.0` the asserted value is the string `"0"`, which is always truthy and never checks the actual sampled flag. Use `assert parts[2] == ("1" if traces_sample_rate == 1.0 else "0")` instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Assertion for traces_sample_rate == 0.0 is always truthy and validates nothing

Due to Python operator precedence, assert parts[2] == "1" if traces_sample_rate == 1.0 else "0" is parsed as assert (parts[2] == "1") if traces_sample_rate == 1.0 else "0" — so when traces_sample_rate == 0.0 the asserted value is the string "0", which is always truthy and never checks the actual sampled flag. Use assert parts[2] == ("1" if traces_sample_rate == 1.0 else "0") instead.

Evidence
  • In test_to_traceparent_span_streaming the new assertion assert parts[2] == "1" if traces_sample_rate == 1.0 else "0" (line 54) parses as (parts[2] == "1") if traces_sample_rate == 1.0 else "0" because the conditional expression binds looser than ==.
  • With the parametrized traces_sample_rate == 0.0 case, the whole expression evaluates to the string literal "0", which is always truthy, so assert never validates the unsampled traceparent flag.
  • The parametrize decorator explicitly includes 0.0 to cover the unsampled path, but that path exercises no real assertion, silently passing regardless of parts[2].

Identified by Warden code-review · HRH-9YM


@pytest.mark.parametrize("sampling_decision", [True, False])
def test_sentrytrace_extraction(sampling_decision):
sentrytrace_header = "12312012123120121231201212312012-0415201309082013-{}".format(
Expand Down Expand Up @@ -75,3 +103,24 @@
assert (
headers["sentry-trace"] == "12312012123120121231201212312012-0415201309082013-0"
)

Check warning on line 106 in tests/tracing/test_http_headers.py

View check run for this annotation

@sentry/warden / warden: code-review

Test `test_iter_headers_span_streaming` uses `traces_sample_rate=0.0`, so `span._iter_headers()` raises `AttributeError` instead of exercising header generation

With `traces_sample_rate=0.0`, the sampling decision drops the span and `start_streamed_span` returns a `NoOpStreamedSpan` rather than a real `StreamedSpan`. The test then calls `span._iter_headers()` directly. `NoOpStreamedSpan` does not override `_iter_headers`, so the inherited `StreamedSpan._iter_headers` runs `if not self._segment`, but `NoOpStreamedSpan.__init__` never assigns the inherited `_segment` slot, raising `AttributeError`. The monkeypatched `StreamedSpan._to_traceparent` is never reached and the `headers["sentry-trace"]` assertion is never evaluated, so the test errors out instead of validating streaming header generation. Note the production path in `Scope.iter_headers` guards with `isinstance(span, (NoOpStreamedSpan, NoOpSpan))` before calling `_iter_headers`, so this only affects the test's direct call. Use `traces_sample_rate=1.0` to obtain a real `StreamedSpan`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test test_iter_headers_span_streaming uses traces_sample_rate=0.0, so span._iter_headers() raises AttributeError instead of exercising header generation

With traces_sample_rate=0.0, the sampling decision drops the span and start_streamed_span returns a NoOpStreamedSpan rather than a real StreamedSpan. The test then calls span._iter_headers() directly. NoOpStreamedSpan does not override _iter_headers, so the inherited StreamedSpan._iter_headers runs if not self._segment, but NoOpStreamedSpan.__init__ never assigns the inherited _segment slot, raising AttributeError. The monkeypatched StreamedSpan._to_traceparent is never reached and the headers["sentry-trace"] assertion is never evaluated, so the test errors out instead of validating streaming header generation. Note the production path in Scope.iter_headers guards with isinstance(span, (NoOpStreamedSpan, NoOpSpan)) before calling _iter_headers, so this only affects the test's direct call. Use traces_sample_rate=1.0 to obtain a real StreamedSpan.

Evidence
  • _make_sampling_decision short-circuits to sampled=False when sample_rate is 0.0; Scope.start_streamed_span (scope.py:1317-1321) then returns NoOpStreamedSpan(scope=self, unsampled_reason=outcome).
  • NoOpStreamedSpan.__init__ (traces.py:617) only sets _scope, _unsampled_reason, _finished; it never assigns the _segment slot declared on StreamedSpan.__slots__.
  • NoOpStreamedSpan does not override _iter_headers; the inherited StreamedSpan._iter_headers (traces.py:534-535) executes if not self._segment, raising AttributeError on the uninitialised slot.
  • Scope.iter_headers (scope.py:706-708) skips _iter_headers for NoOpStreamedSpan, so only this test's direct span._iter_headers() call at line 122 hits the error.

Identified by Warden code-review · P8Z-Q9L


def test_iter_headers_span_streaming(sentry_init, monkeypatch):
sentry_init(
traces_sample_rate=0.0,
_experiments={
"trace_lifecycle": "stream",
},
)
monkeypatch.setattr(
StreamedSpan,
"_to_traceparent",
mock.Mock(return_value="12312012123120121231201212312012-0415201309082013-0"),
)

with sentry_sdk.traces.start_span(name="/interactions/other-dogs/new-dog") as span:
headers = dict(span._iter_headers())
assert (
headers["sentry-trace"]
== "12312012123120121231201212312012-0415201309082013-0"
)
Loading