From 2a4b8d51e9f2bc6b607a8f8ce301494b766f834b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 6 Feb 2023 17:15:28 +0100 Subject: [PATCH 1/2] WIP --- sentry_sdk/client.py | 17 +--- sentry_sdk/consts.py | 1 - sentry_sdk/tracing.py | 99 ++------------------- sentry_sdk/tracing_utils.py | 171 ------------------------------------ tests/test_envelope.py | 61 +++---------- 5 files changed, 20 insertions(+), 329 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 9667751ee1..c4fa13e640 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -29,7 +29,6 @@ from sentry_sdk.sessions import SessionFlusher from sentry_sdk.envelope import Envelope from sentry_sdk.profiler import setup_profiler -from sentry_sdk.tracing_utils import has_tracestate_enabled, reinflate_tracestate from sentry_sdk._types import MYPY @@ -410,13 +409,6 @@ def capture_event( attachments = hint.get("attachments") - # this is outside of the `if` immediately below because even if we don't - # use the value, we want to make sure we remove it before the event is - # sent - raw_tracestate = ( - event_opt.get("contexts", {}).get("trace", {}).pop("tracestate", "") - ) - dynamic_sampling_context = ( event_opt.get("contexts", {}) .get("trace", {}) @@ -432,14 +424,7 @@ def capture_event( "sent_at": format_timestamp(datetime.utcnow()), } - if has_tracestate_enabled(): - tracestate_data = raw_tracestate and reinflate_tracestate( - raw_tracestate.replace("sentry=", "") - ) - - if tracestate_data: - headers["trace"] = tracestate_data - elif dynamic_sampling_context: + if dynamic_sampling_context: headers["trace"] = dynamic_sampling_context envelope = Envelope(headers=headers) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index b2d1ae26c7..ad95f3ce9c 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -33,7 +33,6 @@ "max_spans": Optional[int], "record_sql_params": Optional[bool], "smart_transaction_trimming": Optional[bool], - "propagate_tracestate": Optional[bool], "custom_measurements": Optional[bool], "profiles_sample_rate": Optional[float], "profiler_mode": Optional[str], diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 332b3a0c18..53c9e0761a 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -254,7 +254,7 @@ def continue_from_environ( # type: (...) -> Transaction """ Create a Transaction with the given params, then add in data pulled from - the 'sentry-trace', 'baggage' and 'tracestate' headers from the environ (if any) + the 'sentry-trace' and 'baggage' headers from the environ (if any) before returning the Transaction. This is different from `continue_from_headers` in that it assumes header @@ -277,7 +277,7 @@ def continue_from_headers( # type: (...) -> Transaction """ Create a transaction with the given params (including any data pulled from - the 'sentry-trace', 'baggage' and 'tracestate' headers). + the 'sentry-trace' and 'baggage' headers). """ # TODO move this to the Transaction class if cls is Span: @@ -303,8 +303,6 @@ def continue_from_headers( # baggage will be empty and immutable and won't be populated as head SDK. baggage.freeze() - kwargs.update(extract_tracestate_data(headers.get("tracestate"))) - transaction = Transaction(**kwargs) transaction.same_process_as_parent = False @@ -313,22 +311,12 @@ def continue_from_headers( def iter_headers(self): # type: () -> Iterator[Tuple[str, str]] """ - Creates a generator which returns the span's `sentry-trace`, `baggage` and - `tracestate` headers. - - If the span's containing transaction doesn't yet have a - `sentry_tracestate` value, this will cause one to be generated and - stored. + Creates a generator which returns the span's `sentry-trace` and `baggage` headers. + If the span's containing transaction doesn't yet have a `baggage` value, + this will cause one to be generated and stored. """ yield SENTRY_TRACE_HEADER_NAME, self.to_traceparent() - tracestate = self.to_tracestate() if has_tracestate_enabled(self) else None - # `tracestate` will only be `None` if there's no client or no DSN - # TODO (kmclb) the above will be true once the feature is no longer - # behind a flag - if tracestate: - yield "tracestate", tracestate - if self.containing_transaction: baggage = self.containing_transaction.get_baggage().serialize() if baggage: @@ -369,57 +357,6 @@ def to_traceparent(self): sampled = "0" return "%s-%s-%s" % (self.trace_id, self.span_id, sampled) - def to_tracestate(self): - # type: () -> Optional[str] - """ - Computes the `tracestate` header value using data from the containing - transaction. - - If the containing transaction doesn't yet have a `sentry_tracestate` - value, this will cause one to be generated and stored. - - If there is no containing transaction, a value will be generated but not - stored. - - Returns None if there's no client and/or no DSN. - """ - - sentry_tracestate = self.get_or_set_sentry_tracestate() - third_party_tracestate = ( - self.containing_transaction._third_party_tracestate - if self.containing_transaction - else None - ) - - if not sentry_tracestate: - return None - - header_value = sentry_tracestate - - if third_party_tracestate: - header_value = header_value + "," + third_party_tracestate - - return header_value - - def get_or_set_sentry_tracestate(self): - # type: (Span) -> Optional[str] - """ - Read sentry tracestate off of the span's containing transaction. - - If the transaction doesn't yet have a `_sentry_tracestate` value, - compute one and store it. - """ - transaction = self.containing_transaction - - if transaction: - if not transaction._sentry_tracestate: - transaction._sentry_tracestate = compute_tracestate_entry(self) - - return transaction._sentry_tracestate - - # orphan span - nowhere to store the value, so just return it - return compute_tracestate_entry(self) - def set_tag(self, key, value): # type: (str, Any) -> None self._tags[key] = value @@ -531,15 +468,6 @@ def get_trace_context(self): if self.status: rv["status"] = self.status - # if the transaction didn't inherit a tracestate value, and no outgoing - # requests - whose need for headers would have caused a tracestate value - # to be created - were made as part of the transaction, the transaction - # still won't have a tracestate value, so compute one now - sentry_tracestate = self.get_or_set_sentry_tracestate() - - if sentry_tracestate: - rv["tracestate"] = sentry_tracestate - if self.containing_transaction: rv[ "dynamic_sampling_context" @@ -555,13 +483,6 @@ class Transaction(Span): "parent_sampled", # used to create baggage value for head SDKs in dynamic sampling "sample_rate", - # the sentry portion of the `tracestate` header used to transmit - # correlation context for server-side dynamic sampling, of the form - # `sentry=xxxxx`, where `xxxxx` is the base64-encoded json of the - # correlation context data, missing trailing any = - "_sentry_tracestate", - # tracestate data from other vendors, of the form `dogs=yes,cats=maybe` - "_third_party_tracestate", "_measurements", "_contexts", "_profile", @@ -572,8 +493,6 @@ def __init__( self, name="", # type: str parent_sampled=None, # type: Optional[bool] - sentry_tracestate=None, # type: Optional[str] - third_party_tracestate=None, # type: Optional[str] baggage=None, # type: Optional[Baggage] source=TRANSACTION_SOURCE_CUSTOM, # type: str **kwargs # type: Any @@ -595,11 +514,6 @@ def __init__( self.source = source self.sample_rate = None # type: Optional[float] self.parent_sampled = parent_sampled - # if tracestate isn't inherited and set here, it will get set lazily, - # either the first time an outgoing request needs it for a header or the - # first time an event needs it for inclusion in the captured data - self._sentry_tracestate = sentry_tracestate - self._third_party_tracestate = third_party_tracestate self._measurements = {} # type: Dict[str, Any] self._contexts = {} # type: Dict[str, Any] self._profile = None # type: Optional[sentry_sdk.profiler.Profile] @@ -904,10 +818,7 @@ def finish(self, hub=None, end_timestamp=None): from sentry_sdk.tracing_utils import ( Baggage, EnvironHeaders, - compute_tracestate_entry, extract_sentrytrace_data, - extract_tracestate_data, - has_tracestate_enabled, has_tracing_enabled, is_valid_sample_rate, maybe_create_breadcrumbs_from_span, diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index cc1851ff46..63545f8d0d 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -1,6 +1,5 @@ import re import contextlib -import json import math from numbers import Real @@ -13,10 +12,7 @@ capture_internal_exceptions, Dsn, logger, - safe_str, - to_base64, to_string, - from_base64, ) from sentry_sdk._compat import PY2, iteritems from sentry_sdk._types import MYPY @@ -57,27 +53,6 @@ "([a-zA-Z0-9+/]{2,3})?" ) -# comma-delimited list of entries of the form `xxx=yyy` -tracestate_entry = "[^=]+=[^=]+" -TRACESTATE_ENTRIES_REGEX = re.compile( - # one or more xxxxx=yyyy entries - "^({te})+" - # each entry except the last must be followed by a comma - "(,|$)".format(te=tracestate_entry) -) - -# this doesn't check that the value is valid, just that there's something there -# of the form `sentry=xxxx` -SENTRY_TRACESTATE_ENTRY_REGEX = re.compile( - # either sentry is the first entry or there's stuff immediately before it, - # ending in a comma (this prevents matching something like `coolsentry=xxx`) - "(?:^|.+,)" - # sentry's part, not including the potential comma - "(sentry=[^,]*)" - # either there's a comma and another vendor's entry or we end - "(?:,.+|$)" -) - class EnvironHeaders(Mapping): # type: ignore def __init__( @@ -246,143 +221,6 @@ def extract_sentrytrace_data(header): } -def extract_tracestate_data(header): - # type: (Optional[str]) -> typing.Mapping[str, Optional[str]] - """ - Extracts the sentry tracestate value and any third-party data from the given - tracestate header, returning a dictionary of data. - """ - sentry_entry = third_party_entry = None - before = after = "" - - if header: - # find sentry's entry, if any - sentry_match = SENTRY_TRACESTATE_ENTRY_REGEX.search(header) - - if sentry_match: - sentry_entry = sentry_match.group(1) - - # remove the commas after the split so we don't end up with - # `xxx=yyy,,zzz=qqq` (double commas) when we put them back together - before, after = map(lambda s: s.strip(","), header.split(sentry_entry)) - - # extract sentry's value from its entry and test to make sure it's - # valid; if it isn't, discard the entire entry so that a new one - # will be created - sentry_value = sentry_entry.replace("sentry=", "") - if not re.search("^{b64}$".format(b64=base64_stripped), sentry_value): - sentry_entry = None - else: - after = header - - # if either part is invalid or empty, remove it before gluing them together - third_party_entry = ( - ",".join(filter(TRACESTATE_ENTRIES_REGEX.search, [before, after])) or None - ) - - return { - "sentry_tracestate": sentry_entry, - "third_party_tracestate": third_party_entry, - } - - -def compute_tracestate_value(data): - # type: (typing.Mapping[str, str]) -> str - """ - Computes a new tracestate value using the given data. - - Note: Returns just the base64-encoded data, NOT the full `sentry=...` - tracestate entry. - """ - - tracestate_json = json.dumps(data, default=safe_str) - - # Base64-encoded strings always come out with a length which is a multiple - # of 4. In order to achieve this, the end is padded with one or more `=` - # signs. Because the tracestate standard calls for using `=` signs between - # vendor name and value (`sentry=xxx,dogsaregreat=yyy`), to avoid confusion - # we strip the `=` - return (to_base64(tracestate_json) or "").rstrip("=") - - -def compute_tracestate_entry(span): - # type: (Span) -> Optional[str] - """ - Computes a new sentry tracestate for the span. Includes the `sentry=`. - - Will return `None` if there's no client and/or no DSN. - """ - data = {} - - hub = span.hub or sentry_sdk.Hub.current - - client = hub.client - scope = hub.scope - - if client and client.options.get("dsn"): - options = client.options - user = scope._user - - data = { - "trace_id": span.trace_id, - "environment": options["environment"], - "release": options.get("release"), - "public_key": Dsn(options["dsn"]).public_key, - } - - if user and (user.get("id") or user.get("segment")): - user_data = {} - - if user.get("id"): - user_data["id"] = user["id"] - - if user.get("segment"): - user_data["segment"] = user["segment"] - - data["user"] = user_data - - if span.containing_transaction: - data["transaction"] = span.containing_transaction.name - - return "sentry=" + compute_tracestate_value(data) - - return None - - -def reinflate_tracestate(encoded_tracestate): - # type: (str) -> typing.Optional[Mapping[str, str]] - """ - Given a sentry tracestate value in its encoded form, translate it back into - a dictionary of data. - """ - inflated_tracestate = None - - if encoded_tracestate: - # Base64-encoded strings always come out with a length which is a - # multiple of 4. In order to achieve this, the end is padded with one or - # more `=` signs. Because the tracestate standard calls for using `=` - # signs between vendor name and value (`sentry=xxx,dogsaregreat=yyy`), - # to avoid confusion we strip the `=` when the data is initially - # encoded. Python's decoding function requires they be put back. - # Fortunately, it doesn't complain if there are too many, so we just - # attach two `=` on spec (there will never be more than 2, see - # https://en.wikipedia.org/wiki/Base64#Decoding_Base64_without_padding). - tracestate_json = from_base64(encoded_tracestate + "==") - - try: - assert tracestate_json is not None - inflated_tracestate = json.loads(tracestate_json) - except Exception as err: - logger.warning( - ( - "Unable to attach tracestate data to envelope header: {err}" - + "\nTracestate value is {encoded_tracestate}" - ).format(err=err, encoded_tracestate=encoded_tracestate), - ) - - return inflated_tracestate - - def _format_sql(cursor, sql): # type: (Any, str) -> Optional[str] @@ -403,15 +241,6 @@ def _format_sql(cursor, sql): return real_sql or to_string(sql) -def has_tracestate_enabled(span=None): - # type: (Optional[Span]) -> bool - - client = ((span and span.hub) or sentry_sdk.Hub.current).client - options = client and client.options - - return bool(options and options["_experiments"].get("propagate_tracestate")) - - def has_custom_measurements_enabled(): # type: () -> bool client = sentry_sdk.Hub.current.client diff --git a/tests/test_envelope.py b/tests/test_envelope.py index b6a3ddf8be..ac2494ae57 100644 --- a/tests/test_envelope.py +++ b/tests/test_envelope.py @@ -1,16 +1,8 @@ from sentry_sdk.envelope import Envelope from sentry_sdk.session import Session from sentry_sdk import capture_event -from sentry_sdk.tracing_utils import compute_tracestate_value import sentry_sdk.client -import pytest - -try: - from unittest import mock # python 3.3 and above -except ImportError: - import mock # python < 3.3 - def generate_transaction_item(): return { @@ -26,16 +18,15 @@ def generate_transaction_item(): "parent_span_id": None, "description": "", "op": "greeting.sniff", - "tracestate": compute_tracestate_value( - { - "trace_id": "12312012123120121231201212312012", - "environment": "dogpark", - "release": "off.leash.park", - "public_key": "dogsarebadatkeepingsecrets", - "user": {"id": 12312013, "segment": "bigs"}, - "transaction": "/interactions/other-dogs/new-dog", - } - ), + "dynamic_sampling_context": { + "trace_id": "12312012123120121231201212312012", + "sample_rate": "1.0", + "environment": "dogpark", + "release": "off.leash.park", + "public_key": "dogsarebadatkeepingsecrets", + "user_segment": "bigs", + "transaction": "/interactions/other-dogs/new-dog", + }, } }, "spans": [ @@ -88,23 +79,13 @@ def test_add_and_get_session(): assert item.payload.json == expected.to_json() -# TODO (kmclb) remove this parameterization once tracestate is a real feature -@pytest.mark.parametrize("tracestate_enabled", [True, False]) -def test_envelope_headers( - sentry_init, capture_envelopes, monkeypatch, tracestate_enabled -): +def test_envelope_headers(sentry_init, capture_envelopes, monkeypatch): monkeypatch.setattr( sentry_sdk.client, "format_timestamp", lambda x: "2012-11-21T12:31:12.415908Z", ) - monkeypatch.setattr( - sentry_sdk.client, - "has_tracestate_enabled", - mock.Mock(return_value=tracestate_enabled), - ) - sentry_init( dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", ) @@ -114,24 +95,10 @@ def test_envelope_headers( assert len(envelopes) == 1 - if tracestate_enabled: - assert envelopes[0].headers == { - "event_id": "15210411201320122115110420122013", - "sent_at": "2012-11-21T12:31:12.415908Z", - "trace": { - "trace_id": "12312012123120121231201212312012", - "environment": "dogpark", - "release": "off.leash.park", - "public_key": "dogsarebadatkeepingsecrets", - "user": {"id": 12312013, "segment": "bigs"}, - "transaction": "/interactions/other-dogs/new-dog", - }, - } - else: - assert envelopes[0].headers == { - "event_id": "15210411201320122115110420122013", - "sent_at": "2012-11-21T12:31:12.415908Z", - } + assert envelopes[0].headers == { + "event_id": "15210411201320122115110420122013", + "sent_at": "2012-11-21T12:31:12.415908Z", + } def test_envelope_with_sized_items(): From 4399d933f8e27b5218b019c198944ebc5ea77054 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 16 Feb 2023 14:45:42 +0100 Subject: [PATCH 2/2] Cleaned up tests --- tests/test_envelope.py | 9 + tests/tracing/test_http_headers.py | 278 +----------------------- tests/tracing/test_integration_tests.py | 10 +- tests/tracing/test_misc.py | 17 -- 4 files changed, 14 insertions(+), 300 deletions(-) diff --git a/tests/test_envelope.py b/tests/test_envelope.py index ac2494ae57..136c0e4804 100644 --- a/tests/test_envelope.py +++ b/tests/test_envelope.py @@ -98,6 +98,15 @@ def test_envelope_headers(sentry_init, capture_envelopes, monkeypatch): assert envelopes[0].headers == { "event_id": "15210411201320122115110420122013", "sent_at": "2012-11-21T12:31:12.415908Z", + "trace": { + "trace_id": "12312012123120121231201212312012", + "sample_rate": "1.0", + "environment": "dogpark", + "release": "off.leash.park", + "public_key": "dogsarebadatkeepingsecrets", + "user_segment": "bigs", + "transaction": "/interactions/other-dogs/new-dog", + }, } diff --git a/tests/tracing/test_http_headers.py b/tests/tracing/test_http_headers.py index 3db967b24b..46af3c790e 100644 --- a/tests/tracing/test_http_headers.py +++ b/tests/tracing/test_http_headers.py @@ -1,16 +1,7 @@ -import json - import pytest -import sentry_sdk -from sentry_sdk.tracing import Transaction, Span -from sentry_sdk.tracing_utils import ( - compute_tracestate_value, - extract_sentrytrace_data, - extract_tracestate_data, - reinflate_tracestate, -) -from sentry_sdk.utils import from_base64, to_base64 +from sentry_sdk.tracing import Transaction +from sentry_sdk.tracing_utils import extract_sentrytrace_data try: @@ -19,139 +10,6 @@ import mock # python < 3.3 -def test_tracestate_computation(sentry_init): - sentry_init( - dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", - environment="dogpark", - release="off.leash.park", - ) - - sentry_sdk.set_user({"id": 12312013, "segment": "bigs"}) - - transaction = Transaction( - name="/interactions/other-dogs/new-dog", - op="greeting.sniff", - trace_id="12312012123120121231201212312012", - ) - - # force lazy computation to create a value - transaction.to_tracestate() - - computed_value = transaction._sentry_tracestate.replace("sentry=", "") - # we have to decode and reinflate the data because we can guarantee that the - # order of the entries in the jsonified dict will be the same here as when - # the tracestate is computed - reinflated_trace_data = json.loads(from_base64(computed_value)) - - assert reinflated_trace_data == { - "trace_id": "12312012123120121231201212312012", - "environment": "dogpark", - "release": "off.leash.park", - "public_key": "dogsarebadatkeepingsecrets", - "user": {"id": 12312013, "segment": "bigs"}, - "transaction": "/interactions/other-dogs/new-dog", - } - - -def test_doesnt_add_new_tracestate_to_transaction_when_none_given(sentry_init): - sentry_init( - dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", - environment="dogpark", - release="off.leash.park", - ) - - transaction = Transaction( - name="/interactions/other-dogs/new-dog", - op="greeting.sniff", - # sentry_tracestate=< value would be passed here > - ) - - assert transaction._sentry_tracestate is None - - -def test_adds_tracestate_to_transaction_when_to_traceparent_called(sentry_init): - sentry_init( - dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", - environment="dogpark", - release="off.leash.park", - ) - - transaction = Transaction( - name="/interactions/other-dogs/new-dog", - op="greeting.sniff", - ) - - # no inherited tracestate, and none created in Transaction constructor - assert transaction._sentry_tracestate is None - - transaction.to_tracestate() - - assert transaction._sentry_tracestate is not None - - -def test_adds_tracestate_to_transaction_when_getting_trace_context(sentry_init): - sentry_init( - dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", - environment="dogpark", - release="off.leash.park", - ) - - transaction = Transaction( - name="/interactions/other-dogs/new-dog", - op="greeting.sniff", - ) - - # no inherited tracestate, and none created in Transaction constructor - assert transaction._sentry_tracestate is None - - transaction.get_trace_context() - - assert transaction._sentry_tracestate is not None - - -@pytest.mark.parametrize( - "set_by", ["inheritance", "to_tracestate", "get_trace_context"] -) -def test_tracestate_is_immutable_once_set(sentry_init, monkeypatch, set_by): - monkeypatch.setattr( - sentry_sdk.tracing, - "compute_tracestate_entry", - mock.Mock(return_value="sentry=doGsaREgReaT"), - ) - - sentry_init( - dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", - environment="dogpark", - release="off.leash.park", - ) - - # for each scenario, get to the point where tracestate has been set - if set_by == "inheritance": - transaction = Transaction( - name="/interactions/other-dogs/new-dog", - op="greeting.sniff", - sentry_tracestate=("sentry=doGsaREgReaT"), - ) - else: - transaction = Transaction( - name="/interactions/other-dogs/new-dog", - op="greeting.sniff", - ) - - if set_by == "to_tracestate": - transaction.to_tracestate() - if set_by == "get_trace_context": - transaction.get_trace_context() - - assert transaction._sentry_tracestate == "sentry=doGsaREgReaT" - - # user data would be included in tracestate if it were recomputed at this point - sentry_sdk.set_user({"id": 12312013, "segment": "bigs"}) - - # value hasn't changed - assert transaction._sentry_tracestate == "sentry=doGsaREgReaT" - - @pytest.mark.parametrize("sampled", [True, False, None]) def test_to_traceparent(sentry_init, sampled): @@ -172,50 +30,6 @@ def test_to_traceparent(sentry_init, sampled): ) -def test_to_tracestate(sentry_init): - sentry_init( - dsn="https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012", - environment="dogpark", - release="off.leash.park", - ) - - # it correctly uses the value from the transaction itself or the span's - # containing transaction - transaction_no_third_party = Transaction( - trace_id="12312012123120121231201212312012", - sentry_tracestate="sentry=doGsaREgReaT", - ) - non_orphan_span = Span() - non_orphan_span._containing_transaction = transaction_no_third_party - assert transaction_no_third_party.to_tracestate() == "sentry=doGsaREgReaT" - assert non_orphan_span.to_tracestate() == "sentry=doGsaREgReaT" - - # it combines sentry and third-party values correctly - transaction_with_third_party = Transaction( - trace_id="12312012123120121231201212312012", - sentry_tracestate="sentry=doGsaREgReaT", - third_party_tracestate="maisey=silly", - ) - assert ( - transaction_with_third_party.to_tracestate() - == "sentry=doGsaREgReaT,maisey=silly" - ) - - # it computes a tracestate from scratch for orphan transactions - orphan_span = Span( - trace_id="12312012123120121231201212312012", - ) - assert orphan_span._containing_transaction is None - assert orphan_span.to_tracestate() == "sentry=" + compute_tracestate_value( - { - "trace_id": "12312012123120121231201212312012", - "environment": "dogpark", - "release": "off.leash.park", - "public_key": "dogsarebadatkeepingsecrets", - } - ) - - @pytest.mark.parametrize("sampling_decision", [True, False]) def test_sentrytrace_extraction(sampling_decision): sentrytrace_header = "12312012123120121231201212312012-0415201309082013-{}".format( @@ -228,78 +42,12 @@ def test_sentrytrace_extraction(sampling_decision): } -@pytest.mark.parametrize( - ("incoming_header", "expected_sentry_value", "expected_third_party"), - [ - # sentry only - ("sentry=doGsaREgReaT", "sentry=doGsaREgReaT", None), - # sentry only, invalid (`!` isn't a valid base64 character) - ("sentry=doGsaREgReaT!", None, None), - # stuff before - ("maisey=silly,sentry=doGsaREgReaT", "sentry=doGsaREgReaT", "maisey=silly"), - # stuff after - ("sentry=doGsaREgReaT,maisey=silly", "sentry=doGsaREgReaT", "maisey=silly"), - # stuff before and after - ( - "charlie=goofy,sentry=doGsaREgReaT,maisey=silly", - "sentry=doGsaREgReaT", - "charlie=goofy,maisey=silly", - ), - # multiple before - ( - "charlie=goofy,maisey=silly,sentry=doGsaREgReaT", - "sentry=doGsaREgReaT", - "charlie=goofy,maisey=silly", - ), - # multiple after - ( - "sentry=doGsaREgReaT,charlie=goofy,maisey=silly", - "sentry=doGsaREgReaT", - "charlie=goofy,maisey=silly", - ), - # multiple before and after - ( - "charlie=goofy,maisey=silly,sentry=doGsaREgReaT,bodhi=floppy,cory=loyal", - "sentry=doGsaREgReaT", - "charlie=goofy,maisey=silly,bodhi=floppy,cory=loyal", - ), - # only third-party data - ("maisey=silly", None, "maisey=silly"), - # invalid third-party data, valid sentry data - ("maisey_is_silly,sentry=doGsaREgReaT", "sentry=doGsaREgReaT", None), - # valid third-party data, invalid sentry data - ("maisey=silly,sentry=doGsaREgReaT!", None, "maisey=silly"), - # nothing valid at all - ("maisey_is_silly,sentry=doGsaREgReaT!", None, None), - ], -) -def test_tracestate_extraction( - incoming_header, expected_sentry_value, expected_third_party -): - assert extract_tracestate_data(incoming_header) == { - "sentry_tracestate": expected_sentry_value, - "third_party_tracestate": expected_third_party, - } - - -# TODO (kmclb) remove this parameterization once tracestate is a real feature -@pytest.mark.parametrize("tracestate_enabled", [True, False]) -def test_iter_headers(sentry_init, monkeypatch, tracestate_enabled): +def test_iter_headers(sentry_init, monkeypatch): monkeypatch.setattr( Transaction, "to_traceparent", mock.Mock(return_value="12312012123120121231201212312012-0415201309082013-0"), ) - monkeypatch.setattr( - Transaction, - "to_tracestate", - mock.Mock(return_value="sentry=doGsaREgReaT,charlie=goofy"), - ) - monkeypatch.setattr( - sentry_sdk.tracing, - "has_tracestate_enabled", - mock.Mock(return_value=tracestate_enabled), - ) transaction = Transaction( name="/interactions/other-dogs/new-dog", @@ -310,23 +58,3 @@ def test_iter_headers(sentry_init, monkeypatch, tracestate_enabled): assert ( headers["sentry-trace"] == "12312012123120121231201212312012-0415201309082013-0" ) - if tracestate_enabled: - assert "tracestate" in headers - assert headers["tracestate"] == "sentry=doGsaREgReaT,charlie=goofy" - else: - assert "tracestate" not in headers - - -@pytest.mark.parametrize( - "data", - [ # comes out with no trailing `=` - {"name": "Maisey", "birthday": "12/31/12"}, - # comes out with one trailing `=` - {"dogs": "yes", "cats": "maybe"}, - # comes out with two trailing `=` - {"name": "Charlie", "birthday": "11/21/12"}, - ], -) -def test_tracestate_reinflation(data): - encoded_tracestate = to_base64(json.dumps(data)).strip("=") - assert reinflate_tracestate(encoded_tracestate) == data diff --git a/tests/tracing/test_integration_tests.py b/tests/tracing/test_integration_tests.py index f42df1091b..bf5cabdb64 100644 --- a/tests/tracing/test_integration_tests.py +++ b/tests/tracing/test_integration_tests.py @@ -63,13 +63,9 @@ def test_continue_from_headers(sentry_init, capture_envelopes, sampled, sample_r envelopes = capture_envelopes() # make a parent transaction (normally this would be in a different service) - with start_transaction( - name="hi", sampled=True if sample_rate == 0 else None - ) as parent_transaction: + with start_transaction(name="hi", sampled=True if sample_rate == 0 else None): with start_span() as old_span: old_span.sampled = sampled - tracestate = parent_transaction._sentry_tracestate - headers = dict(Hub.current.iter_trace_propagation_headers(old_span)) headers["baggage"] = ( "other-vendor-value-1=foo;bar;baz, " @@ -79,8 +75,7 @@ def test_continue_from_headers(sentry_init, capture_envelopes, sampled, sample_r "other-vendor-value-2=foo;bar;" ) - # child transaction, to prove that we can read 'sentry-trace' and - # `tracestate` header data correctly + # child transaction, to prove that we can read 'sentry-trace' header data correctly child_transaction = Transaction.continue_from_headers(headers, name="WRONG") assert child_transaction is not None assert child_transaction.parent_sampled == sampled @@ -88,7 +83,6 @@ def test_continue_from_headers(sentry_init, capture_envelopes, sampled, sample_r assert child_transaction.same_process_as_parent is False assert child_transaction.parent_span_id == old_span.span_id assert child_transaction.span_id != old_span.span_id - assert child_transaction._sentry_tracestate == tracestate baggage = child_transaction._baggage assert baggage diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index b51b5dcddb..3200c48a16 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -6,7 +6,6 @@ import sentry_sdk from sentry_sdk import Hub, start_span, start_transaction from sentry_sdk.tracing import Span, Transaction -from sentry_sdk.tracing_utils import has_tracestate_enabled try: from unittest import mock # python 3.3 and above @@ -232,22 +231,6 @@ def test_circular_references(monkeypatch, sentry_init, request): assert gc.collect() == 0 -# TODO (kmclb) remove this test once tracestate is a real feature -@pytest.mark.parametrize("tracestate_enabled", [True, False, None]) -def test_has_tracestate_enabled(sentry_init, tracestate_enabled): - experiments = ( - {"propagate_tracestate": tracestate_enabled} - if tracestate_enabled is not None - else {} - ) - sentry_init(_experiments=experiments) - - if tracestate_enabled is True: - assert has_tracestate_enabled() is True - else: - assert has_tracestate_enabled() is False - - def test_set_meaurement(sentry_init, capture_events): sentry_init(traces_sample_rate=1.0, _experiments={"custom_measurements": True})