From 1957fff5f8aa8aa33d90da85f16e50f6d29b450b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 8 Oct 2024 15:28:22 +0200 Subject: [PATCH] Make transaction names work * while sending the event in the final payload, the transaction name will be picked up from the attributes `sentry.name` or fallback to the `description` when we only have otel instrumentation * for populating DSC in the head case, we are doing a best attempt solution and fetching the transaction name/source from the current and isolation scopes * NOTE that there are cases where this will be inaccurate if we never set the transaction name on the scope in some integration, so we will go through and fix those cases separately. --- .../integrations/opentelemetry/consts.py | 1 + .../opentelemetry/potel_span_processor.py | 7 ++-- .../integrations/opentelemetry/utils.py | 36 +++++++++++++++++-- sentry_sdk/scope.py | 10 ++++++ sentry_sdk/tracing.py | 18 ++++++---- tests/integrations/flask/test_flask.py | 2 ++ 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/consts.py b/sentry_sdk/integrations/opentelemetry/consts.py index a71e304cf5..6d7c91f3f1 100644 --- a/sentry_sdk/integrations/opentelemetry/consts.py +++ b/sentry_sdk/integrations/opentelemetry/consts.py @@ -28,4 +28,5 @@ class SentrySpanAttribute: MEASUREMENT = "sentry.measurement" TAG = "sentry.tag" NAME = "sentry.name" + SOURCE = "sentry.source" CONTEXT = "sentry.context" diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index 0076743245..ed31608516 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -18,6 +18,7 @@ convert_from_otel_timestamp, extract_span_attributes, extract_span_data, + extract_transaction_name_source, get_trace_context, ) from sentry_sdk.integrations.opentelemetry.consts import ( @@ -137,6 +138,7 @@ def _root_span_to_transaction_event(self, span): if event is None: return None + transaction_name, transaction_source = extract_transaction_name_source(span) span_data = extract_span_data(span) (_, description, _, http_status, _) = span_data @@ -152,9 +154,8 @@ def _root_span_to_transaction_event(self, span): event.update( { "type": "transaction", - "transaction": description, - # TODO-neel-potel tx source based on integration - "transaction_info": {"source": "custom"}, + "transaction": transaction_name or description, + "transaction_info": {"source": transaction_source or "custom"}, "contexts": contexts, } ) diff --git a/sentry_sdk/integrations/opentelemetry/utils.py b/sentry_sdk/integrations/opentelemetry/utils.py index ddc9f0c25a..dc0fff7f8c 100644 --- a/sentry_sdk/integrations/opentelemetry/utils.py +++ b/sentry_sdk/integrations/opentelemetry/utils.py @@ -19,7 +19,7 @@ from sentry_sdk.utils import Dsn from sentry_sdk.consts import SPANSTATUS, OP from sentry_sdk.tracing import get_span_status_from_http_code, DEFAULT_SPAN_ORIGIN -from sentry_sdk.tracing_utils import Baggage +from sentry_sdk.tracing_utils import Baggage, LOW_QUALITY_TRANSACTION_SOURCES from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute from sentry_sdk._types import TYPE_CHECKING @@ -98,6 +98,16 @@ def convert_to_otel_timestamp(time): return int(time * 1e9) +def extract_transaction_name_source(span): + # type: (ReadableSpan) -> tuple[Optional[str], Optional[str]] + if not span.attributes: + return (None, None) + return ( + cast("Optional[str]", span.attributes.get(SentrySpanAttribute.NAME)), + cast("Optional[str]", span.attributes.get(SentrySpanAttribute.SOURCE)), + ) + + def extract_span_data(span): # type: (ReadableSpan) -> OtelExtractedSpanData op = span.name @@ -394,5 +404,27 @@ def get_trace_state(span): Baggage.SENTRY_PREFIX + "public_key", Dsn(options["dsn"]).public_key ) - # TODO-neel-potel head dsc transaction name + # we cannot access the root span in most cases here, so we HAVE to rely on the + # scopes to carry the correct transaction name/source. + # IDEALLY we will always move to using the isolation scope here + # but our integrations do all kinds of stuff with both isolation and current + # so I am keeping both for now as a best attempt solution till we get to a better state. + isolation_scope = sentry_sdk.get_isolation_scope() + current_scope = sentry_sdk.get_current_scope() + if ( + current_scope.transaction_name + and current_scope.transaction_source not in LOW_QUALITY_TRANSACTION_SOURCES + ): + trace_state = trace_state.update( + Baggage.SENTRY_PREFIX + "transaction", current_scope.transaction_name + ) + elif ( + isolation_scope.transaction_name + and isolation_scope.transaction_source + not in LOW_QUALITY_TRANSACTION_SOURCES + ): + trace_state = trace_state.update( + Baggage.SENTRY_PREFIX + "transaction", isolation_scope.transaction_name + ) + return trace_state diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index fcc870de95..09a7f9ee89 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -741,6 +741,16 @@ def set_transaction_name(self, name, source=None): if source: self._transaction_info["source"] = source + @property + def transaction_name(self): + # type: () -> Optional[str] + return self._transaction + + @property + def transaction_source(self): + # type: () -> Optional[str] + return self._transaction_info.get("source") + @_attr_setter def user(self, value): # type: (Optional[Dict[str, Any]]) -> None diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 5b0795af62..d4033b52ef 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1292,7 +1292,7 @@ def description(self, value): from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute if value is not None: - self._otel_span.set_attribute(SentrySpanAttribute.DESCRIPTION, value) + self.set_attribute(SentrySpanAttribute.DESCRIPTION, value) @property def origin(self): @@ -1307,7 +1307,7 @@ def origin(self, value): from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute if value is not None: - self._otel_span.set_attribute(SentrySpanAttribute.ORIGIN, value) + self.set_attribute(SentrySpanAttribute.ORIGIN, value) @property def containing_transaction(self): @@ -1380,7 +1380,7 @@ def op(self, value): from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute if value is not None: - self._otel_span.set_attribute(SentrySpanAttribute.OP, value) + self.set_attribute(SentrySpanAttribute.OP, value) @property def name(self): @@ -1395,17 +1395,23 @@ def name(self, value): from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute if value is not None: - self._otel_span.set_attribute(SentrySpanAttribute.NAME, value) + self.set_attribute(SentrySpanAttribute.NAME, value) @property def source(self): # type: () -> str - pass + from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute + + return ( + self.get_attribute(SentrySpanAttribute.SOURCE) or TRANSACTION_SOURCE_CUSTOM + ) @source.setter def source(self, value): # type: (str) -> None - pass + from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute + + self.set_attribute(SentrySpanAttribute.SOURCE, value) @property def start_timestamp(self): diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py index 6febb12b8b..4e92df7e7c 100644 --- a/tests/integrations/flask/test_flask.py +++ b/tests/integrations/flask/test_flask.py @@ -751,12 +751,14 @@ def hi_tx(): assert transaction_event["type"] == "transaction" assert transaction_event["transaction"] == "hi_tx" + assert transaction_event["transaction_info"] == {"source": "component"} assert transaction_event["contexts"]["trace"]["status"] == "ok" assert transaction_event["tags"]["view"] == "yes" assert transaction_event["tags"]["before_request"] == "yes" assert message_event["message"] == "hi" assert message_event["transaction"] == "hi_tx" + assert message_event["transaction_info"] == {"source": "component"} assert message_event["tags"]["view"] == "yes" assert message_event["tags"]["before_request"] == "yes"