From 28effd69809ee010b0e8cc64096be57a7942c4d2 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 10 Jun 2024 20:51:07 +0200 Subject: [PATCH 01/10] Skeletons for new components --- .../opentelemetry/contextvars_context.py | 14 ++++++ .../integrations/opentelemetry/integration.py | 15 +++++- .../opentelemetry/potel_span_exporter.py | 19 ++++++++ .../opentelemetry/potel_span_processor.py | 47 +++++++++++++++++++ 4 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 sentry_sdk/integrations/opentelemetry/contextvars_context.py create mode 100644 sentry_sdk/integrations/opentelemetry/potel_span_exporter.py create mode 100644 sentry_sdk/integrations/opentelemetry/potel_span_processor.py diff --git a/sentry_sdk/integrations/opentelemetry/contextvars_context.py b/sentry_sdk/integrations/opentelemetry/contextvars_context.py new file mode 100644 index 0000000000..7a382064c9 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/contextvars_context.py @@ -0,0 +1,14 @@ +from opentelemetry.context.context import Context # type: ignore +from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext # type: ignore + + +class SentryContextVarsRuntimeContext(ContextVarsRuntimeContext): # type: ignore + def attach(self, context): + # type: (Context) -> object + # TODO-neel-potel do scope management + return super().attach(context) + + def detach(self, token): + # type: (object) -> None + # TODO-neel-potel not sure if we need anything here, see later + super().detach(token) diff --git a/sentry_sdk/integrations/opentelemetry/integration.py b/sentry_sdk/integrations/opentelemetry/integration.py index 9e62d1feca..3a33c7f2d0 100644 --- a/sentry_sdk/integrations/opentelemetry/integration.py +++ b/sentry_sdk/integrations/opentelemetry/integration.py @@ -8,7 +8,12 @@ from importlib import import_module from sentry_sdk.integrations import DidNotEnable, Integration -from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor +from sentry_sdk.integrations.opentelemetry.potel_span_processor import ( + PotelSentrySpanProcessor, +) +from sentry_sdk.integrations.opentelemetry.contextvars_context import ( + SentryContextVarsRuntimeContext, +) from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator from sentry_sdk.utils import logger, _get_installed_modules from sentry_sdk._types import TYPE_CHECKING @@ -21,6 +26,7 @@ ) from opentelemetry.propagate import set_global_textmap # type: ignore from opentelemetry.sdk.trace import TracerProvider # type: ignore + from opentelemetry import context except ImportError: raise DidNotEnable("opentelemetry not installed") @@ -165,9 +171,14 @@ def _import_by_path(path): def _setup_sentry_tracing(): # type: () -> None + + # TODO-neel-potel make sure lifecycle is correct + # TODO-neel-potel contribute upstream so this is not necessary + context._RUNTIME_CONTEXT = SentryContextVarsRuntimeContext() + provider = TracerProvider() - provider.add_span_processor(SentrySpanProcessor()) + provider.add_span_processor(PotelSentrySpanProcessor()) trace.set_tracer_provider(provider) diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py new file mode 100644 index 0000000000..70cfc39105 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py @@ -0,0 +1,19 @@ +from opentelemetry.trace import Span # type: ignore + + +class PotelSentrySpanExporter: + """ + A Sentry-specific exporter that converts OpenTelemetry Spans to Sentry Spans & Transactions. + """ + + def __init__(self): + # type: () -> None + pass + + def export(self, span): + # type: (Span) -> None + pass + + def flush(self, timeout_millis): + # type: (int) -> bool + return True diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py new file mode 100644 index 0000000000..fef0491a10 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -0,0 +1,47 @@ +from opentelemetry.sdk.trace import SpanProcessor # type: ignore +from opentelemetry.context import Context # type: ignore +from opentelemetry.trace import Span # type: ignore + +from sentry_sdk.integrations.opentelemetry.potel_span_exporter import ( + PotelSentrySpanExporter, +) +from sentry_sdk._types import TYPE_CHECKING + +if TYPE_CHECKING: + from typing import Optional + + +class PotelSentrySpanProcessor(SpanProcessor): # type: ignore + """ + Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. + """ + + def __new__(cls): + # type: () -> PotelSentrySpanProcessor + if not hasattr(cls, "instance"): + cls.instance = super().__new__(cls) + + return cls.instance + + def __init__(self): + # type: () -> None + self._exporter = PotelSentrySpanExporter() + + def on_start(self, span, parent_context=None): + # type: (Span, Optional[Context]) -> None + pass + + def on_end(self, span): + # type: (Span) -> None + self._exporter.export(span) + + # TODO-neel-potel not sure we need a clear like JS + def shutdown(self): + # type: () -> None + pass + + # TODO-neel-potel change default? this is 30 sec + # TODO-neel-potel call this in client.flush + def force_flush(self, timeout_millis=30000): + # type: (int) -> bool + return self._exporter.flush(timeout_millis) From e7cbb59b7adc0e545b64f5d6393e107773abc83c Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 12 Jun 2024 13:36:37 +0200 Subject: [PATCH 02/10] Add simple scope management whenever a context is attached * create a new otel context `_SCOPES_KEY` that will hold a tuple of `(curent_scope, isolation_scope)` * the `current_scope` will always be forked (like on every span creation/context update in practice) * note that this is on `attach`, so not on all copy-on-write context object creation but only on apis such as [`trace.use_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L547) or [`tracer.start_as_current_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L329) * basically every otel `context` fork corresponds to our `current_scope` fork * the `isolation_scope` currently will not be forked * these will later be updated, for instance when we update our top level scope apis that fork isolation scope, that will also have a corresponding change in this `attach` function --- .../opentelemetry/contextvars_context.py | 26 ++++++++++++++----- .../integrations/opentelemetry/integration.py | 1 - 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/contextvars_context.py b/sentry_sdk/integrations/opentelemetry/contextvars_context.py index 7a382064c9..3291fca448 100644 --- a/sentry_sdk/integrations/opentelemetry/contextvars_context.py +++ b/sentry_sdk/integrations/opentelemetry/contextvars_context.py @@ -1,14 +1,26 @@ -from opentelemetry.context.context import Context # type: ignore +from opentelemetry.context import Context, create_key, get_value, set_value from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext # type: ignore +from sentry_sdk.scope import Scope + + +_SCOPES_KEY = create_key("sentry_scopes") + class SentryContextVarsRuntimeContext(ContextVarsRuntimeContext): # type: ignore def attach(self, context): # type: (Context) -> object - # TODO-neel-potel do scope management - return super().attach(context) + scopes = get_value(_SCOPES_KEY, context) + + if scopes and isinstance(scopes, tuple): + (current_scope, isolation_scope) = scopes + else: + current_scope = Scope.get_current_scope() + isolation_scope = Scope.get_isolation_scope() + + # TODO-neel-potel fork isolation_scope too like JS + # once we setup our own apis to pass through to otel + new_scopes = (current_scope.fork(), isolation_scope) + new_context = set_value(_SCOPES_KEY, new_scopes, context) - def detach(self, token): - # type: (object) -> None - # TODO-neel-potel not sure if we need anything here, see later - super().detach(token) + return super().attach(new_context) diff --git a/sentry_sdk/integrations/opentelemetry/integration.py b/sentry_sdk/integrations/opentelemetry/integration.py index 3a33c7f2d0..28a497c340 100644 --- a/sentry_sdk/integrations/opentelemetry/integration.py +++ b/sentry_sdk/integrations/opentelemetry/integration.py @@ -172,7 +172,6 @@ def _import_by_path(path): def _setup_sentry_tracing(): # type: () -> None - # TODO-neel-potel make sure lifecycle is correct # TODO-neel-potel contribute upstream so this is not necessary context._RUNTIME_CONTEXT = SentryContextVarsRuntimeContext() From 618d6ca9a76441df411fa8c26c18d38b3882c4a2 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 12 Jun 2024 14:19:57 +0200 Subject: [PATCH 03/10] Don't parse DSN twice --- .../integrations/opentelemetry/span_processor.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index a09a93d284..e5dc86c53a 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -119,11 +119,6 @@ def on_start(self, otel_span, parent_context=None): if not client.dsn: return - try: - _ = Dsn(client.dsn) - except Exception: - return - if client.options["instrumenter"] != INSTRUMENTER.OTEL: return @@ -223,8 +218,12 @@ def _is_sentry_span(self, otel_span): dsn_url = None client = get_client() + if client.dsn: - dsn_url = Dsn(client.dsn).netloc + try: + dsn_url = Dsn(client.dsn).netloc + except Exception: + pass if otel_span_url and dsn_url in otel_span_url: return True From 7dba0299a7494b6a99ae06391f37eaa5f4c0f8ef Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 12 Jun 2024 18:02:00 +0200 Subject: [PATCH 04/10] wip --- .../opentelemetry/potel_span_exporter.py | 4 +-- .../opentelemetry/potel_span_processor.py | 18 ++++++++-- .../opentelemetry/span_processor.py | 26 ++------------- .../integrations/opentelemetry/utils.py | 33 +++++++++++++++++++ 4 files changed, 52 insertions(+), 29 deletions(-) create mode 100644 sentry_sdk/integrations/opentelemetry/utils.py diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py index 70cfc39105..ab36c1df10 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py @@ -1,4 +1,4 @@ -from opentelemetry.trace import Span # type: ignore +from opentelemetry.sdk.trace import ReadableSpan # type: ignore class PotelSentrySpanExporter: @@ -11,7 +11,7 @@ def __init__(self): pass def export(self, span): - # type: (Span) -> None + # type: (ReadableSpan) -> None pass def flush(self, timeout_millis): diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index fef0491a10..8cb1baa898 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -1,10 +1,12 @@ -from opentelemetry.sdk.trace import SpanProcessor # type: ignore +from opentelemetry.trace import INVALID_SPAN, get_current_span # type: ignore from opentelemetry.context import Context # type: ignore -from opentelemetry.trace import Span # type: ignore +from opentelemetry.sdk.trace import Span, ReadableSpan, SpanProcessor # type: ignore +from sentry_sdk.integrations.opentelemetry.utils import is_sentry_span from sentry_sdk.integrations.opentelemetry.potel_span_exporter import ( PotelSentrySpanExporter, ) + from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: @@ -30,9 +32,19 @@ def __init__(self): def on_start(self, span, parent_context=None): # type: (Span, Optional[Context]) -> None pass + # if is_sentry_span(span): + # return + + # parent_span = get_current_span(parent_context) + + # # TODO-neel-potel check remote logic with propagation and incoming trace later + # if parent_span != INVALID_SPAN: + # # TODO-neel once we add our apis, we might need to store references on the span + # # directly, see if we need to do this like JS + # pass def on_end(self, span): - # type: (Span) -> None + # type: (ReadableSpan) -> None self._exporter.export(span) # TODO-neel-potel not sure we need a clear like JS diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index e5dc86c53a..50fc603611 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -22,9 +22,9 @@ SENTRY_BAGGAGE_KEY, SENTRY_TRACE_KEY, ) +from sentry_sdk.integrations.opentelemetry.utils import is_sentry_span from sentry_sdk.scope import add_global_event_processor from sentry_sdk.tracing import Transaction, Span as SentrySpan -from sentry_sdk.utils import Dsn from sentry_sdk._types import TYPE_CHECKING from urllib3.util import parse_url as urlparse @@ -125,7 +125,7 @@ def on_start(self, otel_span, parent_context=None): if not otel_span.get_span_context().is_valid: return - if self._is_sentry_span(otel_span): + if is_sentry_span(otel_span): return trace_data = self._get_trace_data(otel_span, parent_context) @@ -208,28 +208,6 @@ def on_end(self, otel_span): self.open_spans.setdefault(span_start_in_minutes, set()).discard(span_id) self._prune_old_spans() - def _is_sentry_span(self, otel_span): - # type: (OTelSpan) -> bool - """ - Break infinite loop: - HTTP requests to Sentry are caught by OTel and send again to Sentry. - """ - otel_span_url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) - - dsn_url = None - client = get_client() - - if client.dsn: - try: - dsn_url = Dsn(client.dsn).netloc - except Exception: - pass - - if otel_span_url and dsn_url in otel_span_url: - return True - - return False - def _get_otel_context(self, otel_span): # type: (OTelSpan) -> Dict[str, Any] """ diff --git a/sentry_sdk/integrations/opentelemetry/utils.py b/sentry_sdk/integrations/opentelemetry/utils.py new file mode 100644 index 0000000000..bd827449f1 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/utils.py @@ -0,0 +1,33 @@ +from opentelemetry.semconv.trace import SpanAttributes # type: ignore +from opentelemetry.trace import Span # type: ignore + +from sentry_sdk import get_client, start_transaction +from sentry_sdk.utils import Dsn + +def is_sentry_span(span): + # type: (Span) -> bool + """ + Break infinite loop: + HTTP requests to Sentry are caught by OTel and send again to Sentry. + """ + span_url = span.attributes.get(SpanAttributes.HTTP_URL, None) + + if not span_url: + return False + + dsn_url = None + client = get_client() + + if client.dsn: + try: + dsn_url = Dsn(client.dsn).netloc + except Exception: + pass + + if not dsn_url: + return False + + if dsn_url in span_url: + return True + + return False From 1a35dae8e4e961a64f5270a078b52e975f38a0b7 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 10 Jun 2024 20:51:07 +0200 Subject: [PATCH 05/10] Skeletons for new components --- .../opentelemetry/potel_span_exporter.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 sentry_sdk/integrations/opentelemetry/potel_span_exporter.py diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py new file mode 100644 index 0000000000..70cfc39105 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py @@ -0,0 +1,19 @@ +from opentelemetry.trace import Span # type: ignore + + +class PotelSentrySpanExporter: + """ + A Sentry-specific exporter that converts OpenTelemetry Spans to Sentry Spans & Transactions. + """ + + def __init__(self): + # type: () -> None + pass + + def export(self, span): + # type: (Span) -> None + pass + + def flush(self, timeout_millis): + # type: (int) -> bool + return True From c52318219ede830d14c797165e557df61fc78832 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 12 Jun 2024 13:36:37 +0200 Subject: [PATCH 06/10] Add simple scope management whenever a context is attached * create a new otel context `_SCOPES_KEY` that will hold a tuple of `(curent_scope, isolation_scope)` * the `current_scope` will always be forked (like on every span creation/context update in practice) * note that this is on `attach`, so not on all copy-on-write context object creation but only on apis such as [`trace.use_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L547) or [`tracer.start_as_current_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L329) * basically every otel `context` fork corresponds to our `current_scope` fork * the `isolation_scope` currently will not be forked * these will later be updated, for instance when we update our top level scope apis that fork isolation scope, that will also have a corresponding change in this `attach` function --- .../opentelemetry/contextvars_context.py | 26 ++++++++++++++----- .../integrations/opentelemetry/integration.py | 1 - .../opentelemetry/potel_span_exporter.py | 19 -------------- 3 files changed, 19 insertions(+), 27 deletions(-) delete mode 100644 sentry_sdk/integrations/opentelemetry/potel_span_exporter.py diff --git a/sentry_sdk/integrations/opentelemetry/contextvars_context.py b/sentry_sdk/integrations/opentelemetry/contextvars_context.py index 7a382064c9..3291fca448 100644 --- a/sentry_sdk/integrations/opentelemetry/contextvars_context.py +++ b/sentry_sdk/integrations/opentelemetry/contextvars_context.py @@ -1,14 +1,26 @@ -from opentelemetry.context.context import Context # type: ignore +from opentelemetry.context import Context, create_key, get_value, set_value from opentelemetry.context.contextvars_context import ContextVarsRuntimeContext # type: ignore +from sentry_sdk.scope import Scope + + +_SCOPES_KEY = create_key("sentry_scopes") + class SentryContextVarsRuntimeContext(ContextVarsRuntimeContext): # type: ignore def attach(self, context): # type: (Context) -> object - # TODO-neel-potel do scope management - return super().attach(context) + scopes = get_value(_SCOPES_KEY, context) + + if scopes and isinstance(scopes, tuple): + (current_scope, isolation_scope) = scopes + else: + current_scope = Scope.get_current_scope() + isolation_scope = Scope.get_isolation_scope() + + # TODO-neel-potel fork isolation_scope too like JS + # once we setup our own apis to pass through to otel + new_scopes = (current_scope.fork(), isolation_scope) + new_context = set_value(_SCOPES_KEY, new_scopes, context) - def detach(self, token): - # type: (object) -> None - # TODO-neel-potel not sure if we need anything here, see later - super().detach(token) + return super().attach(new_context) diff --git a/sentry_sdk/integrations/opentelemetry/integration.py b/sentry_sdk/integrations/opentelemetry/integration.py index 3a33c7f2d0..28a497c340 100644 --- a/sentry_sdk/integrations/opentelemetry/integration.py +++ b/sentry_sdk/integrations/opentelemetry/integration.py @@ -172,7 +172,6 @@ def _import_by_path(path): def _setup_sentry_tracing(): # type: () -> None - # TODO-neel-potel make sure lifecycle is correct # TODO-neel-potel contribute upstream so this is not necessary context._RUNTIME_CONTEXT = SentryContextVarsRuntimeContext() diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py b/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py deleted file mode 100644 index 70cfc39105..0000000000 --- a/sentry_sdk/integrations/opentelemetry/potel_span_exporter.py +++ /dev/null @@ -1,19 +0,0 @@ -from opentelemetry.trace import Span # type: ignore - - -class PotelSentrySpanExporter: - """ - A Sentry-specific exporter that converts OpenTelemetry Spans to Sentry Spans & Transactions. - """ - - def __init__(self): - # type: () -> None - pass - - def export(self, span): - # type: (Span) -> None - pass - - def flush(self, timeout_millis): - # type: (int) -> bool - return True From 048acc93935c15f46757851614cac903954ef8a4 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 28 Jun 2024 13:24:04 +0200 Subject: [PATCH 07/10] working span processor --- .../opentelemetry/potel_span_processor.py | 42 +++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index 4c3272e4b1..00bf58252f 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -64,8 +64,14 @@ def _flush_root_span(self, span): if not transaction_event: return - children = self._collect_children(span) - # TODO add converted spans + spans = [] + for child in self._collect_children(span): + span_json = self._span_to_json(child) + if span_json: + spans.append(span_json) + transaction_event.setdefault("spans", []).extend(spans) + # TODO-neel-potel sort and cutoff max spans + capture_event(transaction_event) @@ -126,7 +132,37 @@ def _root_span_to_transaction_event(self, span): "contexts": contexts, "start_timestamp": convert_otel_timestamp(span.start_time), "timestamp": convert_otel_timestamp(span.end_time), - "spans": [], } # type: Event return event + + def _span_to_json(self, span): + # type: (ReadableSpan) -> Optional[dict[str, Any]] + if not span.context: + return None + if not span.start_time: + return None + if not span.end_time: + return None + + trace_id = format_trace_id(span.context.trace_id) + span_id = format_span_id(span.context.span_id) + parent_span_id = format_span_id(span.parent.span_id) if span.parent else None + + span_json = { + "trace_id": trace_id, + "span_id": span_id, + "origin": SPAN_ORIGIN, + "op": span.name, # TODO + "description": span.name, # TODO + "status": "ok", # TODO + "start_timestamp": convert_otel_timestamp(span.start_time), + "timestamp": convert_otel_timestamp(span.end_time), + } # type: dict[str, Any] + + if parent_span_id: + span_json["parent_span_id"] = parent_span_id + if span.attributes: + span_json["data"] = dict(span.attributes) + + return span_json From 8a08fb374f2f80ebfd50dd34e29b07b2888e6375 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 28 Jun 2024 13:31:30 +0200 Subject: [PATCH 08/10] lint --- .../opentelemetry/potel_span_processor.py | 38 +++++++++++-------- .../integrations/opentelemetry/utils.py | 9 ++++- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index 00bf58252f..bf11a51838 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -1,21 +1,26 @@ from collections import deque -from datetime import datetime -from opentelemetry.trace import INVALID_SPAN, get_current_span, format_trace_id, format_span_id +from opentelemetry.trace import format_trace_id, format_span_id from opentelemetry.context import Context from opentelemetry.sdk.trace import Span, ReadableSpan, SpanProcessor from sentry_sdk import capture_event -from sentry_sdk.integrations.opentelemetry.utils import is_sentry_span, convert_otel_timestamp -from sentry_sdk.integrations.opentelemetry.consts import OTEL_SENTRY_CONTEXT, SPAN_ORIGIN +from sentry_sdk.integrations.opentelemetry.utils import ( + is_sentry_span, + convert_otel_timestamp, +) +from sentry_sdk.integrations.opentelemetry.consts import ( + OTEL_SENTRY_CONTEXT, + SPAN_ORIGIN, +) from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: - from typing import Optional, List, Any + from typing import Optional, List, Any, Deque from sentry_sdk._types import Event -class PotelSentrySpanProcessor(SpanProcessor): # type: ignore +class PotelSentrySpanProcessor(SpanProcessor): """ Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. """ @@ -74,21 +79,22 @@ def _flush_root_span(self, span): capture_event(transaction_event) - def _collect_children(self, span): # type: (ReadableSpan) -> List[ReadableSpan] if not span.context: return [] children = [] - bfs_queue = deque() + bfs_queue = deque() # type: Deque[int] bfs_queue.append(span.context.span_id) while bfs_queue: parent_span_id = bfs_queue.popleft() node_children = self._children_spans.pop(parent_span_id, []) children.extend(node_children) - bfs_queue.extend([child.context.span_id for child in node_children if child.context]) + bfs_queue.extend( + [child.context.span_id for child in node_children if child.context] + ) return children @@ -112,8 +118,8 @@ def _root_span_to_transaction_event(self, span): "trace_id": trace_id, "span_id": span_id, "origin": SPAN_ORIGIN, - "op": span.name, # TODO - "status": "ok", # TODO + "op": span.name, # TODO + "status": "ok", # TODO } # type: dict[str, Any] if parent_span_id: @@ -127,8 +133,8 @@ def _root_span_to_transaction_event(self, span): event = { "type": "transaction", - "transaction": span.name, # TODO - "transaction_info": {"source": "custom"}, # TODO + "transaction": span.name, # TODO + "transaction_info": {"source": "custom"}, # TODO "contexts": contexts, "start_timestamp": convert_otel_timestamp(span.start_time), "timestamp": convert_otel_timestamp(span.end_time), @@ -153,9 +159,9 @@ def _span_to_json(self, span): "trace_id": trace_id, "span_id": span_id, "origin": SPAN_ORIGIN, - "op": span.name, # TODO - "description": span.name, # TODO - "status": "ok", # TODO + "op": span.name, # TODO + "description": span.name, # TODO + "status": "ok", # TODO "start_timestamp": convert_otel_timestamp(span.start_time), "timestamp": convert_otel_timestamp(span.end_time), } # type: dict[str, Any] diff --git a/sentry_sdk/integrations/opentelemetry/utils.py b/sentry_sdk/integrations/opentelemetry/utils.py index fd8c8e5721..95d6d0accd 100644 --- a/sentry_sdk/integrations/opentelemetry/utils.py +++ b/sentry_sdk/integrations/opentelemetry/utils.py @@ -4,7 +4,7 @@ from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.sdk.trace import ReadableSpan -from sentry_sdk import get_client, start_transaction +from sentry_sdk import get_client from sentry_sdk.utils import Dsn from sentry_sdk._types import TYPE_CHECKING @@ -12,12 +12,16 @@ if TYPE_CHECKING: from typing import Optional + def is_sentry_span(span): # type: (ReadableSpan) -> bool """ Break infinite loop: HTTP requests to Sentry are caught by OTel and send again to Sentry. """ + if not span.attributes: + return False + span_url = span.attributes.get(SpanAttributes.HTTP_URL, None) span_url = cast("Optional[str]", span_url) @@ -41,6 +45,7 @@ def is_sentry_span(span): return False + def convert_otel_timestamp(time): # type: (int) -> datetime - return datetime.fromtimestamp(time / 1e9, timezone.utc) + return datetime.fromtimestamp(time / 1e9, timezone.utc) From 2e2e5b9c2635774c95e9a7cec94ec6416789640b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 2 Jul 2024 14:49:14 +0200 Subject: [PATCH 09/10] Port over op/description/status extraction --- .../opentelemetry/potel_span_processor.py | 21 +++-- .../opentelemetry/span_processor.py | 84 +++---------------- .../integrations/opentelemetry/utils.py | 56 ++++++++++++- 3 files changed, 79 insertions(+), 82 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index bf11a51838..d2bc84811f 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -8,6 +8,7 @@ from sentry_sdk.integrations.opentelemetry.utils import ( is_sentry_span, convert_otel_timestamp, + extract_span_data, ) from sentry_sdk.integrations.opentelemetry.consts import ( OTEL_SENTRY_CONTEXT, @@ -100,7 +101,6 @@ def _collect_children(self, span): # we construct the event from scratch here # and not use the current Transaction class for easier refactoring - # TODO-neel-potel op, description, status logic def _root_span_to_transaction_event(self, span): # type: (ReadableSpan) -> Optional[Event] if not span.context: @@ -114,12 +114,14 @@ def _root_span_to_transaction_event(self, span): span_id = format_span_id(span.context.span_id) parent_span_id = format_span_id(span.parent.span_id) if span.parent else None + (op, description, _) = extract_span_data(span) + trace_context = { "trace_id": trace_id, "span_id": span_id, "origin": SPAN_ORIGIN, - "op": span.name, # TODO - "status": "ok", # TODO + "op": op, + "status": "ok", # TODO-neel-potel span status mapping } # type: dict[str, Any] if parent_span_id: @@ -133,8 +135,9 @@ def _root_span_to_transaction_event(self, span): event = { "type": "transaction", - "transaction": span.name, # TODO - "transaction_info": {"source": "custom"}, # TODO + "transaction": description, + # TODO-neel-potel tx source based on integration + "transaction_info": {"source": "custom"}, "contexts": contexts, "start_timestamp": convert_otel_timestamp(span.start_time), "timestamp": convert_otel_timestamp(span.end_time), @@ -155,13 +158,15 @@ def _span_to_json(self, span): span_id = format_span_id(span.context.span_id) parent_span_id = format_span_id(span.parent.span_id) if span.parent else None + (op, description, _) = extract_span_data(span) + span_json = { "trace_id": trace_id, "span_id": span_id, "origin": SPAN_ORIGIN, - "op": span.name, # TODO - "description": span.name, # TODO - "status": "ok", # TODO + "op": op, + "description": description, + "status": "ok", # TODO-neel-potel span status mapping "start_timestamp": convert_otel_timestamp(span.start_time), "timestamp": convert_otel_timestamp(span.end_time), } # type: dict[str, Any] diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 6a63a8a13b..14f89be9eb 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -4,12 +4,10 @@ from opentelemetry.context import get_value from opentelemetry.sdk.trace import SpanProcessor, ReadableSpan as OTelSpan -from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import ( format_span_id, format_trace_id, get_current_span, - SpanKind, ) from opentelemetry.trace.span import ( INVALID_SPAN_ID, @@ -23,12 +21,14 @@ OTEL_SENTRY_CONTEXT, SPAN_ORIGIN, ) -from sentry_sdk.integrations.opentelemetry.utils import is_sentry_span +from sentry_sdk.integrations.opentelemetry.utils import ( + is_sentry_span, + extract_span_data, +) from sentry_sdk.scope import add_global_event_processor from sentry_sdk.tracing import Transaction, Span as SentrySpan from sentry_sdk._types import TYPE_CHECKING -from urllib3.util import parse_url as urlparse if TYPE_CHECKING: from typing import Any, Optional, Union @@ -289,81 +289,19 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): """ sentry_span.set_data("otel.kind", otel_span.kind) - op = otel_span.name - description = otel_span.name - if otel_span.attributes is not None: for key, val in otel_span.attributes.items(): sentry_span.set_data(key, val) - http_method = otel_span.attributes.get(SpanAttributes.HTTP_METHOD) - http_method = cast("Optional[str]", http_method) - - db_query = otel_span.attributes.get(SpanAttributes.DB_SYSTEM) - - if http_method: - op = "http" - - if otel_span.kind == SpanKind.SERVER: - op += ".server" - elif otel_span.kind == SpanKind.CLIENT: - op += ".client" - - description = http_method - - peer_name = otel_span.attributes.get(SpanAttributes.NET_PEER_NAME, None) - if peer_name: - description += " {}".format(peer_name) - - target = otel_span.attributes.get(SpanAttributes.HTTP_TARGET, None) - if target: - description += " {}".format(target) - - if not peer_name and not target: - url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) - url = cast("Optional[str]", url) - if url: - parsed_url = urlparse(url) - url = "{}://{}{}".format( - parsed_url.scheme, parsed_url.netloc, parsed_url.path - ) - description += " {}".format(url) - - status_code = otel_span.attributes.get( - SpanAttributes.HTTP_STATUS_CODE, None - ) - status_code = cast("Optional[int]", status_code) - if status_code: - sentry_span.set_http_status(status_code) - - elif db_query: - op = "db" - statement = otel_span.attributes.get(SpanAttributes.DB_STATEMENT, None) - statement = cast("Optional[str]", statement) - if statement: - description = statement - + (op, description, status_code) = extract_span_data(otel_span) sentry_span.op = op sentry_span.description = description + if status_code: + sentry_span.set_http_status(status_code) def _update_transaction_with_otel_data(self, sentry_span, otel_span): # type: (SentrySpan, OTelSpan) -> None - if otel_span.attributes is None: - return - - http_method = otel_span.attributes.get(SpanAttributes.HTTP_METHOD) - - if http_method: - status_code = otel_span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) - status_code = cast("Optional[int]", status_code) - if status_code: - sentry_span.set_http_status(status_code) - - op = "http" - - if otel_span.kind == SpanKind.SERVER: - op += ".server" - elif otel_span.kind == SpanKind.CLIENT: - op += ".client" - - sentry_span.op = op + (op, _, status_code) = extract_span_data(otel_span) + sentry_span.op = op + if status_code: + sentry_span.set_http_status(status_code) diff --git a/sentry_sdk/integrations/opentelemetry/utils.py b/sentry_sdk/integrations/opentelemetry/utils.py index 95d6d0accd..1dc77ab150 100644 --- a/sentry_sdk/integrations/opentelemetry/utils.py +++ b/sentry_sdk/integrations/opentelemetry/utils.py @@ -1,8 +1,10 @@ from typing import cast from datetime import datetime, timezone +from opentelemetry.trace import SpanKind from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.sdk.trace import ReadableSpan +from urllib3.util import parse_url as urlparse from sentry_sdk import get_client from sentry_sdk.utils import Dsn @@ -10,7 +12,7 @@ from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: - from typing import Optional + from typing import Optional, Tuple def is_sentry_span(span): @@ -49,3 +51,55 @@ def is_sentry_span(span): def convert_otel_timestamp(time): # type: (int) -> datetime return datetime.fromtimestamp(time / 1e9, timezone.utc) + + +def extract_span_data(span): + # type: (ReadableSpan) -> Tuple[str, str, Optional[int]] + op = span.name + description = span.name + status_code = None + + if span.attributes is None: + return (op, description, status_code) + + http_method = span.attributes.get(SpanAttributes.HTTP_METHOD) + http_method = cast("Optional[str]", http_method) + db_query = span.attributes.get(SpanAttributes.DB_SYSTEM) + + if http_method: + op = "http" + if span.kind == SpanKind.SERVER: + op += ".server" + elif span.kind == SpanKind.CLIENT: + op += ".client" + + description = http_method + + peer_name = span.attributes.get(SpanAttributes.NET_PEER_NAME, None) + if peer_name: + description += " {}".format(peer_name) + + target = span.attributes.get(SpanAttributes.HTTP_TARGET, None) + if target: + description += " {}".format(target) + + if not peer_name and not target: + url = span.attributes.get(SpanAttributes.HTTP_URL, None) + url = cast("Optional[str]", url) + if url: + parsed_url = urlparse(url) + url = "{}://{}{}".format( + parsed_url.scheme, parsed_url.netloc, parsed_url.path + ) + description += " {}".format(url) + + status_code = span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) + elif db_query: + op = "db" + statement = span.attributes.get(SpanAttributes.DB_STATEMENT, None) + statement = cast("Optional[str]", statement) + if statement: + description = statement + + status_code = cast("Optional[int]", status_code) + return (op, description, status_code) From 2c29711223f6328497d8e98227b95bb2ec921067 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 2 Jul 2024 14:59:19 +0200 Subject: [PATCH 10/10] defaultdict --- .../opentelemetry/potel_span_processor.py | 12 +++++++----- .../opentelemetry/test_span_processor.py | 10 +++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index d2bc84811f..faa583a18d 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -1,4 +1,4 @@ -from collections import deque +from collections import deque, defaultdict from opentelemetry.trace import format_trace_id, format_span_id from opentelemetry.context import Context @@ -17,7 +17,7 @@ from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: - from typing import Optional, List, Any, Deque + from typing import Optional, List, Any, Deque, DefaultDict from sentry_sdk._types import Event @@ -35,7 +35,9 @@ def __new__(cls): def __init__(self): # type: () -> None - self._children_spans = {} # type: dict[int, List[ReadableSpan]] + self._children_spans = defaultdict( + list + ) # type: DefaultDict[int, List[ReadableSpan]] def on_start(self, span, parent_context=None): # type: (Span, Optional[Context]) -> None @@ -48,7 +50,7 @@ def on_end(self, span): # TODO-neel-potel-remote only take parent if not remote if span.parent: - self._children_spans.setdefault(span.parent.span_id, []).append(span) + self._children_spans[span.parent.span_id].append(span) else: # if have a root span ending, we build a transaction and send it self._flush_root_span(span) @@ -75,7 +77,7 @@ def _flush_root_span(self, span): span_json = self._span_to_json(child) if span_json: spans.append(span_json) - transaction_event.setdefault("spans", []).extend(spans) + transaction_event["spans"] = spans # TODO-neel-potel sort and cutoff max spans capture_event(transaction_event) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 8064e127f6..cc52735214 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -10,6 +10,7 @@ SentrySpanProcessor, link_trace_context_to_error_event, ) +from sentry_sdk.integrations.opentelemetry.utils import is_sentry_span from sentry_sdk.scope import Scope from sentry_sdk.tracing import Span, Transaction from sentry_sdk.tracing_utils import extract_sentrytrace_data @@ -18,25 +19,24 @@ def test_is_sentry_span(): otel_span = MagicMock() - span_processor = SentrySpanProcessor() - assert not span_processor._is_sentry_span(otel_span) + assert not is_sentry_span(otel_span) client = MagicMock() client.options = {"instrumenter": "otel"} client.dsn = "https://1234567890abcdef@o123456.ingest.sentry.io/123456" Scope.get_global_scope().set_client(client) - assert not span_processor._is_sentry_span(otel_span) + assert not is_sentry_span(otel_span) otel_span.attributes = { "http.url": "https://example.com", } - assert not span_processor._is_sentry_span(otel_span) + assert not is_sentry_span(otel_span) otel_span.attributes = { "http.url": "https://o123456.ingest.sentry.io/api/123/envelope", } - assert span_processor._is_sentry_span(otel_span) + assert is_sentry_span(otel_span) def test_get_otel_context():