Skip to content

Commit

Permalink
fix(tracing): Only propagate headers from spans within transactions
Browse files Browse the repository at this point in the history
This change ensures that we only propagate trace headers from spans that are within a transaction. This fixes a bug where any child transactions of a span created outside a transaction are missing a dynamic sampling context and are part of a trace missing a root transaction (because the root is the span).

Also, remove/modify tests that were asserting the old behavior.

Fixes #3068
  • Loading branch information
szokeasaurusrex committed May 15, 2024
1 parent a02eb9c commit 94a6c2a
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 70 deletions.
14 changes: 10 additions & 4 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,18 @@ def iter_headers(self):
If the span's containing transaction doesn't yet have a ``baggage`` value,
this will cause one to be generated and stored.
"""
if not self.containing_transaction:
# Do not propagate headers if there is no containing transaction. Otherwise, this
# span ends up being the root span of a new trace, and since it does not get sent
# to Sentry, the trace will be missing a root transaction. The dynamic sampling
# context will also be missing, breaking dynamic sampling & traces.
return

yield SENTRY_TRACE_HEADER_NAME, self.to_traceparent()

if self.containing_transaction:
baggage = self.containing_transaction.get_baggage().serialize()
if baggage:
yield BAGGAGE_HEADER_NAME, baggage
baggage = self.containing_transaction.get_baggage().serialize()
if baggage:
yield BAGGAGE_HEADER_NAME, baggage

@classmethod
def from_traceparent(
Expand Down
8 changes: 6 additions & 2 deletions tests/integrations/aiohttp/test_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,13 +404,17 @@ async def hello(request):
# The aiohttp_client is instrumented so will generate the sentry-trace header and add request.
# Get the sentry-trace header from the request so we can later compare with transaction events.
client = await aiohttp_client(app)
resp = await client.get("/")
with start_transaction():
# Headers are only added to the span if there is an active transaction
resp = await client.get("/")

sentry_trace_header = resp.request_info.headers.get("sentry-trace")
trace_id = sentry_trace_header.split("-")[0]

assert resp.status == 500

msg_event, error_event, transaction_event = events
# Last item is the custom transaction event wrapping `client.get("/")`
msg_event, error_event, transaction_event, _ = events

assert msg_event["contexts"]["trace"]
assert "trace_id" in msg_event["contexts"]["trace"]
Expand Down
60 changes: 0 additions & 60 deletions tests/integrations/celery/test_update_celery_task_headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,33 +77,6 @@ def test_span_with_transaction(sentry_init):
)


def test_span_with_no_transaction(sentry_init):
sentry_init(enable_tracing=True)
headers = {}

with sentry_sdk.start_span(op="test_span") as span:
updated_headers = _update_celery_task_headers(headers, span, False)

assert updated_headers["sentry-trace"] == span.to_traceparent()
assert updated_headers["headers"]["sentry-trace"] == span.to_traceparent()
assert "baggage" not in updated_headers.keys()
assert "baggage" not in updated_headers["headers"].keys()


def test_custom_span(sentry_init):
sentry_init(enable_tracing=True)
span = sentry_sdk.tracing.Span()
headers = {}

with sentry_sdk.start_transaction(name="test_transaction"):
updated_headers = _update_celery_task_headers(headers, span, False)

assert updated_headers["sentry-trace"] == span.to_traceparent()
assert updated_headers["headers"]["sentry-trace"] == span.to_traceparent()
assert "baggage" not in updated_headers.keys()
assert "baggage" not in updated_headers["headers"].keys()


def test_span_with_transaction_custom_headers(sentry_init):
sentry_init(enable_tracing=True)
headers = {
Expand Down Expand Up @@ -137,36 +110,3 @@ def test_span_with_transaction_custom_headers(sentry_init):
assert updated_headers["headers"]["baggage"] == combined_baggage.serialize(
include_third_party=True
)


def test_span_with_no_transaction_custom_headers(sentry_init):
sentry_init(enable_tracing=True)
headers = {
"baggage": BAGGAGE_VALUE,
"sentry-trace": SENTRY_TRACE_VALUE,
}

with sentry_sdk.start_span(op="test_span") as span:
updated_headers = _update_celery_task_headers(headers, span, False)

assert updated_headers["sentry-trace"] == span.to_traceparent()
assert updated_headers["headers"]["sentry-trace"] == span.to_traceparent()
assert updated_headers["baggage"] == headers["baggage"]
assert updated_headers["headers"]["baggage"] == headers["baggage"]


def test_custom_span_custom_headers(sentry_init):
sentry_init(enable_tracing=True)
span = sentry_sdk.tracing.Span()
headers = {
"baggage": BAGGAGE_VALUE,
"sentry-trace": SENTRY_TRACE_VALUE,
}

with sentry_sdk.start_transaction(name="test_transaction"):
updated_headers = _update_celery_task_headers(headers, span, False)

assert updated_headers["sentry-trace"] == span.to_traceparent()
assert updated_headers["headers"]["sentry-trace"] == span.to_traceparent()
assert updated_headers["baggage"] == headers["baggage"]
assert updated_headers["headers"]["baggage"] == headers["baggage"]
26 changes: 22 additions & 4 deletions tests/integrations/httpx/test_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pytest
import responses

import sentry_sdk
from sentry_sdk import capture_message, start_transaction
from sentry_sdk.consts import MATCH_ALL, SPANDATA
from sentry_sdk.integrations.httpx import HttpxIntegration
Expand Down Expand Up @@ -258,10 +259,11 @@ def test_option_trace_propagation_targets(
integrations=[HttpxIntegration()],
)

if asyncio.iscoroutinefunction(httpx_client.get):
asyncio.get_event_loop().run_until_complete(httpx_client.get(url))
else:
httpx_client.get(url)
with sentry_sdk.start_transaction(): # Must be in a transaction to propagate headers
if asyncio.iscoroutinefunction(httpx_client.get):
asyncio.get_event_loop().run_until_complete(httpx_client.get(url))
else:
httpx_client.get(url)

request_headers = httpx_mock.get_request().headers

Expand All @@ -271,6 +273,22 @@ def test_option_trace_propagation_targets(
assert "sentry-trace" not in request_headers


def test_do_not_propagate_outside_transaction(sentry_init, httpx_mock):
httpx_mock.add_response()

sentry_init(
traces_sample_rate=1.0,
trace_propagation_targets=[MATCH_ALL],
integrations=[HttpxIntegration()],
)

httpx_client = httpx.Client()
httpx_client.get("http://example.com/")

request_headers = httpx_mock.get_request().headers
assert "sentry-trace" not in request_headers


@pytest.mark.tests_internal_exceptions
def test_omit_url_data_if_parsing_fails(sentry_init, capture_events):
sentry_init(integrations=[HttpxIntegration()])
Expand Down
40 changes: 40 additions & 0 deletions tests/tracing/test_propagation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import sentry_sdk
import pytest


def test_standalone_span_iter_headers(sentry_init):
sentry_init(enable_tracing=True)

with sentry_sdk.start_span(op="test") as span:
with pytest.raises(StopIteration):
# We should not have any propagation headers
next(span.iter_headers())


def test_span_in_span_iter_headers(sentry_init):
sentry_init(enable_tracing=True)

with sentry_sdk.start_span(op="test"):
with sentry_sdk.start_span(op="test2") as span_inner:
with pytest.raises(StopIteration):
# We should not have any propagation headers
next(span_inner.iter_headers())


def test_span_in_transaction(sentry_init):
sentry_init(enable_tracing=True)

with sentry_sdk.start_transaction(op="test"):
with sentry_sdk.start_span(op="test2") as span:
# Ensure the headers are there
next(span.iter_headers())


def test_span_in_span_in_transaction(sentry_init):
sentry_init(enable_tracing=True)

with sentry_sdk.start_transaction(op="test"):
with sentry_sdk.start_span(op="test2"):
with sentry_sdk.start_span(op="test3") as span_inner:
# Ensure the headers are there
next(span_inner.iter_headers())

0 comments on commit 94a6c2a

Please sign in to comment.