From bf8455024044479d58039c991351020609fe2087 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 15 Nov 2024 16:19:28 +0100 Subject: [PATCH 1/2] Fix test_breadcrumbs --- sentry_sdk/tracing.py | 6 ++- tests/test_breadcrumbs.py | 80 +++++++-------------------------------- 2 files changed, 17 insertions(+), 69 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 59971e274e..06cc344b71 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1253,8 +1253,10 @@ def __init__( # Prepopulate some attrs so that they're accessible in traces_sampler attributes = attributes or {} - attributes[SentrySpanAttribute.OP] = op - attributes[SentrySpanAttribute.SOURCE] = source + if op is not None: + attributes[SentrySpanAttribute.OP] = op + if source is not None: + attributes[SentrySpanAttribute.SOURCE] = source if sampled is not None: attributes[SentrySpanAttribute.CUSTOM_SAMPLED] = sampled diff --git a/tests/test_breadcrumbs.py b/tests/test_breadcrumbs.py index 988f536fde..391c24cfc7 100644 --- a/tests/test_breadcrumbs.py +++ b/tests/test_breadcrumbs.py @@ -1,7 +1,6 @@ from unittest import mock import sentry_sdk -from sentry_sdk.consts import OP def test_breadcrumbs(sentry_init, capture_events): @@ -26,7 +25,7 @@ def test_breadcrumbs(sentry_init, capture_events): }, } - with sentry_sdk.start_transaction(name="trx-breadcrumbs"): + with sentry_sdk.start_span(name="trx-breadcrumbs"): sentry_sdk.add_breadcrumb(message="breadcrumb0", **add_breadcrumbs_kwargs) with sentry_sdk.start_span(name="span1", op="function"): @@ -37,26 +36,9 @@ def test_breadcrumbs(sentry_init, capture_events): message="breadcrumb2", **add_breadcrumbs_kwargs ) - # Spans that create breadcrumbs automatically - with sentry_sdk.start_span(name="span3", op=OP.DB_REDIS) as span3: - span3.set_data("span3_data", "data on the redis span") - span3.set_tag("span3_tag", "tag on the redis span") - - with sentry_sdk.start_span(name="span4", op=OP.HTTP_CLIENT) as span4: - span4.set_data("span4_data", "data on the http.client span") - span4.set_tag("span4_tag", "tag on the http.client span") - - with sentry_sdk.start_span(name="span5", op=OP.SUBPROCESS) as span5: - span5.set_data("span5_data", "data on the subprocess span") - span5.set_tag("span5_tag", "tag on the subprocess span") - - with sentry_sdk.start_span(name="span6", op="function") as span6: - # This data on the span is not added to custom breadcrumbs. - # Data from the span is only added to automatic breadcrumbs shown above - span6.set_data("span6_data", "data on span6") - span6.set_tag("span6_tag", "tag on the span6") + with sentry_sdk.start_span(name="span3", op="function"): sentry_sdk.add_breadcrumb( - message="breadcrumb6", **add_breadcrumbs_kwargs + message="breadcrumb3", **add_breadcrumbs_kwargs ) try: @@ -64,14 +46,15 @@ def test_breadcrumbs(sentry_init, capture_events): except ZeroDivisionError as ex: sentry_sdk.capture_exception(ex) - (error,) = events + assert len(events) == 2 + error = events[0] breadcrumbs = error["breadcrumbs"]["values"] for crumb in breadcrumbs: print(crumb) - assert len(breadcrumbs) == 7 + assert len(breadcrumbs) == 4 # Check for my custom breadcrumbs for i in range(0, 3): @@ -88,53 +71,16 @@ def test_breadcrumbs(sentry_init, capture_events): } assert breadcrumbs[i]["timestamp"] == mock.ANY - # Check automatic redis breadcrumbs - assert breadcrumbs[3]["message"] == "span3" - assert breadcrumbs[3]["type"] == "redis" - assert breadcrumbs[3]["category"] == "redis" - assert "level" not in breadcrumbs[3] - assert "origin" not in breadcrumbs[3] + # Check for custom breadcrumbs on span3 + assert breadcrumbs[3]["message"] == "breadcrumb3" + assert breadcrumbs[3]["type"] == "navigation" + assert breadcrumbs[3]["category"] == "unit_tests.breadcrumbs" + assert breadcrumbs[3]["level"] == "fatal" + assert breadcrumbs[3]["origin"] == "unit-tests" assert breadcrumbs[3]["data"] == { - "span3_tag": "tag on the redis span", - } - assert breadcrumbs[3]["timestamp"] == mock.ANY - - # Check automatic http.client breadcrumbs - assert "message" not in breadcrumbs[4] - assert breadcrumbs[4]["type"] == "http" - assert breadcrumbs[4]["category"] == "httplib" - assert "level" not in breadcrumbs[4] - assert "origin" not in breadcrumbs[4] - assert breadcrumbs[4]["data"] == { - "thread.id": mock.ANY, - "thread.name": mock.ANY, - "span4_data": "data on the http.client span", - } - assert breadcrumbs[4]["timestamp"] == mock.ANY - - # Check automatic subprocess breadcrumbs - assert breadcrumbs[5]["message"] == "span5" - assert breadcrumbs[5]["type"] == "subprocess" - assert breadcrumbs[5]["category"] == "subprocess" - assert "level" not in breadcrumbs[5] - assert "origin" not in breadcrumbs[5] - assert breadcrumbs[5]["data"] == { - "thread.id": mock.ANY, - "thread.name": mock.ANY, - "span5_data": "data on the subprocess span", - } - assert breadcrumbs[5]["timestamp"] == mock.ANY - - # Check for custom breadcrumbs on span6 - assert breadcrumbs[6]["message"] == "breadcrumb6" - assert breadcrumbs[6]["type"] == "navigation" - assert breadcrumbs[6]["category"] == "unit_tests.breadcrumbs" - assert breadcrumbs[6]["level"] == "fatal" - assert breadcrumbs[6]["origin"] == "unit-tests" - assert breadcrumbs[6]["data"] == { "string": "foobar", "number": 4.2, "array": [1, 2, 3], "dict": {"foo": "bar"}, } - assert breadcrumbs[6]["timestamp"] == mock.ANY + assert breadcrumbs[3]["timestamp"] == mock.ANY From 5dc59d95a655acce1efd2d816e47d1dc0383548d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 18 Nov 2024 12:43:39 +0100 Subject: [PATCH 2/2] Fix test_dsc and twp baggage handling (#3789) --- .../integrations/opentelemetry/utils.py | 4 +- sentry_sdk/scope.py | 9 ++--- tests/test_dsc.py | 38 +++++++++---------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/utils.py b/sentry_sdk/integrations/opentelemetry/utils.py index 07e60ddd0f..6127ceba5c 100644 --- a/sentry_sdk/integrations/opentelemetry/utils.py +++ b/sentry_sdk/integrations/opentelemetry/utils.py @@ -3,7 +3,7 @@ from datetime import datetime, timezone from urllib3.util import parse_url as urlparse -from urllib.parse import quote +from urllib.parse import quote, unquote from opentelemetry.trace import ( Span as AbstractSpan, SpanKind, @@ -354,7 +354,7 @@ def dsc_from_trace_state(trace_state): for k, v in trace_state.items(): if Baggage.SENTRY_PREFIX_REGEX.match(k): key = re.sub(Baggage.SENTRY_PREFIX_REGEX, "", k) - dsc[key] = v + dsc[unquote(key)] = unquote(v) return dsc diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index 48b8571b98..fbe258fb8a 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -480,13 +480,10 @@ def generate_propagation_context(self, incoming_data=None): def get_dynamic_sampling_context(self): # type: () -> Optional[Dict[str, str]] """ - Returns the Dynamic Sampling Context from the Propagation Context. + Returns the Dynamic Sampling Context from the baggage or populates one. """ - return ( - self._propagation_context.dynamic_sampling_context - if self._propagation_context - else None - ) + baggage = self.get_baggage() + return baggage.dynamic_sampling_context() if baggage else None def get_traceparent(self, *args, **kwargs): # type: (Any, Any) -> Optional[str] diff --git a/tests/test_dsc.py b/tests/test_dsc.py index 3b8cff5baf..e1ceb80a05 100644 --- a/tests/test_dsc.py +++ b/tests/test_dsc.py @@ -27,8 +27,8 @@ def test_dsc_head_of_trace(sentry_init, capture_envelopes): ) envelopes = capture_envelopes() - # We start a new transaction - with sentry_sdk.start_transaction(name="foo"): + # We start a new root_span + with sentry_sdk.start_span(name="foo"): pass assert len(envelopes) == 1 @@ -95,10 +95,10 @@ def test_dsc_continuation_of_trace(sentry_init, capture_envelopes): "HTTP_BAGGAGE": baggage, } - # We continue the incoming trace and start a new transaction - transaction = sentry_sdk.continue_trace(incoming_http_headers) - with sentry_sdk.start_transaction(transaction, name="foo"): - pass + # We continue the incoming trace and start a new root span + with sentry_sdk.continue_trace(incoming_http_headers): + with sentry_sdk.start_span(name="foo"): + pass assert len(envelopes) == 1 @@ -145,7 +145,7 @@ def test_dsc_issue(sentry_init, capture_envelopes): ) envelopes = capture_envelopes() - # No transaction is started, just an error is captured + # No root span is started, just an error is captured try: 1 / 0 except ZeroDivisionError as exp: @@ -181,8 +181,8 @@ def test_dsc_issue(sentry_init, capture_envelopes): def test_dsc_issue_with_tracing(sentry_init, capture_envelopes): """ - Our service has tracing enabled and an error occurs in an transaction. - Envelopes containing errors also have the same DSC than the transaction envelopes. + Our service has tracing enabled and an error occurs in an root span. + Envelopes containing errors also have the same DSC than the root span envelopes. """ sentry_init( dsn="https://mysecret@bla.ingest.sentry.io/12312012", @@ -192,8 +192,8 @@ def test_dsc_issue_with_tracing(sentry_init, capture_envelopes): ) envelopes = capture_envelopes() - # We start a new transaction and an error occurs - with sentry_sdk.start_transaction(name="foo"): + # We start a new root span and an error occurs + with sentry_sdk.start_span(name="foo"): try: 1 / 0 except ZeroDivisionError as exp: @@ -239,7 +239,7 @@ def test_dsc_issue_with_tracing(sentry_init, capture_envelopes): "traces_sample_rate", [ 0, # no traces will be started, but if incoming traces will be continued (by our instrumentations, not happening in this test) - None, # no tracing at all. This service will never create transactions. + None, # no tracing at all. This service will never create root spans. ], ) def test_dsc_issue_twp(sentry_init, capture_envelopes, traces_sample_rate): @@ -278,14 +278,14 @@ def test_dsc_issue_twp(sentry_init, capture_envelopes, traces_sample_rate): } # We continue the trace (meaning: saving the incoming trace information on the scope) - # but in this test, we do not start a transaction. - sentry_sdk.continue_trace(incoming_http_headers) + # but in this test, we do not start a root span. + with sentry_sdk.continue_trace(incoming_http_headers): - # No transaction is started, just an error is captured - try: - 1 / 0 - except ZeroDivisionError as exp: - sentry_sdk.capture_exception(exp) + # No root span is started, just an error is captured + try: + 1 / 0 + except ZeroDivisionError as exp: + sentry_sdk.capture_exception(exp) assert len(envelopes) == 1