Skip to content
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

Do not overwrite existing baggage on outgoing requests #2191

Merged
merged 15 commits into from
Jun 27, 2023
19 changes: 17 additions & 2 deletions sentry_sdk/integrations/celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from sentry_sdk.hub import Hub
from sentry_sdk.integrations import Integration, DidNotEnable
from sentry_sdk.integrations.logging import ignore_logger
from sentry_sdk.tracing import TRANSACTION_SOURCE_TASK
from sentry_sdk.tracing import BAGGAGE_HEADER_NAME, TRANSACTION_SOURCE_TASK
from sentry_sdk._types import TYPE_CHECKING
from sentry_sdk.utils import (
capture_internal_exceptions,
Expand Down Expand Up @@ -158,14 +158,29 @@ def apply_async(*args, **kwargs):
# Note: kwargs can contain headers=None, so no setdefault!
# Unsure which backend though.
kwarg_headers = kwargs.get("headers") or {}
existing_baggage = kwarg_headers.get(BAGGAGE_HEADER_NAME)
kwarg_headers.update(headers)
if existing_baggage:
kwarg_headers[BAGGAGE_HEADER_NAME] = "{},{}".format(
existing_baggage, headers[BAGGAGE_HEADER_NAME]
)

# https://github.com/celery/celery/issues/4875
#
# Need to setdefault the inner headers too since other
# tracing tools (dd-trace-py) also employ this exact
# workaround and we don't want to break them.
kwarg_headers.setdefault("headers", {}).update(headers)
kwarg_headers.setdefault("headers", {})
existing_baggage = kwarg_headers["headers"].get(
BAGGAGE_HEADER_NAME
)
kwarg_headers["headers"].update(headers)
if existing_baggage:
kwarg_headers["headers"][
BAGGAGE_HEADER_NAME
] = "{},{}".format(
existing_baggage, headers[BAGGAGE_HEADER_NAME]
)
antonpirker marked this conversation as resolved.
Show resolved Hide resolved

# Add the Sentry options potentially added in `sentry_apply_entry`
# to the headers (done when auto-instrumenting Celery Beat tasks)
Expand Down
17 changes: 15 additions & 2 deletions sentry_sdk/integrations/httpx.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from sentry_sdk import Hub
from sentry_sdk.consts import OP, SPANDATA
from sentry_sdk.integrations import Integration, DidNotEnable
from sentry_sdk.tracing import BAGGAGE_HEADER_NAME
from sentry_sdk.tracing_utils import should_propagate_trace
from sentry_sdk.utils import (
SENSITIVE_DATA_SUBSTITUTE,
Expand Down Expand Up @@ -72,7 +73,13 @@ def send(self, request, **kwargs):
key=key, value=value, url=request.url
)
)
request.headers[key] = value
if key == BAGGAGE_HEADER_NAME and request.headers.get(
BAGGAGE_HEADER_NAME
):
# do not overwrite any existing baggage, just append to it
request.headers[key] += "," + value
else:
request.headers[key] = value

rv = real_send(self, request, **kwargs)

Expand Down Expand Up @@ -119,7 +126,13 @@ async def send(self, request, **kwargs):
key=key, value=value, url=request.url
)
)
request.headers[key] = value
if key == BAGGAGE_HEADER_NAME and request.headers.get(
BAGGAGE_HEADER_NAME
):
# do not overwrite any existing baggage, just append to it
request.headers[key] += "," + value
else:
request.headers[key] = value

rv = await real_send(self, request, **kwargs)

Expand Down
42 changes: 30 additions & 12 deletions tests/integrations/celery/test_celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

from celery import Celery, VERSION
from celery.bin import worker
from celery.signals import task_success

try:
from unittest import mock # python 3.3 and above
Expand Down Expand Up @@ -360,7 +359,7 @@ def dummy_task(self):
# TODO: This test is hanging when running test with `tox --parallel auto`. Find out why and fix it!
@pytest.mark.skip
@pytest.mark.forked
def test_redis_backend_trace_propagation(init_celery, capture_events_forksafe, tmpdir):
def test_redis_backend_trace_propagation(init_celery, capture_events_forksafe):
celery = init_celery(traces_sample_rate=1.0, backend="redis", debug=True)

events = capture_events_forksafe()
Expand Down Expand Up @@ -493,17 +492,36 @@ def test_task_headers(celery):
"sentry-monitor-check-in-id": "123abc",
}

@celery.task(name="dummy_task")
def dummy_task(x, y):
return x + y

def crons_task_success(sender, **kwargs):
headers = _get_headers(sender)
assert headers == sentry_crons_setup
antonpirker marked this conversation as resolved.
Show resolved Hide resolved

task_success.connect(crons_task_success)
@celery.task(name="dummy_task", bind=True)
def dummy_task(self, x, y):
return _get_headers(self)

# This is how the Celery Beat auto-instrumentation starts a task
# in the monkey patched version of `apply_async`
# in `sentry_sdk/integrations/celery.py::_wrap_apply_async()`
dummy_task.apply_async(args=(1, 0), headers=sentry_crons_setup)
result = dummy_task.apply_async(args=(1, 0), headers=sentry_crons_setup)
assert result.get() == sentry_crons_setup


def test_baggage_propagation(init_celery):
celery = init_celery(traces_sample_rate=1.0, release="abcdef")

@celery.task(name="dummy_task", bind=True)
def dummy_task(self, x, y):
return _get_headers(self)

with start_transaction() as transaction:
result = dummy_task.apply_async(
args=(1, 0),
headers={"baggage": "custom=value"},
).get()

assert sorted(result["baggage"].split(",")) == sorted(
[
"sentry-release=abcdef",
"sentry-trace_id={}".format(transaction.trace_id),
"sentry-environment=production",
"sentry-sample_rate=1.0",
"custom=value",
]
)
40 changes: 40 additions & 0 deletions tests/integrations/httpx/test_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,46 @@ def test_outgoing_trace_headers(sentry_init, httpx_client):
)


@pytest.mark.parametrize(
"httpx_client",
(httpx.Client(), httpx.AsyncClient()),
)
def test_outgoing_trace_headers_append_to_baggage(sentry_init, httpx_client):
sentry_init(
traces_sample_rate=1.0,
integrations=[HttpxIntegration()],
release="d08ebdb9309e1b004c6f52202de58a09c2268e42",
)

url = "http://example.com/"
responses.add(responses.GET, url, status=200)

with start_transaction(
name="/interactions/other-dogs/new-dog",
op="greeting.sniff",
trace_id="01234567890123456789012345678901",
) as transaction:
if asyncio.iscoroutinefunction(httpx_client.get):
response = asyncio.get_event_loop().run_until_complete(
httpx_client.get(url, headers={"baGGage": "custom=data"})
sentrivana marked this conversation as resolved.
Show resolved Hide resolved
)
else:
response = httpx_client.get(url, headers={"baGGage": "custom=data"})

request_span = transaction._span_recorder.spans[-1]
assert response.request.headers[
"sentry-trace"
] == "{trace_id}-{parent_span_id}-{sampled}".format(
trace_id=transaction.trace_id,
parent_span_id=request_span.span_id,
sampled=1,
)
assert (
response.request.headers["baggage"]
== "custom=data,sentry-trace_id=01234567890123456789012345678901,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0"
)


@pytest.mark.parametrize(
"httpx_client,trace_propagation_targets,url,trace_propagated",
[
Expand Down