From 625e1b3608862f68295006edee00d0d0916787f2 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Tue, 27 Jun 2023 11:21:00 +0200 Subject: [PATCH 1/5] Do not overwrite existing baggage on outgoing requests (#2191) --- sentry_sdk/integrations/celery.py | 19 ++++++++++- sentry_sdk/integrations/httpx.py | 17 ++++++++-- tests/integrations/celery/test_celery.py | 42 +++++++++++++++++------- tests/integrations/httpx/test_httpx.py | 40 ++++++++++++++++++++++ 4 files changed, 103 insertions(+), 15 deletions(-) diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py index 741a2c8bb7..443fcdad45 100644 --- a/sentry_sdk/integrations/celery.py +++ b/sentry_sdk/integrations/celery.py @@ -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, @@ -158,7 +158,20 @@ 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) + sentry_baggage = headers.get(BAGGAGE_HEADER_NAME) + + combined_baggage = sentry_baggage or existing_baggage + if sentry_baggage and existing_baggage: + combined_baggage = "{},{}".format( + existing_baggage, + sentry_baggage, + ) + kwarg_headers.update(headers) + if combined_baggage: + kwarg_headers[BAGGAGE_HEADER_NAME] = combined_baggage # https://github.com/celery/celery/issues/4875 # @@ -166,6 +179,10 @@ def apply_async(*args, **kwargs): # tracing tools (dd-trace-py) also employ this exact # workaround and we don't want to break them. kwarg_headers.setdefault("headers", {}).update(headers) + if combined_baggage: + kwarg_headers["headers"][ + BAGGAGE_HEADER_NAME + ] = combined_baggage # Add the Sentry options potentially added in `sentry_apply_entry` # to the headers (done when auto-instrumenting Celery Beat tasks) diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index e84a28d165..04db5047b4 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -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, @@ -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) @@ -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) diff --git a/tests/integrations/celery/test_celery.py b/tests/integrations/celery/test_celery.py index d120d34a12..304f6c2f04 100644 --- a/tests/integrations/celery/test_celery.py +++ b/tests/integrations/celery/test_celery.py @@ -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 @@ -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() @@ -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 - - 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", + ] + ) diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index 72188a23e3..9b7842fbb7 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -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"}) + ) + 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", [ From d3f95685b397cca83649052bc0014c3aeb26e152 Mon Sep 17 00:00:00 2001 From: Evgeny Seregin Date: Wed, 28 Jun 2023 12:48:35 +0600 Subject: [PATCH 2/5] Fix TaskLockedException handling (#2206) --- sentry_sdk/integrations/huey.py | 4 ++-- tests/integrations/huey/test_huey.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/huey.py b/sentry_sdk/integrations/huey.py index 7c3fcbc70c..52b0e549a2 100644 --- a/sentry_sdk/integrations/huey.py +++ b/sentry_sdk/integrations/huey.py @@ -26,12 +26,12 @@ try: from huey.api import Huey, Result, ResultGroup, Task - from huey.exceptions import CancelExecution, RetryTask + from huey.exceptions import CancelExecution, RetryTask, TaskLockedException except ImportError: raise DidNotEnable("Huey is not installed") -HUEY_CONTROL_FLOW_EXCEPTIONS = (CancelExecution, RetryTask) +HUEY_CONTROL_FLOW_EXCEPTIONS = (CancelExecution, RetryTask, TaskLockedException) class HueyIntegration(Integration): diff --git a/tests/integrations/huey/test_huey.py b/tests/integrations/huey/test_huey.py index 819a4816d7..29e4d37027 100644 --- a/tests/integrations/huey/test_huey.py +++ b/tests/integrations/huey/test_huey.py @@ -118,6 +118,34 @@ def retry_task(context): assert len(huey) == 0 +@pytest.mark.parametrize("lock_name", ["lock.a", "lock.b"], ids=["locked", "unlocked"]) +def test_task_lock(capture_events, init_huey, lock_name): + huey = init_huey() + + task_lock_name = "lock.a" + should_be_locked = task_lock_name == lock_name + + @huey.task() + @huey.lock_task(task_lock_name) + def maybe_locked_task(): + pass + + events = capture_events() + + with huey.lock_task(lock_name): + assert huey.is_locked(task_lock_name) == should_be_locked + result = execute_huey_task(huey, maybe_locked_task) + + (event,) = events + + assert event["transaction"] == "maybe_locked_task" + assert event["tags"]["huey_task_id"] == result.task.id + assert ( + event["contexts"]["trace"]["status"] == "aborted" if should_be_locked else "ok" + ) + assert len(huey) == 0 + + def test_huey_enqueue(init_huey, capture_events): huey = init_huey() From d4ecab3956ff01165b66238dde19875df5cef16f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 28 Jun 2023 10:09:34 +0200 Subject: [PATCH 3/5] Use new top level api in `trace_propagation_meta` (#2202) Use new top level api in trace_propagation_meta and also move the functions into the Hub, so they can be used in the Hub. (following the pattern of other top level API) Refs #2186 --- sentry_sdk/api.py | 37 ++-------------------- sentry_sdk/hub.py | 81 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 78 insertions(+), 40 deletions(-) diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index feb95ea669..f0c6a87432 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -4,10 +4,6 @@ from sentry_sdk.hub import Hub from sentry_sdk.scope import Scope from sentry_sdk.tracing import NoOpSpan, Transaction -from sentry_sdk.tracing_utils import ( - has_tracing_enabled, - normalize_incoming_data, -) if TYPE_CHECKING: from typing import Any @@ -254,12 +250,7 @@ def get_traceparent(): """ Returns the traceparent either from the active span or from the scope. """ - hub = Hub.current - if hub.client is not None: - if has_tracing_enabled(hub.client.options) and hub.scope.span is not None: - return hub.scope.span.to_traceparent() - - return hub.scope.get_traceparent() + return Hub.current.get_traceparent() def get_baggage(): @@ -267,20 +258,7 @@ def get_baggage(): """ Returns Baggage either from the active span or from the scope. """ - hub = Hub.current - if ( - hub.client is not None - and has_tracing_enabled(hub.client.options) - and hub.scope.span is not None - ): - baggage = hub.scope.span.to_baggage() - else: - baggage = hub.scope.get_baggage() - - if baggage is not None: - return baggage.serialize() - - return None + return Hub.current.get_baggage() def continue_trace(environ_or_headers, op=None, name=None, source=None): @@ -288,13 +266,4 @@ def continue_trace(environ_or_headers, op=None, name=None, source=None): """ Sets the propagation context from environment or headers and returns a transaction. """ - with Hub.current.configure_scope() as scope: - scope.generate_propagation_context(environ_or_headers) - - transaction = Transaction.continue_from_headers( - normalize_incoming_data(environ_or_headers), - op=op, - name=name, - source=source, - ) - return transaction + return Hub.current.continue_trace(environ_or_headers, op, name, source) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index bb755f4101..553222d672 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -9,9 +9,19 @@ from sentry_sdk.scope import Scope from sentry_sdk.client import Client from sentry_sdk.profiler import Profile -from sentry_sdk.tracing import NoOpSpan, Span, Transaction +from sentry_sdk.tracing import ( + NoOpSpan, + Span, + Transaction, + BAGGAGE_HEADER_NAME, + SENTRY_TRACE_HEADER_NAME, +) from sentry_sdk.session import Session -from sentry_sdk.tracing_utils import has_tracing_enabled +from sentry_sdk.tracing_utils import ( + has_tracing_enabled, + normalize_incoming_data, +) + from sentry_sdk.utils import ( exc_info_from_error, event_from_exception, @@ -533,6 +543,22 @@ def start_transaction( return transaction + def continue_trace(self, environ_or_headers, op=None, name=None, source=None): + # type: (Dict[str, Any], Optional[str], Optional[str], Optional[str]) -> Transaction + """ + Sets the propagation context from environment or headers and returns a transaction. + """ + with self.configure_scope() as scope: + scope.generate_propagation_context(environ_or_headers) + + transaction = Transaction.continue_from_headers( + normalize_incoming_data(environ_or_headers), + op=op, + name=name, + source=source, + ) + return transaction + @overload def push_scope( self, callback=None # type: Optional[None] @@ -699,6 +725,36 @@ def flush( if client is not None: return client.flush(timeout=timeout, callback=callback) + def get_traceparent(self): + # type: () -> Optional[str] + """ + Returns the traceparent either from the active span or from the scope. + """ + if self.client is not None: + if has_tracing_enabled(self.client.options) and self.scope.span is not None: + return self.scope.span.to_traceparent() + + return self.scope.get_traceparent() + + def get_baggage(self): + # type: () -> Optional[str] + """ + Returns Baggage either from the active span or from the scope. + """ + if ( + self.client is not None + and has_tracing_enabled(self.client.options) + and self.scope.span is not None + ): + baggage = self.scope.span.to_baggage() + else: + baggage = self.scope.get_baggage() + + if baggage is not None: + return baggage.serialize() + + return None + def iter_trace_propagation_headers(self, span=None): # type: (Optional[Span]) -> Generator[Tuple[str, str], None, None] """ @@ -723,13 +779,26 @@ def iter_trace_propagation_headers(self, span=None): def trace_propagation_meta(self, span=None): # type: (Optional[Span]) -> str """ - Return meta tags which should be injected into the HTML template - to allow propagation of trace data. + Return meta tags which should be injected into HTML templates + to allow propagation of trace information. """ + if span is None: + logger.warning( + "The parameter `span` in trace_propagation_meta() is deprecated and will be removed in the future." + ) + meta = "" - for name, content in self.iter_trace_propagation_headers(span): - meta += '' % (name, content) + sentry_trace = self.get_traceparent() + if sentry_trace is not None: + meta += '' % ( + SENTRY_TRACE_HEADER_NAME, + sentry_trace, + ) + + baggage = self.get_baggage() + if baggage is not None: + meta += '' % (BAGGAGE_HEADER_NAME, baggage) return meta From d26e4a92b280a343453515baa4fa303e01d74a74 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 28 Jun 2023 13:02:18 +0200 Subject: [PATCH 4/5] Change API doc theme (#2210) The previously used `alabaster` theme had issues with text overlapping. --- .github/workflows/ci.yml | 2 +- docs-requirements.txt | 2 +- docs/conf.py | 16 +++++++++------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c397adabb..798768015b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,7 +84,7 @@ jobs: - uses: actions/checkout@v2 - uses: actions/setup-python@v4 with: - python-version: 3.9 + python-version: 3.11 - run: | pip install virtualenv diff --git a/docs-requirements.txt b/docs-requirements.txt index 2a98682baa..e1f694004b 100644 --- a/docs-requirements.txt +++ b/docs-requirements.txt @@ -1,4 +1,4 @@ +shibuya sphinx==7.0.1 -sphinx-rtd-theme sphinx-autodoc-typehints[type_comments]>=1.8.0 typing-extensions diff --git a/docs/conf.py b/docs/conf.py index 9dde301cfb..0420f7f5ef 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -2,16 +2,16 @@ import os import sys - import typing +from datetime import datetime # prevent circular imports import sphinx.builders.html import sphinx.builders.latex import sphinx.builders.texinfo import sphinx.builders.text -import sphinx.ext.autodoc -import urllib3.exceptions +import sphinx.ext.autodoc # noqa: F401 +import urllib3.exceptions # noqa: F401 typing.TYPE_CHECKING = True @@ -27,7 +27,7 @@ # -- Project information ----------------------------------------------------- project = "sentry-python" -copyright = "2019, Sentry Team and Contributors" +copyright = "2019-{}, Sentry Team and Contributors".format(datetime.now().year) author = "Sentry Team and Contributors" release = "1.26.0" @@ -87,13 +87,15 @@ on_rtd = os.environ.get("READTHEDOCS", None) == "True" -html_theme = "alabaster" +html_theme = "shibuya" # Theme options are theme-specific and customize the look and feel of a theme # further. For a list of options available for each theme, see the # documentation. # -# html_theme_options = {} +html_theme_options = { + "github_url": "https://github.com/getsentry/sentry-python", +} # Add any paths that contain custom static files (such as style sheets) here, # relative to this directory. They are copied after the builtin static files, @@ -167,7 +169,7 @@ "sentry-python Documentation", author, "sentry-python", - "One line description of project.", + "The official Sentry SDK for Python.", "Miscellaneous", ) ] From 679529541d72a49ace509b2106984152f29f67d4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 28 Jun 2023 15:31:56 +0200 Subject: [PATCH 5/5] Fix trace context in event payload (#2205) Make sure that always a trace context is added to the event payload. But also make sure that if there is already a trace context in the event, do not overwrite it. (This used to be the behavior before tracing without performance. See: https://github.com/getsentry/sentry-python/blob/1.25.1/sentry_sdk/scope.py#L420) --------- Co-authored-by: Ivana Kellyerova --- sentry_sdk/scope.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index 3ad61d31d5..c25b5efec2 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -605,11 +605,11 @@ def _drop(cause, ty): contexts = event.setdefault("contexts", {}) - if has_tracing_enabled(options): - if self._span is not None: + if contexts.get("trace") is None: + if has_tracing_enabled(options) and self._span is not None: contexts["trace"] = self._span.get_trace_context() - else: - contexts["trace"] = self.get_trace_context() + else: + contexts["trace"] = self.get_trace_context() exc_info = hint.get("exc_info") if exc_info is not None: