Skip to content

fix(tracing): Only propagate headers from spans within transactions #3070

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
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
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())