From 60c53b7175faa866803cc7e0e6106e25acb6a7a4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Oct 2022 17:01:53 +0200 Subject: [PATCH 01/46] Added boilerplate for otel. --- sentry_sdk/integrations/opentelemetry/__init__.py | 3 +++ sentry_sdk/integrations/opentelemetry/span_processor.py | 9 +++++++++ 2 files changed, 12 insertions(+) create mode 100644 sentry_sdk/integrations/opentelemetry/__init__.py create mode 100644 sentry_sdk/integrations/opentelemetry/span_processor.py diff --git a/sentry_sdk/integrations/opentelemetry/__init__.py b/sentry_sdk/integrations/opentelemetry/__init__.py new file mode 100644 index 0000000000..2ec31af8c9 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/__init__.py @@ -0,0 +1,3 @@ +from sentry_sdk.integrations.opentelemetry.span_processor import ( # noqa: F401 + SentrySpanProcessor, +) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py new file mode 100644 index 0000000000..5cd4147987 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -0,0 +1,9 @@ +from opentelemetry.sdk.trace import SpanProcessor + + +class SentrySpanProcessor(SpanProcessor): + def on_start(self, span, parent_context=None): + print(f"[] in on_start {span}") + + def on_end(self, span): + print(f"~ in on_end {span}") From b575bfcf72e01fb3d34391e270ab6253cf36b184 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 21 Oct 2022 09:02:46 +0200 Subject: [PATCH 02/46] Added first version of a span processor --- .../opentelemetry/span_processor.py | 49 +++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 5cd4147987..b5c9fb8d66 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,9 +1,50 @@ from opentelemetry.sdk.trace import SpanProcessor +from sentry_sdk.hub import Hub +from sentry_sdk.tracing import Transaction + class SentrySpanProcessor(SpanProcessor): - def on_start(self, span, parent_context=None): - print(f"[] in on_start {span}") + otel_span_map = {} + + def on_start(self, otel_span, parent_context=None): + print(f"[SENTRY/OTEL] in on_start {otel_span}") + hub = Hub.current + sentry_span = None + parent_sentry_span = hub.scope.span + + if parent_sentry_span: + sentry_span = parent_sentry_span.start_child(op=otel_span.name) + else: + # TODO: add the baggagae sentry-trace stuff to transaction + sentry_span = hub.start_transaction(name=otel_span.name) + + hub.scope.span = sentry_span + self.otel_span_map[otel_span.context.span_id] = ( + sentry_span, + parent_sentry_span, + ) + + def on_end(self, otel_span): + print(f"[SENTRY/OTEL] in on_end {otel_span}") + hub = Hub.current + sentry_span, parent_sentry_span = self.otel_span_map[otel_span.context.span_id] + + if not sentry_span: + return + + sentry_span.op = otel_span.name + if isinstance(sentry_span, Transaction): + hub.scope.set_transaction_name(otel_span.name) + + for key in otel_span.attributes: + val = otel_span.attributes[key] + sentry_span.set_data(key, val) + + if key == "db.statement": + sentry_span.description = val + + sentry_span.finish() - def on_end(self, span): - print(f"~ in on_end {span}") + if parent_sentry_span: + hub.scope.span = parent_sentry_span From 4de5aaa7c19e11828b6f50f2e3d57ceed245bcd7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 21 Oct 2022 11:04:31 +0200 Subject: [PATCH 03/46] Trying to make a centralized noop for performance capturing functions. --- sentry_sdk/consts.py | 56 ++++++++++--------- sentry_sdk/hub.py | 14 ++++- .../opentelemetry/span_processor.py | 10 +++- sentry_sdk/integrations/stdlib.py | 3 + sentry_sdk/tracing.py | 42 ++++++++++++++ 5 files changed, 96 insertions(+), 29 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index a0d0184a72..1f54a519c2 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -44,6 +44,36 @@ DEFAULT_MAX_BREADCRUMBS = 100 +class INSTRUMENTER: + SENTRY = "sentry" + OTEL = "otel" + + +class OP: + DB = "db" + DB_REDIS = "db.redis" + EVENT_DJANGO = "event.django" + FUNCTION = "function" + FUNCTION_AWS = "function.aws" + FUNCTION_GCP = "function.gcp" + HTTP_CLIENT = "http.client" + HTTP_CLIENT_STREAM = "http.client.stream" + HTTP_SERVER = "http.server" + MIDDLEWARE_DJANGO = "middleware.django" + MIDDLEWARE_STARLETTE = "middleware.starlette" + MIDDLEWARE_STARLETTE_RECEIVE = "middleware.starlette.receive" + MIDDLEWARE_STARLETTE_SEND = "middleware.starlette.send" + QUEUE_SUBMIT_CELERY = "queue.submit.celery" + QUEUE_TASK_CELERY = "queue.task.celery" + QUEUE_TASK_RQ = "queue.task.rq" + SUBPROCESS = "subprocess" + SUBPROCESS_WAIT = "subprocess.wait" + SUBPROCESS_COMMUNICATE = "subprocess.communicate" + TEMPLATE_RENDER = "template.render" + VIEW_RENDER = "view.render" + WEBSOCKET_SERVER = "websocket.server" + + # This type exists to trick mypy and PyCharm into thinking `init` and `Client` # take these arguments (even though they take opaque **kwargs) class ClientConstructor(object): @@ -57,6 +87,7 @@ def __init__( server_name=None, # type: Optional[str] shutdown_timeout=2, # type: float integrations=[], # type: Sequence[Integration] # noqa: B006 + instrumenter=INSTRUMENTER.SENTRY, # type: Optional[str] in_app_include=[], # type: List[str] # noqa: B006 in_app_exclude=[], # type: List[str] # noqa: B006 default_integrations=True, # type: bool @@ -105,28 +136,3 @@ def _get_default_options(): VERSION = "1.9.10" - - -class OP: - DB = "db" - DB_REDIS = "db.redis" - EVENT_DJANGO = "event.django" - FUNCTION = "function" - FUNCTION_AWS = "function.aws" - FUNCTION_GCP = "function.gcp" - HTTP_CLIENT = "http.client" - HTTP_CLIENT_STREAM = "http.client.stream" - HTTP_SERVER = "http.server" - MIDDLEWARE_DJANGO = "middleware.django" - MIDDLEWARE_STARLETTE = "middleware.starlette" - MIDDLEWARE_STARLETTE_RECEIVE = "middleware.starlette.receive" - MIDDLEWARE_STARLETTE_SEND = "middleware.starlette.send" - QUEUE_SUBMIT_CELERY = "queue.submit.celery" - QUEUE_TASK_CELERY = "queue.task.celery" - QUEUE_TASK_RQ = "queue.task.rq" - SUBPROCESS = "subprocess" - SUBPROCESS_WAIT = "subprocess.wait" - SUBPROCESS_COMMUNICATE = "subprocess.communicate" - TEMPLATE_RENDER = "template.render" - VIEW_RENDER = "view.render" - WEBSOCKET_SERVER = "websocket.server" diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 3d4a28d526..402b03da59 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -5,9 +5,10 @@ from contextlib import contextmanager from sentry_sdk._compat import with_metaclass +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.scope import Scope from sentry_sdk.client import Client -from sentry_sdk.tracing import Span, Transaction +from sentry_sdk.tracing import NoOp, Span, Transaction from sentry_sdk.session import Session from sentry_sdk.utils import ( exc_info_from_error, @@ -464,6 +465,10 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ + if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: + logger.warn("start_span does NOTHING because instrumenter is OTEL") + return NoOp() + # TODO: consider removing this in a future release. # This is for backwards compatibility with releases before # start_transaction existed, to allow for a smoother transition. @@ -519,6 +524,13 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ + import ipdb + + ipdb.set_trace() + if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: + logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOp() + custom_sampling_context = kwargs.pop("custom_sampling_context", {}) # if we haven't been given a transaction, make one diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index b5c9fb8d66..537a0dfc17 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -14,10 +14,14 @@ def on_start(self, otel_span, parent_context=None): parent_sentry_span = hub.scope.span if parent_sentry_span: - sentry_span = parent_sentry_span.start_child(op=otel_span.name) + sentry_span = parent_sentry_span.start_child( + op="SENTRYOTEL-{}".format(otel_span.name) + ) else: # TODO: add the baggagae sentry-trace stuff to transaction - sentry_span = hub.start_transaction(name=otel_span.name) + sentry_span = hub.start_transaction( + name="SENTRYOTEL-{}".format(otel_span.name) + ) hub.scope.span = sentry_span self.otel_span_map[otel_span.context.span_id] = ( @@ -35,7 +39,7 @@ def on_end(self, otel_span): sentry_span.op = otel_span.name if isinstance(sentry_span, Transaction): - hub.scope.set_transaction_name(otel_span.name) + hub.scope.set_transaction_name("SENTRYOTEL-{}".format(otel_span.name)) for key in otel_span.attributes: val = otel_span.attributes[key] diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 3b81b6c2c5..30c63d3559 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -186,6 +186,9 @@ def sentry_patched_popen_init(self, *a, **kw): env = None + import ipdb + + ipdb.set_trace() with hub.start_span(op=OP.SUBPROCESS, description=description) as span: for k, v in hub.iter_trace_propagation_headers(span): diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index aacb3a5bb3..23a5c3263b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -6,6 +6,7 @@ from datetime import datetime, timedelta import sentry_sdk +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.utils import logger from sentry_sdk._types import MYPY @@ -75,6 +76,40 @@ def add(self, span): self.spans.append(span) +class NoOp: + def __init__(self, *args, **kwargs): + # type: (*Any, **Any) -> None + pass + + def __enter__(self): + # type: () -> Any + return self + + def __exit__(self, *args, **kwargs): + # type: (*Any, **Any) -> None + pass + + def __setattr__(self, *args, **kwargs): + import ipdb + + ipdb.set_trace() + pass + + def __getattr__(self, *args, **kwargs): + import ipdb + + ipdb.set_trace() + name = args[0] + if name in ["start_span", "start_transaction", "start_child"]: + + def nothing(*args, **kwargs): + return NoOp(*args, **kwargs) + + return nothing + + pass + + class Span(object): __slots__ = ( "trace_id", @@ -213,6 +248,13 @@ def start_child(self, **kwargs): trace id, sampling decision, transaction pointer, and span recorder are inherited from the current span/transaction. """ + import ipdb + + ipdb.set_trace() + if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: + logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOp() + kwargs.setdefault("sampled", self.sampled) child = Span( From 03c4c20b119955bb63a868083b68be056fddfacc Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 2 Nov 2022 09:41:06 +0100 Subject: [PATCH 04/46] Committing some otel changes from last week. --- sentry_sdk/hub.py | 29 ++++--- .../opentelemetry/span_processor.py | 52 ++++++++++-- sentry_sdk/integrations/stdlib.py | 4 - sentry_sdk/integrations/wsgi.py | 4 + sentry_sdk/tracing.py | 85 ++++++++++--------- 5 files changed, 111 insertions(+), 63 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 402b03da59..904a29ede2 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -8,7 +8,7 @@ from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.scope import Scope from sentry_sdk.client import Client -from sentry_sdk.tracing import NoOp, Span, Transaction +from sentry_sdk.tracing import NoOpSpan, Span, Transaction from sentry_sdk.session import Session from sentry_sdk.utils import ( exc_info_from_error, @@ -451,6 +451,7 @@ def add_breadcrumb( def start_span( self, span=None, # type: Optional[Span] + instrumenter=None, # type: Optional[str] **kwargs # type: Any ): # type: (...) -> Span @@ -465,9 +466,14 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ - if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: - logger.warn("start_span does NOTHING because instrumenter is OTEL") - return NoOp() + instrumenter = ( + instrumenter + if instrumenter is not None + else self.client.options["instrumenter"] + ) + if instrumenter == INSTRUMENTER.OTEL: + # logger.warn("start_span does NOTHING because instrumenter is OTEL") + return NoOpSpan() # TODO: consider removing this in a future release. # This is for backwards compatibility with releases before @@ -499,6 +505,7 @@ def start_span( def start_transaction( self, transaction=None, # type: Optional[Transaction] + instrumenter=None, # type: Optional[str] **kwargs # type: Any ): # type: (...) -> Transaction @@ -524,12 +531,14 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ - import ipdb - - ipdb.set_trace() - if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: - logger.warn("start_transaction does NOTHING because instrumenter is OTEL") - return NoOp() + instrumenter = ( + instrumenter + if instrumenter is not None + else self.client.options["instrumenter"] + ) + if instrumenter == INSTRUMENTER.OTEL: + # logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOpSpan() custom_sampling_context = kwargs.pop("custom_sampling_context", {}) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 537a0dfc17..dd129d6728 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -8,20 +8,41 @@ class SentrySpanProcessor(SpanProcessor): otel_span_map = {} def on_start(self, otel_span, parent_context=None): - print(f"[SENTRY/OTEL] in on_start {otel_span}") + # print(f"[SENTRY/OTEL] in on_start {otel_span}") + print(f"######### START SPAN: {otel_span.context.span_id} ({otel_span.name})") hub = Hub.current + print(f"HUB: {hub}") + if not hub: + return + + scope = hub.scope + print(f"SCOPE: {scope}") + if not scope: + return + + # TODO: isSentryRequest + sentry_span = None - parent_sentry_span = hub.scope.span + parent_sentry_span = scope.span + print(f"SPAN: {parent_sentry_span}") if parent_sentry_span: + print(f"######### START CHILD for PARENT: {parent_sentry_span}") sentry_span = parent_sentry_span.start_child( - op="SENTRYOTEL-{}".format(otel_span.name) + instrumenter="sentry", + op="SENTRYOTEL-{}".format(otel_span.name), ) + print(f"######### CHILD: {sentry_span}") else: # TODO: add the baggagae sentry-trace stuff to transaction + print( + f"######### START TRANSACTION: {otel_span.context.span_id} ({otel_span.name})" + ) sentry_span = hub.start_transaction( - name="SENTRYOTEL-{}".format(otel_span.name) + instrumenter="sentry", + name="SENTRYOTEL-{}".format(otel_span.name), ) + print(f"######### TRANSACTION SPAN: {sentry_span}") hub.scope.span = sentry_span self.otel_span_map[otel_span.context.span_id] = ( @@ -30,16 +51,32 @@ def on_start(self, otel_span, parent_context=None): ) def on_end(self, otel_span): - print(f"[SENTRY/OTEL] in on_end {otel_span}") + # print(f"[SENTRY/OTEL] in on_end {otel_span}") + print(f"######### END SPAN: {otel_span.context.span_id} ({otel_span.name})") + hub = Hub.current - sentry_span, parent_sentry_span = self.otel_span_map[otel_span.context.span_id] + print(f"HUB: {hub}") + if not hub: + return + scope = hub.scope + print(f"SCOPE: {scope}") + if not scope: + return + + sentry_span, parent_sentry_span = self.otel_span_map.pop( + otel_span.context.span_id + ) + print(f"SPAN: {sentry_span}") + print(f"PARENT SPAN: {parent_sentry_span}") if not sentry_span: return sentry_span.op = otel_span.name if isinstance(sentry_span, Transaction): hub.scope.set_transaction_name("SENTRYOTEL-{}".format(otel_span.name)) + # TODO: set otel context + # hub.scope.set_context(...) for key in otel_span.attributes: val = otel_span.attributes[key] @@ -51,4 +88,5 @@ def on_end(self, otel_span): sentry_span.finish() if parent_sentry_span: - hub.scope.span = parent_sentry_span + print(f"SET NEW CURRENT SPAN: {parent_sentry_span}") + scope.span = parent_sentry_span diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 30c63d3559..687d9dd2c1 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -186,11 +186,7 @@ def sentry_patched_popen_init(self, *a, **kw): env = None - import ipdb - - ipdb.set_trace() with hub.start_span(op=OP.SUBPROCESS, description=description) as span: - for k, v in hub.iter_trace_propagation_headers(span): if env is None: env = _init_argument( diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 03ce665489..22ed3cbd98 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -105,6 +105,10 @@ def __init__(self, app, use_x_forwarded_for=False): def __call__(self, environ, start_response): # type: (Dict[str, str], Callable[..., Any]) -> _ScopedResponse + import ipdb + + ipdb.set_trace() + if _wsgi_middleware_applied.get(False): return self.app(environ, start_response) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 23a5c3263b..ddaa227d66 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -76,40 +76,6 @@ def add(self, span): self.spans.append(span) -class NoOp: - def __init__(self, *args, **kwargs): - # type: (*Any, **Any) -> None - pass - - def __enter__(self): - # type: () -> Any - return self - - def __exit__(self, *args, **kwargs): - # type: (*Any, **Any) -> None - pass - - def __setattr__(self, *args, **kwargs): - import ipdb - - ipdb.set_trace() - pass - - def __getattr__(self, *args, **kwargs): - import ipdb - - ipdb.set_trace() - name = args[0] - if name in ["start_span", "start_transaction", "start_child"]: - - def nothing(*args, **kwargs): - return NoOp(*args, **kwargs) - - return nothing - - pass - - class Span(object): __slots__ = ( "trace_id", @@ -239,8 +205,8 @@ def containing_transaction(self): # referencing themselves) return self._containing_transaction - def start_child(self, **kwargs): - # type: (**Any) -> Span + def start_child(self, instrumenter=None, **kwargs): + # type: (Optional[str], **Any) -> Span """ Start a sub-span from the current span or transaction. @@ -248,12 +214,14 @@ def start_child(self, **kwargs): trace id, sampling decision, transaction pointer, and span recorder are inherited from the current span/transaction. """ - import ipdb - - ipdb.set_trace() - if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: - logger.warn("start_transaction does NOTHING because instrumenter is OTEL") - return NoOp() + instrumenter = ( + instrumenter + if instrumenter is not None + else self.client.options["instrumenter"] + ) + if instrumenter == INSTRUMENTER.OTEL: + # logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOpSpan() kwargs.setdefault("sampled", self.sampled) @@ -695,6 +663,7 @@ def finish(self, hub=None): # to a concrete decision. if self.sampled is None: logger.warning("Discarding transaction without sampling decision.") + return None finished_spans = [ @@ -863,6 +832,38 @@ def _set_initial_sampling_decision(self, sampling_context): ) +class NoOpSpan(Span): + def __repr__(self): + return self.__class__.__name__ + + def __enter__(self): + return self + + def __exit__(self, ty, value, tb): + pass + + def start_child(self, **kwargs): + pass + + def new_span(self, **kwargs): + pass + + def set_tag(self, key, value): + pass + + def set_data(self, key, value): + pass + + def set_status(self, value): + pass + + def set_http_status(self, http_status): + pass + + def finish(self, hub=None): + pass + + # Circular imports from sentry_sdk.tracing_utils import ( From e28d836d579d53acfabb66a2c11e71e1a5bedaea Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 10 Nov 2022 10:18:35 +0100 Subject: [PATCH 05/46] Cleanup --- .../opentelemetry/span_processor.py | 107 +++++++++++------- sentry_sdk/integrations/wsgi.py | 4 - 2 files changed, 63 insertions(+), 48 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index dd129d6728..b6bdb96010 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,92 +1,111 @@ from opentelemetry.sdk.trace import SpanProcessor +from opentelemetry.semconv.trace import SpanAttributes from sentry_sdk.hub import Hub from sentry_sdk.tracing import Transaction class SentrySpanProcessor(SpanProcessor): - otel_span_map = {} + """ + Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. + """ + + otel_span_map = {} # The mapping from otel span ids to sentry spans def on_start(self, otel_span, parent_context=None): - # print(f"[SENTRY/OTEL] in on_start {otel_span}") - print(f"######### START SPAN: {otel_span.context.span_id} ({otel_span.name})") hub = Hub.current - print(f"HUB: {hub}") if not hub: return - scope = hub.scope - print(f"SCOPE: {scope}") - if not scope: - return + # TODO: check for isSentryRequest and if sdk is initialized... - # TODO: isSentryRequest + span_id = otel_span.context.span_id + parent_span_id = otel_span.parent.span_id if otel_span.parent else None + trace_id = otel_span.context.trace_id - sentry_span = None - parent_sentry_span = scope.span - print(f"SPAN: {parent_sentry_span}") + parent_sentry_span = ( + self.otel_span_map[parent_span_id] if parent_span_id else None + ) + sentry_span = None if parent_sentry_span: - print(f"######### START CHILD for PARENT: {parent_sentry_span}") sentry_span = parent_sentry_span.start_child( + span_id=span_id, + description=otel_span.name, instrumenter="sentry", - op="SENTRYOTEL-{}".format(otel_span.name), ) - print(f"######### CHILD: {sentry_span}") else: # TODO: add the baggagae sentry-trace stuff to transaction - print( - f"######### START TRANSACTION: {otel_span.context.span_id} ({otel_span.name})" - ) sentry_span = hub.start_transaction( + name=otel_span.name, + span_id=span_id, + parent_span_id=parent_span_id, + trace_id=trace_id, + baggage={}, # TODO: add baggage + # TODO: check if we can set start_timestamp instrumenter="sentry", - name="SENTRYOTEL-{}".format(otel_span.name), ) - print(f"######### TRANSACTION SPAN: {sentry_span}") - hub.scope.span = sentry_span - self.otel_span_map[otel_span.context.span_id] = ( - sentry_span, - parent_sentry_span, - ) + self.otel_span_map[span_id] = sentry_span def on_end(self, otel_span): - # print(f"[SENTRY/OTEL] in on_end {otel_span}") - print(f"######### END SPAN: {otel_span.context.span_id} ({otel_span.name})") - hub = Hub.current - print(f"HUB: {hub}") if not hub: return scope = hub.scope - print(f"SCOPE: {scope}") if not scope: return - sentry_span, parent_sentry_span = self.otel_span_map.pop( - otel_span.context.span_id - ) - print(f"SPAN: {sentry_span}") - print(f"PARENT SPAN: {parent_sentry_span}") + span_id = otel_span.context.span_id + sentry_span = self.otel_span_map.pop(span_id) if not sentry_span: return sentry_span.op = otel_span.name + if isinstance(sentry_span, Transaction): - hub.scope.set_transaction_name("SENTRYOTEL-{}".format(otel_span.name)) - # TODO: set otel context - # hub.scope.set_context(...) + scope.set_transaction_name(otel_span.name) + # TODO: set "otel" context scope.set_context(...) + else: + self._update_span_with_otel_data(sentry_span, otel_span) + + sentry_span.finish() + def _update_span_with_otel_data(self, sentry_span, otel_span): for key in otel_span.attributes: val = otel_span.attributes[key] sentry_span.set_data(key, val) - if key == "db.statement": - sentry_span.description = val + op = otel_span.name + description = otel_span.name - sentry_span.finish() + http_method = otel_span.attributes.get(SpanAttributes.HTTP_METHOD, None) + db_query = otel_span.attributes.get(SpanAttributes.DB_SYSTEM, None) - if parent_sentry_span: - print(f"SET NEW CURRENT SPAN: {parent_sentry_span}") - scope.span = parent_sentry_span + if http_method: + op = "http.{}".format(http_method) + 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) + + status_code = otel_span.attributes.get( + SpanAttributes.HTTP_STATUS_CODE, None + ) + if status_code: + sentry_span.set_http_status(status_code) + + elif db_query: + op = "db" + statement = otel_span.attributes.get(SpanAttributes.DB_STATEMENT, None) + if statement: + description = statement + + sentry_span.op = op + sentry_span.description = description diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 22ed3cbd98..03ce665489 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -105,10 +105,6 @@ def __init__(self, app, use_x_forwarded_for=False): def __call__(self, environ, start_response): # type: (Dict[str, str], Callable[..., Any]) -> _ScopedResponse - import ipdb - - ipdb.set_trace() - if _wsgi_middleware_applied.get(False): return self.app(environ, start_response) From e4bb1ffcc4cf0677b877d7b19264d529acd707d6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Oct 2022 17:01:53 +0200 Subject: [PATCH 06/46] Added boilerplate for otel. --- sentry_sdk/integrations/opentelemetry/__init__.py | 3 +++ sentry_sdk/integrations/opentelemetry/span_processor.py | 9 +++++++++ 2 files changed, 12 insertions(+) create mode 100644 sentry_sdk/integrations/opentelemetry/__init__.py create mode 100644 sentry_sdk/integrations/opentelemetry/span_processor.py diff --git a/sentry_sdk/integrations/opentelemetry/__init__.py b/sentry_sdk/integrations/opentelemetry/__init__.py new file mode 100644 index 0000000000..2ec31af8c9 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/__init__.py @@ -0,0 +1,3 @@ +from sentry_sdk.integrations.opentelemetry.span_processor import ( # noqa: F401 + SentrySpanProcessor, +) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py new file mode 100644 index 0000000000..5cd4147987 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -0,0 +1,9 @@ +from opentelemetry.sdk.trace import SpanProcessor + + +class SentrySpanProcessor(SpanProcessor): + def on_start(self, span, parent_context=None): + print(f"[] in on_start {span}") + + def on_end(self, span): + print(f"~ in on_end {span}") From 6148b595cd2ad215c4a39193010df02ba5a170e8 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 21 Oct 2022 09:02:46 +0200 Subject: [PATCH 07/46] Added first version of a span processor --- .../opentelemetry/span_processor.py | 49 +++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 5cd4147987..b5c9fb8d66 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,9 +1,50 @@ from opentelemetry.sdk.trace import SpanProcessor +from sentry_sdk.hub import Hub +from sentry_sdk.tracing import Transaction + class SentrySpanProcessor(SpanProcessor): - def on_start(self, span, parent_context=None): - print(f"[] in on_start {span}") + otel_span_map = {} + + def on_start(self, otel_span, parent_context=None): + print(f"[SENTRY/OTEL] in on_start {otel_span}") + hub = Hub.current + sentry_span = None + parent_sentry_span = hub.scope.span + + if parent_sentry_span: + sentry_span = parent_sentry_span.start_child(op=otel_span.name) + else: + # TODO: add the baggagae sentry-trace stuff to transaction + sentry_span = hub.start_transaction(name=otel_span.name) + + hub.scope.span = sentry_span + self.otel_span_map[otel_span.context.span_id] = ( + sentry_span, + parent_sentry_span, + ) + + def on_end(self, otel_span): + print(f"[SENTRY/OTEL] in on_end {otel_span}") + hub = Hub.current + sentry_span, parent_sentry_span = self.otel_span_map[otel_span.context.span_id] + + if not sentry_span: + return + + sentry_span.op = otel_span.name + if isinstance(sentry_span, Transaction): + hub.scope.set_transaction_name(otel_span.name) + + for key in otel_span.attributes: + val = otel_span.attributes[key] + sentry_span.set_data(key, val) + + if key == "db.statement": + sentry_span.description = val + + sentry_span.finish() - def on_end(self, span): - print(f"~ in on_end {span}") + if parent_sentry_span: + hub.scope.span = parent_sentry_span From b3c537ea96ad989f2f65275e216949484de6e60f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 10 Nov 2022 10:20:21 +0100 Subject: [PATCH 08/46] Trying to make a centralized noop for performance capturing functions. --- sentry_sdk/consts.py | 56 ++++++++++--------- sentry_sdk/hub.py | 14 ++++- .../opentelemetry/span_processor.py | 10 +++- sentry_sdk/integrations/stdlib.py | 3 + sentry_sdk/tracing.py | 42 ++++++++++++++ 5 files changed, 96 insertions(+), 29 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index c920fc8fa5..f73cdd413e 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -44,6 +44,36 @@ DEFAULT_MAX_BREADCRUMBS = 100 +class INSTRUMENTER: + SENTRY = "sentry" + OTEL = "otel" + + +class OP: + DB = "db" + DB_REDIS = "db.redis" + EVENT_DJANGO = "event.django" + FUNCTION = "function" + FUNCTION_AWS = "function.aws" + FUNCTION_GCP = "function.gcp" + HTTP_CLIENT = "http.client" + HTTP_CLIENT_STREAM = "http.client.stream" + HTTP_SERVER = "http.server" + MIDDLEWARE_DJANGO = "middleware.django" + MIDDLEWARE_STARLETTE = "middleware.starlette" + MIDDLEWARE_STARLETTE_RECEIVE = "middleware.starlette.receive" + MIDDLEWARE_STARLETTE_SEND = "middleware.starlette.send" + QUEUE_SUBMIT_CELERY = "queue.submit.celery" + QUEUE_TASK_CELERY = "queue.task.celery" + QUEUE_TASK_RQ = "queue.task.rq" + SUBPROCESS = "subprocess" + SUBPROCESS_WAIT = "subprocess.wait" + SUBPROCESS_COMMUNICATE = "subprocess.communicate" + TEMPLATE_RENDER = "template.render" + VIEW_RENDER = "view.render" + WEBSOCKET_SERVER = "websocket.server" + + # This type exists to trick mypy and PyCharm into thinking `init` and `Client` # take these arguments (even though they take opaque **kwargs) class ClientConstructor(object): @@ -57,6 +87,7 @@ def __init__( server_name=None, # type: Optional[str] shutdown_timeout=2, # type: float integrations=[], # type: Sequence[Integration] # noqa: B006 + instrumenter=INSTRUMENTER.SENTRY, # type: Optional[str] in_app_include=[], # type: List[str] # noqa: B006 in_app_exclude=[], # type: List[str] # noqa: B006 default_integrations=True, # type: bool @@ -105,28 +136,3 @@ def _get_default_options(): VERSION = "1.10.1" - - -class OP: - DB = "db" - DB_REDIS = "db.redis" - EVENT_DJANGO = "event.django" - FUNCTION = "function" - FUNCTION_AWS = "function.aws" - FUNCTION_GCP = "function.gcp" - HTTP_CLIENT = "http.client" - HTTP_CLIENT_STREAM = "http.client.stream" - HTTP_SERVER = "http.server" - MIDDLEWARE_DJANGO = "middleware.django" - MIDDLEWARE_STARLETTE = "middleware.starlette" - MIDDLEWARE_STARLETTE_RECEIVE = "middleware.starlette.receive" - MIDDLEWARE_STARLETTE_SEND = "middleware.starlette.send" - QUEUE_SUBMIT_CELERY = "queue.submit.celery" - QUEUE_TASK_CELERY = "queue.task.celery" - QUEUE_TASK_RQ = "queue.task.rq" - SUBPROCESS = "subprocess" - SUBPROCESS_WAIT = "subprocess.wait" - SUBPROCESS_COMMUNICATE = "subprocess.communicate" - TEMPLATE_RENDER = "template.render" - VIEW_RENDER = "view.render" - WEBSOCKET_SERVER = "websocket.server" diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 3d4a28d526..402b03da59 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -5,9 +5,10 @@ from contextlib import contextmanager from sentry_sdk._compat import with_metaclass +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.scope import Scope from sentry_sdk.client import Client -from sentry_sdk.tracing import Span, Transaction +from sentry_sdk.tracing import NoOp, Span, Transaction from sentry_sdk.session import Session from sentry_sdk.utils import ( exc_info_from_error, @@ -464,6 +465,10 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ + if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: + logger.warn("start_span does NOTHING because instrumenter is OTEL") + return NoOp() + # TODO: consider removing this in a future release. # This is for backwards compatibility with releases before # start_transaction existed, to allow for a smoother transition. @@ -519,6 +524,13 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ + import ipdb + + ipdb.set_trace() + if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: + logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOp() + custom_sampling_context = kwargs.pop("custom_sampling_context", {}) # if we haven't been given a transaction, make one diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index b5c9fb8d66..537a0dfc17 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -14,10 +14,14 @@ def on_start(self, otel_span, parent_context=None): parent_sentry_span = hub.scope.span if parent_sentry_span: - sentry_span = parent_sentry_span.start_child(op=otel_span.name) + sentry_span = parent_sentry_span.start_child( + op="SENTRYOTEL-{}".format(otel_span.name) + ) else: # TODO: add the baggagae sentry-trace stuff to transaction - sentry_span = hub.start_transaction(name=otel_span.name) + sentry_span = hub.start_transaction( + name="SENTRYOTEL-{}".format(otel_span.name) + ) hub.scope.span = sentry_span self.otel_span_map[otel_span.context.span_id] = ( @@ -35,7 +39,7 @@ def on_end(self, otel_span): sentry_span.op = otel_span.name if isinstance(sentry_span, Transaction): - hub.scope.set_transaction_name(otel_span.name) + hub.scope.set_transaction_name("SENTRYOTEL-{}".format(otel_span.name)) for key in otel_span.attributes: val = otel_span.attributes[key] diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 3b81b6c2c5..30c63d3559 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -186,6 +186,9 @@ def sentry_patched_popen_init(self, *a, **kw): env = None + import ipdb + + ipdb.set_trace() with hub.start_span(op=OP.SUBPROCESS, description=description) as span: for k, v in hub.iter_trace_propagation_headers(span): diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index aacb3a5bb3..23a5c3263b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -6,6 +6,7 @@ from datetime import datetime, timedelta import sentry_sdk +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.utils import logger from sentry_sdk._types import MYPY @@ -75,6 +76,40 @@ def add(self, span): self.spans.append(span) +class NoOp: + def __init__(self, *args, **kwargs): + # type: (*Any, **Any) -> None + pass + + def __enter__(self): + # type: () -> Any + return self + + def __exit__(self, *args, **kwargs): + # type: (*Any, **Any) -> None + pass + + def __setattr__(self, *args, **kwargs): + import ipdb + + ipdb.set_trace() + pass + + def __getattr__(self, *args, **kwargs): + import ipdb + + ipdb.set_trace() + name = args[0] + if name in ["start_span", "start_transaction", "start_child"]: + + def nothing(*args, **kwargs): + return NoOp(*args, **kwargs) + + return nothing + + pass + + class Span(object): __slots__ = ( "trace_id", @@ -213,6 +248,13 @@ def start_child(self, **kwargs): trace id, sampling decision, transaction pointer, and span recorder are inherited from the current span/transaction. """ + import ipdb + + ipdb.set_trace() + if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: + logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOp() + kwargs.setdefault("sampled", self.sampled) child = Span( From fb05f20ee245feefd404c78da268047ab3115cbc Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 2 Nov 2022 09:41:06 +0100 Subject: [PATCH 09/46] Committing some otel changes from last week. --- sentry_sdk/hub.py | 29 ++++--- .../opentelemetry/span_processor.py | 52 ++++++++++-- sentry_sdk/integrations/stdlib.py | 4 - sentry_sdk/integrations/wsgi.py | 4 + sentry_sdk/tracing.py | 85 ++++++++++--------- 5 files changed, 111 insertions(+), 63 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 402b03da59..904a29ede2 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -8,7 +8,7 @@ from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.scope import Scope from sentry_sdk.client import Client -from sentry_sdk.tracing import NoOp, Span, Transaction +from sentry_sdk.tracing import NoOpSpan, Span, Transaction from sentry_sdk.session import Session from sentry_sdk.utils import ( exc_info_from_error, @@ -451,6 +451,7 @@ def add_breadcrumb( def start_span( self, span=None, # type: Optional[Span] + instrumenter=None, # type: Optional[str] **kwargs # type: Any ): # type: (...) -> Span @@ -465,9 +466,14 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ - if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: - logger.warn("start_span does NOTHING because instrumenter is OTEL") - return NoOp() + instrumenter = ( + instrumenter + if instrumenter is not None + else self.client.options["instrumenter"] + ) + if instrumenter == INSTRUMENTER.OTEL: + # logger.warn("start_span does NOTHING because instrumenter is OTEL") + return NoOpSpan() # TODO: consider removing this in a future release. # This is for backwards compatibility with releases before @@ -499,6 +505,7 @@ def start_span( def start_transaction( self, transaction=None, # type: Optional[Transaction] + instrumenter=None, # type: Optional[str] **kwargs # type: Any ): # type: (...) -> Transaction @@ -524,12 +531,14 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ - import ipdb - - ipdb.set_trace() - if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: - logger.warn("start_transaction does NOTHING because instrumenter is OTEL") - return NoOp() + instrumenter = ( + instrumenter + if instrumenter is not None + else self.client.options["instrumenter"] + ) + if instrumenter == INSTRUMENTER.OTEL: + # logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOpSpan() custom_sampling_context = kwargs.pop("custom_sampling_context", {}) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 537a0dfc17..dd129d6728 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -8,20 +8,41 @@ class SentrySpanProcessor(SpanProcessor): otel_span_map = {} def on_start(self, otel_span, parent_context=None): - print(f"[SENTRY/OTEL] in on_start {otel_span}") + # print(f"[SENTRY/OTEL] in on_start {otel_span}") + print(f"######### START SPAN: {otel_span.context.span_id} ({otel_span.name})") hub = Hub.current + print(f"HUB: {hub}") + if not hub: + return + + scope = hub.scope + print(f"SCOPE: {scope}") + if not scope: + return + + # TODO: isSentryRequest + sentry_span = None - parent_sentry_span = hub.scope.span + parent_sentry_span = scope.span + print(f"SPAN: {parent_sentry_span}") if parent_sentry_span: + print(f"######### START CHILD for PARENT: {parent_sentry_span}") sentry_span = parent_sentry_span.start_child( - op="SENTRYOTEL-{}".format(otel_span.name) + instrumenter="sentry", + op="SENTRYOTEL-{}".format(otel_span.name), ) + print(f"######### CHILD: {sentry_span}") else: # TODO: add the baggagae sentry-trace stuff to transaction + print( + f"######### START TRANSACTION: {otel_span.context.span_id} ({otel_span.name})" + ) sentry_span = hub.start_transaction( - name="SENTRYOTEL-{}".format(otel_span.name) + instrumenter="sentry", + name="SENTRYOTEL-{}".format(otel_span.name), ) + print(f"######### TRANSACTION SPAN: {sentry_span}") hub.scope.span = sentry_span self.otel_span_map[otel_span.context.span_id] = ( @@ -30,16 +51,32 @@ def on_start(self, otel_span, parent_context=None): ) def on_end(self, otel_span): - print(f"[SENTRY/OTEL] in on_end {otel_span}") + # print(f"[SENTRY/OTEL] in on_end {otel_span}") + print(f"######### END SPAN: {otel_span.context.span_id} ({otel_span.name})") + hub = Hub.current - sentry_span, parent_sentry_span = self.otel_span_map[otel_span.context.span_id] + print(f"HUB: {hub}") + if not hub: + return + scope = hub.scope + print(f"SCOPE: {scope}") + if not scope: + return + + sentry_span, parent_sentry_span = self.otel_span_map.pop( + otel_span.context.span_id + ) + print(f"SPAN: {sentry_span}") + print(f"PARENT SPAN: {parent_sentry_span}") if not sentry_span: return sentry_span.op = otel_span.name if isinstance(sentry_span, Transaction): hub.scope.set_transaction_name("SENTRYOTEL-{}".format(otel_span.name)) + # TODO: set otel context + # hub.scope.set_context(...) for key in otel_span.attributes: val = otel_span.attributes[key] @@ -51,4 +88,5 @@ def on_end(self, otel_span): sentry_span.finish() if parent_sentry_span: - hub.scope.span = parent_sentry_span + print(f"SET NEW CURRENT SPAN: {parent_sentry_span}") + scope.span = parent_sentry_span diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 30c63d3559..687d9dd2c1 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -186,11 +186,7 @@ def sentry_patched_popen_init(self, *a, **kw): env = None - import ipdb - - ipdb.set_trace() with hub.start_span(op=OP.SUBPROCESS, description=description) as span: - for k, v in hub.iter_trace_propagation_headers(span): if env is None: env = _init_argument( diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 03ce665489..22ed3cbd98 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -105,6 +105,10 @@ def __init__(self, app, use_x_forwarded_for=False): def __call__(self, environ, start_response): # type: (Dict[str, str], Callable[..., Any]) -> _ScopedResponse + import ipdb + + ipdb.set_trace() + if _wsgi_middleware_applied.get(False): return self.app(environ, start_response) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 23a5c3263b..ddaa227d66 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -76,40 +76,6 @@ def add(self, span): self.spans.append(span) -class NoOp: - def __init__(self, *args, **kwargs): - # type: (*Any, **Any) -> None - pass - - def __enter__(self): - # type: () -> Any - return self - - def __exit__(self, *args, **kwargs): - # type: (*Any, **Any) -> None - pass - - def __setattr__(self, *args, **kwargs): - import ipdb - - ipdb.set_trace() - pass - - def __getattr__(self, *args, **kwargs): - import ipdb - - ipdb.set_trace() - name = args[0] - if name in ["start_span", "start_transaction", "start_child"]: - - def nothing(*args, **kwargs): - return NoOp(*args, **kwargs) - - return nothing - - pass - - class Span(object): __slots__ = ( "trace_id", @@ -239,8 +205,8 @@ def containing_transaction(self): # referencing themselves) return self._containing_transaction - def start_child(self, **kwargs): - # type: (**Any) -> Span + def start_child(self, instrumenter=None, **kwargs): + # type: (Optional[str], **Any) -> Span """ Start a sub-span from the current span or transaction. @@ -248,12 +214,14 @@ def start_child(self, **kwargs): trace id, sampling decision, transaction pointer, and span recorder are inherited from the current span/transaction. """ - import ipdb - - ipdb.set_trace() - if self.client.options["instrumenter"] == INSTRUMENTER.OTEL: - logger.warn("start_transaction does NOTHING because instrumenter is OTEL") - return NoOp() + instrumenter = ( + instrumenter + if instrumenter is not None + else self.client.options["instrumenter"] + ) + if instrumenter == INSTRUMENTER.OTEL: + # logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + return NoOpSpan() kwargs.setdefault("sampled", self.sampled) @@ -695,6 +663,7 @@ def finish(self, hub=None): # to a concrete decision. if self.sampled is None: logger.warning("Discarding transaction without sampling decision.") + return None finished_spans = [ @@ -863,6 +832,38 @@ def _set_initial_sampling_decision(self, sampling_context): ) +class NoOpSpan(Span): + def __repr__(self): + return self.__class__.__name__ + + def __enter__(self): + return self + + def __exit__(self, ty, value, tb): + pass + + def start_child(self, **kwargs): + pass + + def new_span(self, **kwargs): + pass + + def set_tag(self, key, value): + pass + + def set_data(self, key, value): + pass + + def set_status(self, value): + pass + + def set_http_status(self, http_status): + pass + + def finish(self, hub=None): + pass + + # Circular imports from sentry_sdk.tracing_utils import ( From 7cf1c94fa215ab12a78cb5984e2ff86a1570fbd2 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 10 Nov 2022 10:18:35 +0100 Subject: [PATCH 10/46] Cleanup --- .../opentelemetry/span_processor.py | 107 +++++++++++------- sentry_sdk/integrations/wsgi.py | 4 - 2 files changed, 63 insertions(+), 48 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index dd129d6728..b6bdb96010 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,92 +1,111 @@ from opentelemetry.sdk.trace import SpanProcessor +from opentelemetry.semconv.trace import SpanAttributes from sentry_sdk.hub import Hub from sentry_sdk.tracing import Transaction class SentrySpanProcessor(SpanProcessor): - otel_span_map = {} + """ + Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. + """ + + otel_span_map = {} # The mapping from otel span ids to sentry spans def on_start(self, otel_span, parent_context=None): - # print(f"[SENTRY/OTEL] in on_start {otel_span}") - print(f"######### START SPAN: {otel_span.context.span_id} ({otel_span.name})") hub = Hub.current - print(f"HUB: {hub}") if not hub: return - scope = hub.scope - print(f"SCOPE: {scope}") - if not scope: - return + # TODO: check for isSentryRequest and if sdk is initialized... - # TODO: isSentryRequest + span_id = otel_span.context.span_id + parent_span_id = otel_span.parent.span_id if otel_span.parent else None + trace_id = otel_span.context.trace_id - sentry_span = None - parent_sentry_span = scope.span - print(f"SPAN: {parent_sentry_span}") + parent_sentry_span = ( + self.otel_span_map[parent_span_id] if parent_span_id else None + ) + sentry_span = None if parent_sentry_span: - print(f"######### START CHILD for PARENT: {parent_sentry_span}") sentry_span = parent_sentry_span.start_child( + span_id=span_id, + description=otel_span.name, instrumenter="sentry", - op="SENTRYOTEL-{}".format(otel_span.name), ) - print(f"######### CHILD: {sentry_span}") else: # TODO: add the baggagae sentry-trace stuff to transaction - print( - f"######### START TRANSACTION: {otel_span.context.span_id} ({otel_span.name})" - ) sentry_span = hub.start_transaction( + name=otel_span.name, + span_id=span_id, + parent_span_id=parent_span_id, + trace_id=trace_id, + baggage={}, # TODO: add baggage + # TODO: check if we can set start_timestamp instrumenter="sentry", - name="SENTRYOTEL-{}".format(otel_span.name), ) - print(f"######### TRANSACTION SPAN: {sentry_span}") - hub.scope.span = sentry_span - self.otel_span_map[otel_span.context.span_id] = ( - sentry_span, - parent_sentry_span, - ) + self.otel_span_map[span_id] = sentry_span def on_end(self, otel_span): - # print(f"[SENTRY/OTEL] in on_end {otel_span}") - print(f"######### END SPAN: {otel_span.context.span_id} ({otel_span.name})") - hub = Hub.current - print(f"HUB: {hub}") if not hub: return scope = hub.scope - print(f"SCOPE: {scope}") if not scope: return - sentry_span, parent_sentry_span = self.otel_span_map.pop( - otel_span.context.span_id - ) - print(f"SPAN: {sentry_span}") - print(f"PARENT SPAN: {parent_sentry_span}") + span_id = otel_span.context.span_id + sentry_span = self.otel_span_map.pop(span_id) if not sentry_span: return sentry_span.op = otel_span.name + if isinstance(sentry_span, Transaction): - hub.scope.set_transaction_name("SENTRYOTEL-{}".format(otel_span.name)) - # TODO: set otel context - # hub.scope.set_context(...) + scope.set_transaction_name(otel_span.name) + # TODO: set "otel" context scope.set_context(...) + else: + self._update_span_with_otel_data(sentry_span, otel_span) + + sentry_span.finish() + def _update_span_with_otel_data(self, sentry_span, otel_span): for key in otel_span.attributes: val = otel_span.attributes[key] sentry_span.set_data(key, val) - if key == "db.statement": - sentry_span.description = val + op = otel_span.name + description = otel_span.name - sentry_span.finish() + http_method = otel_span.attributes.get(SpanAttributes.HTTP_METHOD, None) + db_query = otel_span.attributes.get(SpanAttributes.DB_SYSTEM, None) - if parent_sentry_span: - print(f"SET NEW CURRENT SPAN: {parent_sentry_span}") - scope.span = parent_sentry_span + if http_method: + op = "http.{}".format(http_method) + 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) + + status_code = otel_span.attributes.get( + SpanAttributes.HTTP_STATUS_CODE, None + ) + if status_code: + sentry_span.set_http_status(status_code) + + elif db_query: + op = "db" + statement = otel_span.attributes.get(SpanAttributes.DB_STATEMENT, None) + if statement: + description = statement + + sentry_span.op = op + sentry_span.description = description diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py index 22ed3cbd98..03ce665489 100644 --- a/sentry_sdk/integrations/wsgi.py +++ b/sentry_sdk/integrations/wsgi.py @@ -105,10 +105,6 @@ def __init__(self, app, use_x_forwarded_for=False): def __call__(self, environ, start_response): # type: (Dict[str, str], Callable[..., Any]) -> _ScopedResponse - import ipdb - - ipdb.set_trace() - if _wsgi_middleware_applied.get(False): return self.app(environ, start_response) From 0977c0a33480d64f99f456545f901e5743113f95 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 10 Nov 2022 16:22:17 +0100 Subject: [PATCH 11/46] Setting otel context in spans --- .../opentelemetry/span_processor.py | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index b6bdb96010..1d61705ac4 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -5,6 +5,9 @@ from sentry_sdk.tracing import Transaction +OPEN_TELEMETRY_CONTEXT = "otel" + + class SentrySpanProcessor(SpanProcessor): """ Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. @@ -66,13 +69,29 @@ def on_end(self, otel_span): if isinstance(sentry_span, Transaction): scope.set_transaction_name(otel_span.name) - # TODO: set "otel" context scope.set_context(...) + scope.set_context(OPEN_TELEMETRY_CONTEXT, self._get_otel_context(otel_span)) + else: self._update_span_with_otel_data(sentry_span, otel_span) sentry_span.finish() + def _get_otel_context(self, otel_span): + ctx = {} + + if otel_span.attributes: + ctx["attributes"] = dict(otel_span.attributes) + + if otel_span.resource.attributes: + ctx["resource"] = dict(otel_span.resource.attributes) + + return ctx + def _update_span_with_otel_data(self, sentry_span, otel_span): + """ + Convert OTel span data and update the Sentry span with it. + This should eventually happen on the server when ingesting the spans. + """ for key in otel_span.attributes: val = otel_span.attributes[key] sentry_span.set_data(key, val) From 5d9ed166d3a6dc273dca432adf1dfde8afb97f56 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 15 Nov 2022 09:46:50 +0100 Subject: [PATCH 12/46] Setting the contexts on span. --- .../integrations/opentelemetry/span_processor.py | 6 ++++-- sentry_sdk/tracing.py | 12 +++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 1d61705ac4..bae18beef6 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -68,8 +68,10 @@ def on_end(self, otel_span): sentry_span.op = otel_span.name if isinstance(sentry_span, Transaction): - scope.set_transaction_name(otel_span.name) - scope.set_context(OPEN_TELEMETRY_CONTEXT, self._get_otel_context(otel_span)) + sentry_span.name = otel_span.name + sentry_span.set_context( + OPEN_TELEMETRY_CONTEXT, self._get_otel_context(otel_span) + ) else: self._update_span_with_otel_data(sentry_span, otel_span) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index ddaa227d66..00bc51d4e9 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -91,6 +91,7 @@ class Span(object): "timestamp", "_tags", "_data", + "_contexts", "_span_recorder", "hub", "_context_manager_state", @@ -137,6 +138,7 @@ def __init__( self.hub = hub self._tags = {} # type: Dict[str, str] self._data = {} # type: Dict[str, Any] + self._contexts = {} # type: Dict[str, Any] self._containing_transaction = containing_transaction self.start_timestamp = datetime.utcnow() try: @@ -461,6 +463,10 @@ def set_http_status(self, http_status): else: self.set_status("unknown_error") + def set_context(self, key, value): + # type: (str, Any) -> None + self._contexts[key] = value + def is_success(self): # type: () -> bool return self.status == "ok" @@ -678,11 +684,15 @@ def finish(self, hub=None): # to be garbage collected self._span_recorder = None + contexts = {} + contexts.update(self._contexts) + contexts.update({"trace": self.get_trace_context()}) + event = { "type": "transaction", "transaction": self.name, "transaction_info": {"source": self.source}, - "contexts": {"trace": self.get_trace_context()}, + "contexts": contexts, "tags": self._tags, "timestamp": self.timestamp, "start_timestamp": self.start_timestamp, From 35b3d887e8a70e563b00a8a440051409bcdc4608 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 17 Nov 2022 15:02:22 +0100 Subject: [PATCH 13/46] Wiring up spans correctly --- .../opentelemetry/span_processor.py | 37 +++++++++---------- sentry_sdk/tracing.py | 2 +- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index bae18beef6..3660a9c7b6 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,3 +1,4 @@ +from opentelemetry.trace import format_span_id, format_trace_id from opentelemetry.sdk.trace import SpanProcessor from opentelemetry.semconv.trace import SpanAttributes @@ -20,47 +21,39 @@ def on_start(self, otel_span, parent_context=None): if not hub: return - # TODO: check for isSentryRequest and if sdk is initialized... + span_id = format_span_id(otel_span.context.span_id) + trace_id = format_trace_id(otel_span.context.trace_id) - span_id = otel_span.context.span_id - parent_span_id = otel_span.parent.span_id if otel_span.parent else None - trace_id = otel_span.context.trace_id - - parent_sentry_span = ( + parent_span_id = ( + format_span_id(otel_span.parent.span_id) if otel_span.parent else None + ) + sentry_parent_span = ( self.otel_span_map[parent_span_id] if parent_span_id else None ) sentry_span = None - if parent_sentry_span: - sentry_span = parent_sentry_span.start_child( + if sentry_parent_span: + sentry_span = sentry_parent_span.start_child( span_id=span_id, description=otel_span.name, + # start_timestamp = xxx, TODO: add start_timestamp to start_child and start_transaction. instrumenter="sentry", ) else: - # TODO: add the baggagae sentry-trace stuff to transaction sentry_span = hub.start_transaction( name=otel_span.name, span_id=span_id, parent_span_id=parent_span_id, trace_id=trace_id, - baggage={}, # TODO: add baggage - # TODO: check if we can set start_timestamp + # baggage={}, # TODO: get baggage from propagator + # start_timestamp = xxx, TODO: add start_timestamp to start_child and start_transaction. instrumenter="sentry", ) self.otel_span_map[span_id] = sentry_span def on_end(self, otel_span): - hub = Hub.current - if not hub: - return - - scope = hub.scope - if not scope: - return - - span_id = otel_span.context.span_id + span_id = format_span_id(otel_span.context.span_id) sentry_span = self.otel_span_map.pop(span_id) if not sentry_span: return @@ -79,6 +72,10 @@ def on_end(self, otel_span): sentry_span.finish() def _get_otel_context(self, otel_span): + """ + Returns the OTel context for Sentry. + See: https://develop.sentry.dev/sdk/performance/opentelemetry/#step-5-add-opentelemetry-context + """ ctx = {} if otel_span.attributes: diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 00bc51d4e9..58c2fde056 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -222,7 +222,7 @@ def start_child(self, instrumenter=None, **kwargs): else self.client.options["instrumenter"] ) if instrumenter == INSTRUMENTER.OTEL: - # logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + # logger.warn("start_child does NOTHING because instrumenter is OTEL") return NoOpSpan() kwargs.setdefault("sampled", self.sampled) From 94e0be8e03a6e2e7b623ef7ff440975affc15284 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 18 Nov 2022 13:21:54 +0100 Subject: [PATCH 14/46] First version of sentry otel propagator. --- .../integrations/opentelemetry/propagator.py | 101 ++++++++++++++++++ .../opentelemetry/span_processor.py | 6 ++ sentry_sdk/tracing.py | 6 ++ 3 files changed, 113 insertions(+) create mode 100644 sentry_sdk/integrations/opentelemetry/propagator.py diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py new file mode 100644 index 0000000000..3e58209fca --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -0,0 +1,101 @@ +import typing + +from opentelemetry import trace +from opentelemetry.context import Context, create_key, get_current +from opentelemetry.propagators.textmap import ( + CarrierT, + Getter, + Setter, + TextMapPropagator, + default_getter, + default_setter, +) +from opentelemetry.trace import TraceFlags, NonRecordingSpan, SpanContext + +from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor +from sentry_sdk.tracing import Transaction +from sentry_sdk.tracing_utils import Baggage + + +SENTRY_TRACE_HEADER_NAME = "sentry-trace" +BAGGAGE_HEADER_NAME = "sentry-baggage" + + +SENTRY_TRACE_KEY = create_key("sentry-trace") +SENTRY_BAGGAGE_KEY = create_key("sentry-baggage") + + +class SentryPropagator(TextMapPropagator): + def extract( + self, + carrier: CarrierT, + context: typing.Optional[Context] = None, + getter: Getter = default_getter, + ) -> Context: + if context is None: + context = get_current() + + sentry_trace = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) + if not sentry_trace: + return context + + sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace) + context = context.set_value(SENTRY_TRACE_KEY, sentry_trace_data) + + trace_id, span_id, _parent_sampled = sentry_trace_data + + span_context = SpanContext( + trace_id=trace_id, + span_id=span_id, + # we simulate a sampled trace on the otel side and leave the sampling to sentry + trace_flags=TraceFlags.SAMPLED, + is_remote=True, + ) + + baggage_header = getter.get(carrier, BAGGAGE_HEADER_NAME) + + if baggage_header: + baggage = Baggage.from_incoming_header(baggage_header) + else: + # If there's an incoming sentry-trace but no incoming baggage header, + # for instance in traces coming from older SDKs, + # baggage will be empty and frozen and won't be populated as head SDK. + baggage = Baggage() + + baggage.freeze() + context = context.set_value(SENTRY_BAGGAGE_KEY, baggage) + + span = NonRecordingSpan(span_context) + modified_context = trace.set_span_in_context(span, context) + return modified_context + + def inject( + self, + carrier: CarrierT, + context: typing.Optional[Context] = None, + setter: Setter = default_setter, + ) -> None: + if context is None: + context = get_current() + + current_span = trace.get_current_span(context) + span_id = trace.format_span_id(current_span.context.span_id) + + span_map = SentrySpanProcessor().otel_span_map + sentry_span = span_map.get(span_id, None) + if not sentry_span: + return + + setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_traceparent()) + + baggage = sentry_span.get_baggage() + if baggage: + setter.set(carrier, BAGGAGE_HEADER_NAME, baggage) + + @property + def fields(self) -> typing.Set[str]: + return { + self.TRACE_ID_KEY, + self.SPAN_ID_KEY, + self.SAMPLED_KEY, + } diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 3660a9c7b6..5d8a8ea204 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -16,6 +16,12 @@ class SentrySpanProcessor(SpanProcessor): otel_span_map = {} # The mapping from otel span ids to sentry spans + def __new__(cls): + if not hasattr(cls, "instance"): + cls.instance = super(SentrySpanProcessor, cls).__new__(cls) + + return cls.instance + def on_start(self, otel_span, parent_context=None): hub = Hub.current if not hub: diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 58c2fde056..1c596d8229 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -367,6 +367,12 @@ def to_traceparent(self): sampled = "0" return "%s-%s-%s" % (self.trace_id, self.span_id, sampled) + @classmethod + def extract_sentry_trace(sentry_trace): + trace_id, parent_span_id, parent_sampled = sentry_trace.split("-") + parent_sampled = True if parent_sampled == "1" else False + return (trace_id, parent_span_id, parent_sampled) + def to_tracestate(self): # type: () -> Optional[str] """ From 993dc330d5eb2d75e496459636c5052165fba900 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 18 Nov 2022 14:33:41 +0100 Subject: [PATCH 15/46] Moved SENTRY_TRACE_HEADER_NAME to tracing.py and using it everywhere --- sentry_sdk/integrations/flask.py | 9 ++++++--- sentry_sdk/integrations/opentelemetry/propagator.py | 3 +-- sentry_sdk/tracing.py | 12 +++++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py index 52cce0b4b4..67c87b64f6 100644 --- a/sentry_sdk/integrations/flask.py +++ b/sentry_sdk/integrations/flask.py @@ -6,7 +6,7 @@ from sentry_sdk.integrations._wsgi_common import RequestExtractor from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware from sentry_sdk.scope import Scope -from sentry_sdk.tracing import SOURCE_FOR_STYLE +from sentry_sdk.tracing import SENTRY_TRACE_HEADER_NAME, SOURCE_FOR_STYLE from sentry_sdk.utils import ( capture_internal_exceptions, event_from_exception, @@ -101,8 +101,11 @@ def _add_sentry_trace(sender, template, context, **extra): sentry_span = Hub.current.scope.span context["sentry_trace"] = ( Markup( - '' - % (sentry_span.to_traceparent(),) + '' + % ( + SENTRY_TRACE_HEADER_NAME, + sentry_span.to_traceparent(), + ) ) if sentry_span else "" diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 3e58209fca..28e00068d6 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -13,11 +13,10 @@ from opentelemetry.trace import TraceFlags, NonRecordingSpan, SpanContext from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor -from sentry_sdk.tracing import Transaction +from sentry_sdk.tracing import SENTRY_TRACE_HEADER_NAME, Transaction from sentry_sdk.tracing_utils import Baggage -SENTRY_TRACE_HEADER_NAME = "sentry-trace" BAGGAGE_HEADER_NAME = "sentry-baggage" diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 1c596d8229..a3fd6013a8 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -26,6 +26,8 @@ from sentry_sdk._types import Event, SamplingContext, MeasurementUnit +SENTRY_TRACE_HEADER_NAME = "sentry-trace" + # Transaction source # see https://develop.sentry.dev/sdk/event-payloads/transaction/#transaction-annotations TRANSACTION_SOURCE_CUSTOM = "custom" @@ -293,7 +295,9 @@ def continue_from_headers( baggage = Baggage.from_incoming_header(headers.get("baggage")) kwargs.update({"baggage": baggage}) - sentrytrace_kwargs = extract_sentrytrace_data(headers.get("sentry-trace")) + sentrytrace_kwargs = extract_sentrytrace_data( + headers.get(SENTRY_TRACE_HEADER_NAME) + ) if sentrytrace_kwargs is not None: kwargs.update(sentrytrace_kwargs) @@ -320,7 +324,7 @@ def iter_headers(self): `sentry_tracestate` value, this will cause one to be generated and stored. """ - yield "sentry-trace", self.to_traceparent() + 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 @@ -356,7 +360,9 @@ def from_traceparent( if not traceparent: return None - return cls.continue_from_headers({"sentry-trace": traceparent}, **kwargs) + return cls.continue_from_headers( + {SENTRY_TRACE_HEADER_NAME: traceparent}, **kwargs + ) def to_traceparent(self): # type: () -> str From 485d776e05be421f6ba7911ef018918b4e846c87 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 23 Nov 2022 12:21:24 +0100 Subject: [PATCH 16/46] Fixed extracting tracing information from incoming request --- .../integrations/opentelemetry/propagator.py | 18 +++++++++--------- .../opentelemetry/span_processor.py | 2 +- sentry_sdk/tracing.py | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 28e00068d6..97f5be65c0 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -1,7 +1,7 @@ import typing from opentelemetry import trace -from opentelemetry.context import Context, create_key, get_current +from opentelemetry.context import Context, create_key, get_current, set_value from opentelemetry.propagators.textmap import ( CarrierT, Getter, @@ -17,7 +17,7 @@ from sentry_sdk.tracing_utils import Baggage -BAGGAGE_HEADER_NAME = "sentry-baggage" +BAGGAGE_HEADER_NAME = "baggage" SENTRY_TRACE_KEY = create_key("sentry-trace") @@ -38,23 +38,23 @@ def extract( if not sentry_trace: return context - sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace) - context = context.set_value(SENTRY_TRACE_KEY, sentry_trace_data) + sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace[0]) + context = set_value(SENTRY_TRACE_KEY, sentry_trace_data, context) trace_id, span_id, _parent_sampled = sentry_trace_data span_context = SpanContext( - trace_id=trace_id, - span_id=span_id, + trace_id=int(trace_id, 16), + span_id=int(span_id, 16), # we simulate a sampled trace on the otel side and leave the sampling to sentry - trace_flags=TraceFlags.SAMPLED, + trace_flags=TraceFlags(TraceFlags.SAMPLED), is_remote=True, ) baggage_header = getter.get(carrier, BAGGAGE_HEADER_NAME) if baggage_header: - baggage = Baggage.from_incoming_header(baggage_header) + baggage = Baggage.from_incoming_header(baggage_header[0]) else: # If there's an incoming sentry-trace but no incoming baggage header, # for instance in traces coming from older SDKs, @@ -62,7 +62,7 @@ def extract( baggage = Baggage() baggage.freeze() - context = context.set_value(SENTRY_BAGGAGE_KEY, baggage) + context = set_value(SENTRY_BAGGAGE_KEY, baggage, context) span = NonRecordingSpan(span_context) modified_context = trace.set_span_in_context(span, context) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 5d8a8ea204..f30133171f 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -34,7 +34,7 @@ def on_start(self, otel_span, parent_context=None): format_span_id(otel_span.parent.span_id) if otel_span.parent else None ) sentry_parent_span = ( - self.otel_span_map[parent_span_id] if parent_span_id else None + self.otel_span_map.get(parent_span_id, None) if parent_span_id else None ) sentry_span = None diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index a3fd6013a8..fbba6d783b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -374,7 +374,7 @@ def to_traceparent(self): return "%s-%s-%s" % (self.trace_id, self.span_id, sampled) @classmethod - def extract_sentry_trace(sentry_trace): + def extract_sentry_trace(cls, sentry_trace): trace_id, parent_span_id, parent_sampled = sentry_trace.split("-") parent_sampled = True if parent_sampled == "1" else False return (trace_id, parent_span_id, parent_sampled) From 8cf16cbeee1fadf6f3b9e563acfff273db798d0f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 23 Nov 2022 15:19:22 +0100 Subject: [PATCH 17/46] Get baggabe from the otel propagator and add it to the Sentry transaction. --- .vscode/settings.json | 6 ++- .../integrations/opentelemetry/propagator.py | 5 +- .../opentelemetry/span_processor.py | 49 +++++++++++++++---- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index c167a13dc2..ba2472c4c9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,4 +1,6 @@ { "python.pythonPath": ".venv/bin/python", - "python.formatting.provider": "black" -} \ No newline at end of file + "python.formatting.provider": "black", + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true +} diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 97f5be65c0..8b7895c767 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -12,7 +12,6 @@ ) from opentelemetry.trace import TraceFlags, NonRecordingSpan, SpanContext -from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor from sentry_sdk.tracing import SENTRY_TRACE_HEADER_NAME, Transaction from sentry_sdk.tracing_utils import Baggage @@ -80,6 +79,10 @@ def inject( current_span = trace.get_current_span(context) span_id = trace.format_span_id(current_span.context.span_id) + from sentry_sdk.integrations.opentelemetry.span_processor import ( + SentrySpanProcessor, + ) + span_map = SentrySpanProcessor().otel_span_map sentry_span = span_map.get(span_id, None) if not sentry_span: diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index f30133171f..63c7810ab2 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,8 +1,13 @@ from opentelemetry.trace import format_span_id, format_trace_id from opentelemetry.sdk.trace import SpanProcessor from opentelemetry.semconv.trace import SpanAttributes +from opentelemetry.context import get_value from sentry_sdk.hub import Hub +from sentry_sdk.integrations.opentelemetry.propagator import ( + SENTRY_BAGGAGE_KEY, + SENTRY_TRACE_KEY, +) from sentry_sdk.tracing import Transaction @@ -27,12 +32,9 @@ def on_start(self, otel_span, parent_context=None): if not hub: return - span_id = format_span_id(otel_span.context.span_id) - trace_id = format_trace_id(otel_span.context.trace_id) + trace_data = self._get_trace_data(otel_span, parent_context) - parent_span_id = ( - format_span_id(otel_span.parent.span_id) if otel_span.parent else None - ) + parent_span_id = trace_data["parent_span_id"] sentry_parent_span = ( self.otel_span_map.get(parent_span_id, None) if parent_span_id else None ) @@ -40,7 +42,7 @@ def on_start(self, otel_span, parent_context=None): sentry_span = None if sentry_parent_span: sentry_span = sentry_parent_span.start_child( - span_id=span_id, + span_id=trace_data["span_id"], description=otel_span.name, # start_timestamp = xxx, TODO: add start_timestamp to start_child and start_transaction. instrumenter="sentry", @@ -48,15 +50,15 @@ def on_start(self, otel_span, parent_context=None): else: sentry_span = hub.start_transaction( name=otel_span.name, - span_id=span_id, + span_id=trace_data["span_id"], parent_span_id=parent_span_id, - trace_id=trace_id, - # baggage={}, # TODO: get baggage from propagator + trace_id=trace_data["trace_id"], + baggage=trace_data["baggage"], # start_timestamp = xxx, TODO: add start_timestamp to start_child and start_transaction. instrumenter="sentry", ) - self.otel_span_map[span_id] = sentry_span + self.otel_span_map[trace_data["span_id"]] = sentry_span def on_end(self, otel_span): span_id = format_span_id(otel_span.context.span_id) @@ -92,6 +94,33 @@ def _get_otel_context(self, otel_span): return ctx + def _get_trace_data(self, otel_span, parent_context): + """ + Extracts tracing information from one OTel span and its parent OTel context. + """ + trace_data = {} + + span_id = format_span_id(otel_span.context.span_id) + trace_data["span_id"] = span_id + + trace_id = format_trace_id(otel_span.context.trace_id) + trace_data["trace_id"] = trace_id + + parent_span_id = ( + format_span_id(otel_span.parent.span_id) if otel_span.parent else None + ) + trace_data["parent_span_id"] = parent_span_id + + sentry_trace_data = get_value(SENTRY_TRACE_KEY, parent_context) + trace_data["parent_sampled"] = ( + sentry_trace_data[2] if sentry_trace_data else None + ) + + baggage = get_value(SENTRY_BAGGAGE_KEY, parent_context) + trace_data["baggage"] = baggage + + return trace_data + def _update_span_with_otel_data(self, sentry_span, otel_span): """ Convert OTel span data and update the Sentry span with it. From f14c2149572f2e75304b10e71662c27e0f95d14f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 23 Nov 2022 15:20:15 +0100 Subject: [PATCH 18/46] Code formatting --- sentry_sdk/integrations/opentelemetry/span_processor.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 63c7810ab2..547f6e818c 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,7 +1,7 @@ -from opentelemetry.trace import format_span_id, format_trace_id +from opentelemetry.context import get_value from opentelemetry.sdk.trace import SpanProcessor from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.context import get_value +from opentelemetry.trace import format_span_id, format_trace_id from sentry_sdk.hub import Hub from sentry_sdk.integrations.opentelemetry.propagator import ( @@ -10,7 +10,6 @@ ) from sentry_sdk.tracing import Transaction - OPEN_TELEMETRY_CONTEXT = "otel" From c2607a4998089955aa0ec04b052b352aecf617db Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 23 Nov 2022 15:58:02 +0100 Subject: [PATCH 19/46] Use start and end time from otel span --- .../opentelemetry/span_processor.py | 10 ++++++--- sentry_sdk/tracing.py | 22 ++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 547f6e818c..97dae82e10 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,3 +1,5 @@ +from datetime import datetime + from opentelemetry.context import get_value from opentelemetry.sdk.trace import SpanProcessor from opentelemetry.semconv.trace import SpanAttributes @@ -43,7 +45,7 @@ def on_start(self, otel_span, parent_context=None): sentry_span = sentry_parent_span.start_child( span_id=trace_data["span_id"], description=otel_span.name, - # start_timestamp = xxx, TODO: add start_timestamp to start_child and start_transaction. + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), instrumenter="sentry", ) else: @@ -53,7 +55,7 @@ def on_start(self, otel_span, parent_context=None): parent_span_id=parent_span_id, trace_id=trace_data["trace_id"], baggage=trace_data["baggage"], - # start_timestamp = xxx, TODO: add start_timestamp to start_child and start_transaction. + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), instrumenter="sentry", ) @@ -76,7 +78,9 @@ def on_end(self, otel_span): else: self._update_span_with_otel_data(sentry_span, otel_span) - sentry_span.finish() + sentry_span.finish( + end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) + ) def _get_otel_context(self, otel_span): """ diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index fbba6d783b..1111c647d5 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -127,6 +127,7 @@ def __init__( status=None, # type: Optional[str] transaction=None, # type: Optional[str] # deprecated containing_transaction=None, # type: Optional[Transaction] + start_timestamp=None, # type: Optional[datetime] ): # type: (...) -> None self.trace_id = trace_id or uuid.uuid4().hex @@ -142,7 +143,7 @@ def __init__( self._data = {} # type: Dict[str, Any] self._contexts = {} # type: Dict[str, Any] self._containing_transaction = containing_transaction - self.start_timestamp = datetime.utcnow() + self.start_timestamp = start_timestamp or datetime.utcnow() try: # TODO: For Python 3.7+, we could use a clock with ns resolution: # self._start_timestamp_monotonic = time.perf_counter_ns() @@ -483,8 +484,8 @@ def is_success(self): # type: () -> bool return self.status == "ok" - def finish(self, hub=None): - # type: (Optional[sentry_sdk.Hub]) -> Optional[str] + def finish(self, hub=None, end_timestamp=None): + # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str] # XXX: would be type: (Optional[sentry_sdk.Hub]) -> None, but that leads # to incompatible return types for Span.finish and Transaction.finish. if self.timestamp is not None: @@ -494,8 +495,13 @@ def finish(self, hub=None): hub = hub or self.hub or sentry_sdk.Hub.current try: - duration_seconds = time.perf_counter() - self._start_timestamp_monotonic - self.timestamp = self.start_timestamp + timedelta(seconds=duration_seconds) + if end_timestamp: + self.timestamp = end_timestamp + else: + duration_seconds = time.perf_counter() - self._start_timestamp_monotonic + self.timestamp = self.start_timestamp + timedelta( + seconds=duration_seconds + ) except AttributeError: self.timestamp = datetime.utcnow() @@ -641,8 +647,8 @@ def containing_transaction(self): # reference. return self - def finish(self, hub=None): - # type: (Optional[sentry_sdk.Hub]) -> Optional[str] + def finish(self, hub=None, end_timestamp=None): + # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str] if self.timestamp is not None: # This transaction is already finished, ignore. return None @@ -674,7 +680,7 @@ def finish(self, hub=None): ) self.name = "" - Span.finish(self, hub) + Span.finish(self, hub, end_timestamp) if not self.sampled: # At this point a `sampled = None` should have already been resolved From ff45019e32ebc6f820037de8df1026c64c34737e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 24 Nov 2022 14:24:37 +0100 Subject: [PATCH 20/46] Fixed typing --- sentry_sdk/api.py | 3 +- sentry_sdk/hub.py | 22 +++++-------- .../integrations/opentelemetry/propagator.py | 21 ++++++++---- .../opentelemetry/span_processor.py | 27 +++++++++++---- sentry_sdk/tracing.py | 33 +++++++++++++------ 5 files changed, 69 insertions(+), 37 deletions(-) diff --git a/sentry_sdk/api.py b/sentry_sdk/api.py index cec914aca1..ffa017cfc1 100644 --- a/sentry_sdk/api.py +++ b/sentry_sdk/api.py @@ -4,6 +4,7 @@ from sentry_sdk.scope import Scope from sentry_sdk._types import MYPY +from sentry_sdk.tracing import NoOpSpan if MYPY: from typing import Any @@ -210,5 +211,5 @@ def start_transaction( transaction=None, # type: Optional[Transaction] **kwargs # type: Any ): - # type: (...) -> Transaction + # type: (...) -> Union[Transaction, NoOpSpan] return Hub.current.start_transaction(transaction, **kwargs) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 904a29ede2..5da32603de 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -451,7 +451,6 @@ def add_breadcrumb( def start_span( self, span=None, # type: Optional[Span] - instrumenter=None, # type: Optional[str] **kwargs # type: Any ): # type: (...) -> Span @@ -466,13 +465,12 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ - instrumenter = ( - instrumenter - if instrumenter is not None - else self.client.options["instrumenter"] + instrumenter = kwargs.get("instrumenter", None) or ( + self.client and self.client.options["instrumenter"] ) + if instrumenter == INSTRUMENTER.OTEL: - # logger.warn("start_span does NOTHING because instrumenter is OTEL") + # logger.warn("start_child does NOTHING because instrumenter is OTEL") return NoOpSpan() # TODO: consider removing this in a future release. @@ -505,10 +503,9 @@ def start_span( def start_transaction( self, transaction=None, # type: Optional[Transaction] - instrumenter=None, # type: Optional[str] **kwargs # type: Any ): - # type: (...) -> Transaction + # type: (...) -> Union[Transaction, NoOpSpan] """ Start and return a transaction. @@ -531,13 +528,12 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ - instrumenter = ( - instrumenter - if instrumenter is not None - else self.client.options["instrumenter"] + instrumenter = kwargs.get("instrumenter", None) or ( + self.client and self.client.options["instrumenter"] ) + if instrumenter == INSTRUMENTER.OTEL: - # logger.warn("start_transaction does NOTHING because instrumenter is OTEL") + # logger.warn("start_child does NOTHING because instrumenter is OTEL") return NoOpSpan() custom_sampling_context = kwargs.pop("custom_sampling_context", {}) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 8b7895c767..3873da56d9 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -1,8 +1,13 @@ import typing -from opentelemetry import trace -from opentelemetry.context import Context, create_key, get_current, set_value -from opentelemetry.propagators.textmap import ( +from opentelemetry import trace # type: ignore +from opentelemetry.context import ( # type: ignore + Context, + create_key, + get_current, + set_value, +) +from opentelemetry.propagators.textmap import ( # type: ignore CarrierT, Getter, Setter, @@ -10,7 +15,11 @@ default_getter, default_setter, ) -from opentelemetry.trace import TraceFlags, NonRecordingSpan, SpanContext +from opentelemetry.trace import ( # type: ignore + TraceFlags, + NonRecordingSpan, + SpanContext, +) from sentry_sdk.tracing import SENTRY_TRACE_HEADER_NAME, Transaction from sentry_sdk.tracing_utils import Baggage @@ -23,7 +32,7 @@ SENTRY_BAGGAGE_KEY = create_key("sentry-baggage") -class SentryPropagator(TextMapPropagator): +class SentryPropagator(TextMapPropagator): # type: ignore def extract( self, carrier: CarrierT, @@ -58,7 +67,7 @@ def extract( # If there's an incoming sentry-trace but no incoming baggage header, # for instance in traces coming from older SDKs, # baggage will be empty and frozen and won't be populated as head SDK. - baggage = Baggage() + baggage = Baggage(sentry_items={}) baggage.freeze() context = set_value(SENTRY_BAGGAGE_KEY, baggage, context) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 97dae82e10..e42a00a046 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,34 +1,43 @@ from datetime import datetime -from opentelemetry.context import get_value -from opentelemetry.sdk.trace import SpanProcessor -from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.trace import format_span_id, format_trace_id +from opentelemetry.context import get_value, SpanContext # type: ignore +from opentelemetry.sdk.trace import SpanProcessor # type: ignore +from opentelemetry.semconv.trace import SpanAttributes # type: ignore +from opentelemetry.trace import format_span_id, format_trace_id, Span as OTelSpan # type: ignore from sentry_sdk.hub import Hub from sentry_sdk.integrations.opentelemetry.propagator import ( SENTRY_BAGGAGE_KEY, SENTRY_TRACE_KEY, ) -from sentry_sdk.tracing import Transaction +from sentry_sdk.tracing import Transaction, Span as SentrySpan +from sentry_sdk._types import MYPY + +if MYPY: + from typing import Any + from typing import Dict + from typing import Union OPEN_TELEMETRY_CONTEXT = "otel" -class SentrySpanProcessor(SpanProcessor): +class SentrySpanProcessor(SpanProcessor): # type: ignore """ Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. """ - otel_span_map = {} # The mapping from otel span ids to sentry spans + # The mapping from otel span ids to sentry spans + otel_span_map = {} # type: Dict[str, Union[Transaction, OTelSpan]] def __new__(cls): + # type: () -> SentrySpanProcessor if not hasattr(cls, "instance"): cls.instance = super(SentrySpanProcessor, cls).__new__(cls) return cls.instance def on_start(self, otel_span, parent_context=None): + # type: (OTelSpan, SpanContext) -> None hub = Hub.current if not hub: return @@ -62,6 +71,7 @@ def on_start(self, otel_span, parent_context=None): self.otel_span_map[trace_data["span_id"]] = sentry_span def on_end(self, otel_span): + # type: (OTelSpan) -> None span_id = format_span_id(otel_span.context.span_id) sentry_span = self.otel_span_map.pop(span_id) if not sentry_span: @@ -83,6 +93,7 @@ def on_end(self, otel_span): ) def _get_otel_context(self, otel_span): + # type: (OTelSpan) -> Dict[str, Any] """ Returns the OTel context for Sentry. See: https://develop.sentry.dev/sdk/performance/opentelemetry/#step-5-add-opentelemetry-context @@ -98,6 +109,7 @@ def _get_otel_context(self, otel_span): return ctx def _get_trace_data(self, otel_span, parent_context): + # type: (OTelSpan, SpanContext) -> Dict[str, Any] """ Extracts tracing information from one OTel span and its parent OTel context. """ @@ -125,6 +137,7 @@ def _get_trace_data(self, otel_span, parent_context): return trace_data def _update_span_with_otel_data(self, sentry_span, otel_span): + # type: (SentrySpan, OTelSpan) -> None """ Convert OTel span data and update the Sentry span with it. This should eventually happen on the server when ingesting the spans. diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 1111c647d5..afcaff6daa 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -210,8 +210,8 @@ def containing_transaction(self): # referencing themselves) return self._containing_transaction - def start_child(self, instrumenter=None, **kwargs): - # type: (Optional[str], **Any) -> Span + def start_child(self, **kwargs): + # type: (**Any) -> Span """ Start a sub-span from the current span or transaction. @@ -219,10 +219,11 @@ def start_child(self, instrumenter=None, **kwargs): trace id, sampling decision, transaction pointer, and span recorder are inherited from the current span/transaction. """ - instrumenter = ( - instrumenter - if instrumenter is not None - else self.client.options["instrumenter"] + hub = self.hub or sentry_sdk.Hub.current + client = hub.client + + instrumenter = kwargs.get("instrumenter", None) or ( + client and client.options["instrumenter"] ) if instrumenter == INSTRUMENTER.OTEL: # logger.warn("start_child does NOTHING because instrumenter is OTEL") @@ -376,6 +377,7 @@ def to_traceparent(self): @classmethod def extract_sentry_trace(cls, sentry_trace): + # type: (str) -> Tuple[str, str, bool] trace_id, parent_span_id, parent_sampled = sentry_trace.split("-") parent_sampled = True if parent_sampled == "1" else False return (trace_id, parent_span_id, parent_sampled) @@ -484,7 +486,7 @@ def is_success(self): # type: () -> bool return self.status == "ok" - def finish(self, hub=None, end_timestamp=None): + def finish(self, hub=None, **kwargs): # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str] # XXX: would be type: (Optional[sentry_sdk.Hub]) -> None, but that leads # to incompatible return types for Span.finish and Transaction.finish. @@ -495,6 +497,7 @@ def finish(self, hub=None, end_timestamp=None): hub = hub or self.hub or sentry_sdk.Hub.current try: + end_timestamp = kwargs.get("end_timestamp", None) if end_timestamp: self.timestamp = end_timestamp else: @@ -647,7 +650,7 @@ def containing_transaction(self): # reference. return self - def finish(self, hub=None, end_timestamp=None): + def finish(self, hub=None, **kwargs): # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str] if self.timestamp is not None: # This transaction is already finished, ignore. @@ -680,7 +683,7 @@ def finish(self, hub=None, end_timestamp=None): ) self.name = "" - Span.finish(self, hub, end_timestamp) + Span.finish(self, hub, **kwargs) if not self.sampled: # At this point a `sampled = None` should have already been resolved @@ -862,33 +865,43 @@ def _set_initial_sampling_decision(self, sampling_context): class NoOpSpan(Span): def __repr__(self): + # type: () -> Any return self.__class__.__name__ def __enter__(self): + # type: () -> Any return self def __exit__(self, ty, value, tb): + # type: (Any, Any, Any) -> Any pass def start_child(self, **kwargs): + # type: (**Any) -> Any pass def new_span(self, **kwargs): + # type: (**Any) -> Any pass def set_tag(self, key, value): + # type: (Any, Any) -> Any pass def set_data(self, key, value): + # type: (Any, Any) -> Any pass def set_status(self, value): + # type: (Any) -> Any pass def set_http_status(self, http_status): + # type: (Any) -> Any pass - def finish(self, hub=None): + def finish(self, hub=None, **kwargs): + # type: (Any, **Any) -> Any pass From e403c6d9d792d64daac327fadb35f06cf5042229 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 24 Nov 2022 14:57:34 +0100 Subject: [PATCH 21/46] Made typing Py2.7 compatible --- .../integrations/opentelemetry/propagator.py | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 3873da56d9..3a2ec22e1a 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -1,5 +1,3 @@ -import typing - from opentelemetry import trace # type: ignore from opentelemetry.context import ( # type: ignore Context, @@ -23,6 +21,11 @@ from sentry_sdk.tracing import SENTRY_TRACE_HEADER_NAME, Transaction from sentry_sdk.tracing_utils import Baggage +from sentry_sdk._types import MYPY + +if MYPY: + from typing import Optional + from typing import Set BAGGAGE_HEADER_NAME = "baggage" @@ -33,12 +36,8 @@ class SentryPropagator(TextMapPropagator): # type: ignore - def extract( - self, - carrier: CarrierT, - context: typing.Optional[Context] = None, - getter: Getter = default_getter, - ) -> Context: + def extract(self, carrier, context=None, getter=default_getter): + # type: (CarrierT, Optional[Context], Getter) -> Context if context is None: context = get_current() @@ -47,9 +46,10 @@ def extract( return context sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace[0]) + context = set_value(SENTRY_TRACE_KEY, sentry_trace_data, context) - trace_id, span_id, _parent_sampled = sentry_trace_data + trace_id, span_id, _ = sentry_trace_data span_context = SpanContext( trace_id=int(trace_id, 16), @@ -76,12 +76,9 @@ def extract( modified_context = trace.set_span_in_context(span, context) return modified_context - def inject( - self, - carrier: CarrierT, - context: typing.Optional[Context] = None, - setter: Setter = default_setter, - ) -> None: + def inject(self, carrier, context=None, setter=default_setter): + # type: (CarrierT, Optional[Context], Setter) -> None + if context is None: context = get_current() @@ -104,7 +101,8 @@ def inject( setter.set(carrier, BAGGAGE_HEADER_NAME, baggage) @property - def fields(self) -> typing.Set[str]: + def fields(self): + # type: () -> Set[str] return { self.TRACE_ID_KEY, self.SPAN_ID_KEY, From ff6dbe7932967bbc0a06a0da70c2cfe8fce484ad Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 24 Nov 2022 15:22:17 +0100 Subject: [PATCH 22/46] Test outline --- tests/integrations/opentelemetry/__init__.py | 0 .../opentelemetry/test_propagator.py | 34 +++++++++++++++++++ .../opentelemetry/test_span_processor.py | 0 tests/test_tracing.py | 9 +++++ 4 files changed, 43 insertions(+) create mode 100644 tests/integrations/opentelemetry/__init__.py create mode 100644 tests/integrations/opentelemetry/test_propagator.py create mode 100644 tests/integrations/opentelemetry/test_span_processor.py create mode 100644 tests/test_tracing.py diff --git a/tests/integrations/opentelemetry/__init__.py b/tests/integrations/opentelemetry/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py new file mode 100644 index 0000000000..35162a26eb --- /dev/null +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -0,0 +1,34 @@ +# test extract + +# no context, no SENTRY_TRACE_HEADER_NAME in getter +# should return context as is +# some context, no SENTRY_TRACE_HEADER_NAME in getter +# should return context as is + +# no context but sentry trace data and baggage in getter +# return context should have baggage in it and also a noopspan with trace data (span and trace id) + +# no context but sentry trace data and NO baggage in getter +# return context should have baggage in it and also a noopspan with trace data (span and trace id) + + +# some context but sentry trace data and baggage in getter +# return context should have given context and baggage in it and also a noopspan with trace data (span and trace id) + +# some context but sentry trace data and NO baggage in getter +# return context should have given context and empty baggage in it and also a noopspan with trace data (span and trace id) + + +# test inject + +# give empty otel_span_map +# should return none + +# give empty otel_span_map with sentry span and also a context that has span with span id. the span id in otel_span_map and context should NOT match +# should return none + +# give otel_span_map with sentry span with matching span id, without baggage +# check that setter.set is called with sentry span to_traceparent() but no call to setter.set baggage header + +# give otel_span_map with sentry span with baggage +# call to setter.set baggage header diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/test_tracing.py b/tests/test_tracing.py new file mode 100644 index 0000000000..c37ccec506 --- /dev/null +++ b/tests/test_tracing.py @@ -0,0 +1,9 @@ +# @classmethod +# def extract_sentry_trace(cls, sentry_trace): +# # type: (str) -> Tuple[str, str, bool] +# trace_id, parent_span_id, parent_sampled = sentry_trace.split("-") +# parent_sampled = True if parent_sampled == "1" else False +# return (trace_id, parent_span_id, parent_sampled) + + +# Transaction.extract_sentry_trace(sentry_trace[0]) From 73307ec781bf5dddd7076cef743b16fbe9cf5350 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 24 Nov 2022 15:50:53 +0100 Subject: [PATCH 23/46] Test extract_sentry_trace --- tests/test_tracing.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/test_tracing.py b/tests/test_tracing.py index c37ccec506..843da54db8 100644 --- a/tests/test_tracing.py +++ b/tests/test_tracing.py @@ -1,9 +1,19 @@ -# @classmethod -# def extract_sentry_trace(cls, sentry_trace): -# # type: (str) -> Tuple[str, str, bool] -# trace_id, parent_span_id, parent_sampled = sentry_trace.split("-") -# parent_sampled = True if parent_sampled == "1" else False -# return (trace_id, parent_span_id, parent_sampled) +from sentry_sdk.tracing import Transaction -# Transaction.extract_sentry_trace(sentry_trace[0]) +def test_extract_sentry_trace(): + # type: () -> None + + trace_id, parent_span_id, parent_sampled = Transaction.extract_sentry_trace( + "4c79f60c11214eb38604f4ae0781bfb2-fa90fdead5f74052-1" + ) + assert trace_id == "4c79f60c11214eb38604f4ae0781bfb2" + assert parent_span_id == "fa90fdead5f74052" + assert parent_sampled + + trace_id, parent_span_id, parent_sampled = Transaction.extract_sentry_trace( + "5e79f60c11214eb38604f4ae0781bfb2-0390fdead5f74052-0" + ) + assert trace_id == "5e79f60c11214eb38604f4ae0781bfb2" + assert parent_span_id == "0390fdead5f74052" + assert not parent_sampled From 236ce04644fd94ac8bbed4cd0371bede982d7c92 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 25 Nov 2022 12:37:46 +0100 Subject: [PATCH 24/46] Tests for OTel propagator --- .../integrations/opentelemetry/propagator.py | 1 - .../opentelemetry/span_processor.py | 4 +- sentry_sdk/tracing.py | 2 + tests/integrations/opentelemetry/__init__.py | 3 + .../opentelemetry/test_propagator.py | 258 ++++++++++++++++-- tests/test_tracing.py | 19 -- tests/tracing/test_misc.py | 18 ++ tox.ini | 4 + 8 files changed, 265 insertions(+), 44 deletions(-) delete mode 100644 tests/test_tracing.py diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 3a2ec22e1a..3d109eeb11 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -78,7 +78,6 @@ def extract(self, carrier, context=None, getter=default_getter): def inject(self, carrier, context=None, setter=default_setter): # type: (CarrierT, Optional[Context], Setter) -> None - if context is None: context = get_current() diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index e42a00a046..b49ba19a3e 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -1,9 +1,9 @@ from datetime import datetime -from opentelemetry.context import get_value, SpanContext # type: ignore +from opentelemetry.context import get_value # type: ignore from opentelemetry.sdk.trace import SpanProcessor # type: ignore from opentelemetry.semconv.trace import SpanAttributes # type: ignore -from opentelemetry.trace import format_span_id, format_trace_id, Span as OTelSpan # type: ignore +from opentelemetry.trace import format_span_id, format_trace_id, SpanContext, Span as OTelSpan # type: ignore from sentry_sdk.hub import Hub from sentry_sdk.integrations.opentelemetry.propagator import ( diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index afcaff6daa..d40f6179b8 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -128,6 +128,7 @@ def __init__( transaction=None, # type: Optional[str] # deprecated containing_transaction=None, # type: Optional[Transaction] start_timestamp=None, # type: Optional[datetime] + instrumenter=None, # type: Optional[str] ): # type: (...) -> None self.trace_id = trace_id or uuid.uuid4().hex @@ -225,6 +226,7 @@ def start_child(self, **kwargs): instrumenter = kwargs.get("instrumenter", None) or ( client and client.options["instrumenter"] ) + if instrumenter == INSTRUMENTER.OTEL: # logger.warn("start_child does NOTHING because instrumenter is OTEL") return NoOpSpan() diff --git a/tests/integrations/opentelemetry/__init__.py b/tests/integrations/opentelemetry/__init__.py index e69de29bb2..39ecc610d5 100644 --- a/tests/integrations/opentelemetry/__init__.py +++ b/tests/integrations/opentelemetry/__init__.py @@ -0,0 +1,3 @@ +import pytest + +django = pytest.importorskip("opentelemetry") diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 35162a26eb..0022f9e189 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -1,34 +1,248 @@ -# test extract +from mock import MagicMock +import mock -# no context, no SENTRY_TRACE_HEADER_NAME in getter -# should return context as is -# some context, no SENTRY_TRACE_HEADER_NAME in getter -# should return context as is +from opentelemetry.context import get_current +from opentelemetry.trace.propagation import get_current_span +from opentelemetry.trace import ( + set_span_in_context, + TraceFlags, + SpanContext, +) -# no context but sentry trace data and baggage in getter -# return context should have baggage in it and also a noopspan with trace data (span and trace id) +from sentry_sdk.integrations.opentelemetry.propagator import ( + SENTRY_BAGGAGE_KEY, + SENTRY_TRACE_KEY, + SentryPropagator, +) +from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor -# no context but sentry trace data and NO baggage in getter -# return context should have baggage in it and also a noopspan with trace data (span and trace id) +def test_extract_no_context_no_sentry_trace_header(): + # type: () -> None + """ + No context and NO Sentry trace data in getter. + Extract should return empty context. + """ + carrier = None + context = None + getter = MagicMock() + getter.get.return_value = None -# some context but sentry trace data and baggage in getter -# return context should have given context and baggage in it and also a noopspan with trace data (span and trace id) + modified_context = SentryPropagator().extract(carrier, context, getter) -# some context but sentry trace data and NO baggage in getter -# return context should have given context and empty baggage in it and also a noopspan with trace data (span and trace id) + assert modified_context == {} -# test inject +def test_extract_context_no_sentry_trace_header(): + # type: () -> None + """ + Context but NO Sentry trace data in getter. + Extract should return context as is. + """ + carrier = None + context = {"some": "value"} + getter = MagicMock() + getter.get.return_value = None -# give empty otel_span_map -# should return none + modified_context = SentryPropagator().extract(carrier, context, getter) -# give empty otel_span_map with sentry span and also a context that has span with span id. the span id in otel_span_map and context should NOT match -# should return none + assert modified_context == context -# give otel_span_map with sentry span with matching span id, without baggage -# check that setter.set is called with sentry span to_traceparent() but no call to setter.set baggage header -# give otel_span_map with sentry span with baggage -# call to setter.set baggage header +def test_extract_empty_context_sentry_trace_header_no_baggage(): + # type: () -> None + """ + Empty context but Sentry trace data but NO Baggage in getter. + Extract should return context that has empty baggage in it and also a NoopSpan with span_id and trace_id. + """ + carrier = None + context = {} + getter = MagicMock() + getter.get.return_value = ["1234567890abcdef1234567890abcdef-1234567890abcdef-1"] + + modified_context = SentryPropagator().extract(carrier, context, getter) + + assert len(modified_context.keys()) == 3 + + assert modified_context[SENTRY_TRACE_KEY] == ( + "1234567890abcdef1234567890abcdef", + "1234567890abcdef", + True, + ) + assert modified_context[SENTRY_BAGGAGE_KEY].serialize() == "" + + span_context = get_current_span(modified_context).get_span_context() + assert span_context.span_id == int("1234567890abcdef", 16) + assert span_context.trace_id == int("1234567890abcdef1234567890abcdef", 16) + + +def test_extract_context_sentry_trace_header_baggage(): + # type: () -> None + """ + Empty context but Sentry trace data and Baggage in getter. + Extract should return context that has baggage in it and also a NoopSpan with span_id and trace_id. + """ + baggage_header = ( + "other-vendor-value-1=foo;bar;baz, sentry-trace_id=771a43a4192642f0b136d5159a501700, " + "sentry-public_key=49d0f7386ad645858ae85020e393bef3, sentry-sample_rate=0.01337, " + "sentry-user_id=Am%C3%A9lie, other-vendor-value-2=foo;bar;" + ) + + carrier = None + context = {"some": "value"} + getter = MagicMock() + getter.get.side_effect = [ + ["1234567890abcdef1234567890abcdef-1234567890abcdef-1"], + [baggage_header], + ] + + modified_context = SentryPropagator().extract(carrier, context, getter) + + assert len(modified_context.keys()) == 4 + + assert modified_context[SENTRY_TRACE_KEY] == ( + "1234567890abcdef1234567890abcdef", + "1234567890abcdef", + True, + ) + assert modified_context[SENTRY_BAGGAGE_KEY].serialize() == ( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + "sentry-sample_rate=0.01337,sentry-user_id=Am%C3%A9lie" + ) + + span_context = get_current_span(modified_context).get_span_context() + assert span_context.span_id == int("1234567890abcdef", 16) + assert span_context.trace_id == int("1234567890abcdef1234567890abcdef", 16) + + +def test_inject_empty_otel_span_map(): + # type: () -> None + """ + Empty otel_span_map. + So there is no sentry_span to be found in inject() + and the function is returned early and no setters are called. + """ + carrier = None + context = get_current() + setter = MagicMock() + setter.set = MagicMock() + + span_context = SpanContext( + trace_id=int("1234567890abcdef1234567890abcdef", 16), + span_id=int("1234567890abcdef", 16), + trace_flags=TraceFlags(TraceFlags.SAMPLED), + is_remote=True, + ) + span = MagicMock() + span.context = span_context + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.propagator.trace.get_current_span", + return_value=span, + ): + full_context = set_span_in_context(span, context) + SentryPropagator().inject(carrier, full_context, setter) + + setter.set.assert_not_called() + + +def test_inject_sentry_span_no_baggage(): + # type: () -> None + """ + Inject a sentry span with no baggage. + """ + carrier = None + context = get_current() + setter = MagicMock() + setter.set = MagicMock() + + trace_id = "1234567890abcdef1234567890abcdef" + span_id = "1234567890abcdef" + + span_context = SpanContext( + trace_id=int(trace_id, 16), + span_id=int(span_id, 16), + trace_flags=TraceFlags(TraceFlags.SAMPLED), + is_remote=True, + ) + span = MagicMock() + span.context = span_context + + sentry_span = MagicMock() + sentry_span.to_traceparent = mock.Mock( + return_value="1234567890abcdef1234567890abcdef-1234567890abcdef-1" + ) + sentry_span.get_baggage = mock.Mock(return_value=None) + + span_processor = SentrySpanProcessor() + span_processor.otel_span_map[span_id] = sentry_span + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.propagator.trace.get_current_span", + return_value=span, + ): + full_context = set_span_in_context(span, context) + SentryPropagator().inject(carrier, full_context, setter) + + setter.set.assert_called_once_with( + carrier, + "sentry-trace", + "1234567890abcdef1234567890abcdef-1234567890abcdef-1", + ) + + +def test_inject_sentry_span_baggage(): + # type: () -> None + """ + Inject a sentry span with baggage. + """ + carrier = None + context = get_current() + setter = MagicMock() + setter.set = MagicMock() + + trace_id = "1234567890abcdef1234567890abcdef" + span_id = "1234567890abcdef" + + span_context = SpanContext( + trace_id=int(trace_id, 16), + span_id=int(span_id, 16), + trace_flags=TraceFlags(TraceFlags.SAMPLED), + is_remote=True, + ) + span = MagicMock() + span.context = span_context + + sentry_span = MagicMock() + sentry_span.to_traceparent = mock.Mock( + return_value="1234567890abcdef1234567890abcdef-1234567890abcdef-1" + ) + baggage = ( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + "sentry-sample_rate=0.01337,sentry-user_id=Am%C3%A9lie" + ) + sentry_span.get_baggage = mock.Mock(return_value=baggage) + + span_processor = SentrySpanProcessor() + span_processor.otel_span_map[span_id] = sentry_span + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.propagator.trace.get_current_span", + return_value=span, + ): + full_context = set_span_in_context(span, context) + SentryPropagator().inject(carrier, full_context, setter) + + setter.set.assert_any_call( + carrier, + "sentry-trace", + "1234567890abcdef1234567890abcdef-1234567890abcdef-1", + ) + + setter.set.assert_any_call( + carrier, + "baggage", + baggage, + ) diff --git a/tests/test_tracing.py b/tests/test_tracing.py deleted file mode 100644 index 843da54db8..0000000000 --- a/tests/test_tracing.py +++ /dev/null @@ -1,19 +0,0 @@ -from sentry_sdk.tracing import Transaction - - -def test_extract_sentry_trace(): - # type: () -> None - - trace_id, parent_span_id, parent_sampled = Transaction.extract_sentry_trace( - "4c79f60c11214eb38604f4ae0781bfb2-fa90fdead5f74052-1" - ) - assert trace_id == "4c79f60c11214eb38604f4ae0781bfb2" - assert parent_span_id == "fa90fdead5f74052" - assert parent_sampled - - trace_id, parent_span_id, parent_sampled = Transaction.extract_sentry_trace( - "5e79f60c11214eb38604f4ae0781bfb2-0390fdead5f74052-0" - ) - assert trace_id == "5e79f60c11214eb38604f4ae0781bfb2" - assert parent_span_id == "0390fdead5f74052" - assert not parent_sampled diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index b51b5dcddb..ca3217acb3 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -274,3 +274,21 @@ def test_set_meaurement(sentry_init, capture_events): assert event["measurements"]["metric.bar"] == {"value": 456, "unit": "second"} assert event["measurements"]["metric.baz"] == {"value": 420.69, "unit": "custom"} assert event["measurements"]["metric.foobar"] == {"value": 17.99, "unit": "percent"} + + +def test_extract_sentry_trace(): + # type: () -> None + + trace_id, parent_span_id, parent_sampled = Transaction.extract_sentry_trace( + "4c79f60c11214eb38604f4ae0781bfb2-fa90fdead5f74052-1" + ) + assert trace_id == "4c79f60c11214eb38604f4ae0781bfb2" + assert parent_span_id == "fa90fdead5f74052" + assert parent_sampled + + trace_id, parent_span_id, parent_sampled = Transaction.extract_sentry_trace( + "5e79f60c11214eb38604f4ae0781bfb2-0390fdead5f74052-0" + ) + assert trace_id == "5e79f60c11214eb38604f4ae0781bfb2" + assert parent_span_id == "0390fdead5f74052" + assert not parent_sampled diff --git a/tox.ini b/tox.ini index 98505caab1..34a9166902 100644 --- a/tox.ini +++ b/tox.ini @@ -101,6 +101,8 @@ envlist = {py3.6,py3.7,py3.8,py3.9,py3.10}-pymongo-{4.0} {py3.7,py3.8,py3.9,py3.10}-pymongo-{4.1,4.2} + {py3.7,py3.8,py3.9,py3.10}-opentelemetry + [testenv] deps = # if you change test-requirements.txt and your change is not being reflected @@ -293,6 +295,8 @@ deps = pymongo-4.1: pymongo>=4.1,<4.2 pymongo-4.2: pymongo>=4.2,<4.3 + opentelemetry: opentelemetry-distro + setenv = PYTHONDONTWRITEBYTECODE=1 TESTPATH=tests From de399b92d5ce8e6f49550967b26469b6ded36558 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 25 Nov 2022 13:09:08 +0100 Subject: [PATCH 25/46] Fixed one test --- tests/integrations/opentelemetry/test_propagator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 0022f9e189..17355db9d0 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -58,7 +58,10 @@ def test_extract_empty_context_sentry_trace_header_no_baggage(): carrier = None context = {} getter = MagicMock() - getter.get.return_value = ["1234567890abcdef1234567890abcdef-1234567890abcdef-1"] + getter.get.side_effect = [ + ["1234567890abcdef1234567890abcdef-1234567890abcdef-1"], + None, + ] modified_context = SentryPropagator().extract(carrier, context, getter) From b3b5453cd29c372a1b25da9bd26d49e80bf8f040 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 25 Nov 2022 16:05:12 +0100 Subject: [PATCH 26/46] Added some tests for span processor --- .../opentelemetry/span_processor.py | 2 +- .../opentelemetry/test_propagator.py | 7 - .../opentelemetry/test_span_processor.py | 258 ++++++++++++++++++ 3 files changed, 259 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index b49ba19a3e..209f9fade0 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -153,7 +153,7 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): db_query = otel_span.attributes.get(SpanAttributes.DB_SYSTEM, None) if http_method: - op = "http.{}".format(http_method) + op = "http.{}".format(http_method.lower()) description = http_method peer_name = otel_span.attributes.get(SpanAttributes.NET_PEER_NAME, None) diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 17355db9d0..5cf50021a5 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -18,7 +18,6 @@ def test_extract_no_context_no_sentry_trace_header(): - # type: () -> None """ No context and NO Sentry trace data in getter. Extract should return empty context. @@ -34,7 +33,6 @@ def test_extract_no_context_no_sentry_trace_header(): def test_extract_context_no_sentry_trace_header(): - # type: () -> None """ Context but NO Sentry trace data in getter. Extract should return context as is. @@ -50,7 +48,6 @@ def test_extract_context_no_sentry_trace_header(): def test_extract_empty_context_sentry_trace_header_no_baggage(): - # type: () -> None """ Empty context but Sentry trace data but NO Baggage in getter. Extract should return context that has empty baggage in it and also a NoopSpan with span_id and trace_id. @@ -80,7 +77,6 @@ def test_extract_empty_context_sentry_trace_header_no_baggage(): def test_extract_context_sentry_trace_header_baggage(): - # type: () -> None """ Empty context but Sentry trace data and Baggage in getter. Extract should return context that has baggage in it and also a NoopSpan with span_id and trace_id. @@ -120,7 +116,6 @@ def test_extract_context_sentry_trace_header_baggage(): def test_inject_empty_otel_span_map(): - # type: () -> None """ Empty otel_span_map. So there is no sentry_span to be found in inject() @@ -151,7 +146,6 @@ def test_inject_empty_otel_span_map(): def test_inject_sentry_span_no_baggage(): - # type: () -> None """ Inject a sentry span with no baggage. """ @@ -196,7 +190,6 @@ def test_inject_sentry_span_no_baggage(): def test_inject_sentry_span_baggage(): - # type: () -> None """ Inject a sentry span with baggage. """ diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index e69de29bb2..b1c97385c3 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -0,0 +1,258 @@ +from datetime import datetime +from mock import MagicMock +import mock +import time +from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor +from sentry_sdk.tracing import Span + + +def test_get_otel_context(): + otel_span = MagicMock() + otel_span.attributes = {"foo": "bar"} + otel_span.resource = MagicMock() + otel_span.resource.attributes = {"baz": "qux"} + + span_processor = SentrySpanProcessor() + otel_context = span_processor._get_otel_context(otel_span) + + assert otel_context == { + "attributes": {"foo": "bar"}, + "resource": {"baz": "qux"}, + } + + +def test_get_trace_data_with_span_and_trace(): + otel_span = MagicMock() + otel_span.context = MagicMock() + otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16) + otel_span.context.span_id = int("1234567890abcdef", 16) + otel_span.parent = None + + parent_context = {} + + span_processor = SentrySpanProcessor() + sentry_trace_data = span_processor._get_trace_data(otel_span, parent_context) + assert sentry_trace_data["trace_id"] == "1234567890abcdef1234567890abcdef" + assert sentry_trace_data["span_id"] == "1234567890abcdef" + assert sentry_trace_data["parent_span_id"] is None + assert sentry_trace_data["parent_sampled"] is None + assert sentry_trace_data["baggage"] is None + + +def test_get_trace_data_with_span_and_trace_and_parent(): + otel_span = MagicMock() + otel_span.context = MagicMock() + otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16) + otel_span.context.span_id = int("1234567890abcdef", 16) + otel_span.parent = MagicMock() + otel_span.parent.span_id = int("abcdef1234567890", 16) + + parent_context = {} + + span_processor = SentrySpanProcessor() + sentry_trace_data = span_processor._get_trace_data(otel_span, parent_context) + assert sentry_trace_data["trace_id"] == "1234567890abcdef1234567890abcdef" + assert sentry_trace_data["span_id"] == "1234567890abcdef" + assert sentry_trace_data["parent_span_id"] == "abcdef1234567890" + assert sentry_trace_data["parent_sampled"] is None + assert sentry_trace_data["baggage"] is None + + +def test_get_trace_data_with_sentry_trace(): + otel_span = MagicMock() + otel_span.context = MagicMock() + otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16) + otel_span.context.span_id = int("1234567890abcdef", 16) + otel_span.parent = MagicMock() + otel_span.parent.span_id = int("abcdef1234567890", 16) + + parent_context = {} + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.get_value", + side_effect=[ + ("1234567890abcdef1234567890abcdef", "1234567890abcdef", True), + None, + ], + ): + span_processor = SentrySpanProcessor() + sentry_trace_data = span_processor._get_trace_data(otel_span, parent_context) + assert sentry_trace_data["trace_id"] == "1234567890abcdef1234567890abcdef" + assert sentry_trace_data["span_id"] == "1234567890abcdef" + assert sentry_trace_data["parent_span_id"] == "abcdef1234567890" + assert sentry_trace_data["parent_sampled"] is True + assert sentry_trace_data["baggage"] is None + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.get_value", + side_effect=[ + ("1234567890abcdef1234567890abcdef", "1234567890abcdef", False), + None, + ], + ): + span_processor = SentrySpanProcessor() + sentry_trace_data = span_processor._get_trace_data(otel_span, parent_context) + assert sentry_trace_data["trace_id"] == "1234567890abcdef1234567890abcdef" + assert sentry_trace_data["span_id"] == "1234567890abcdef" + assert sentry_trace_data["parent_span_id"] == "abcdef1234567890" + assert sentry_trace_data["parent_sampled"] is False + assert sentry_trace_data["baggage"] is None + + +def test_get_trace_data_with_sentry_trace_and_baggage(): + otel_span = MagicMock() + otel_span.context = MagicMock() + otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16) + otel_span.context.span_id = int("1234567890abcdef", 16) + otel_span.parent = MagicMock() + otel_span.parent.span_id = int("abcdef1234567890", 16) + + parent_context = {} + + baggage = ( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=49d0f7386ad645858ae85020e393bef3," + "sentry-sample_rate=0.01337,sentry-user_id=Am%C3%A9lie" + ) + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.get_value", + side_effect=[ + ("1234567890abcdef1234567890abcdef", "1234567890abcdef", True), + baggage, + ], + ): + span_processor = SentrySpanProcessor() + sentry_trace_data = span_processor._get_trace_data(otel_span, parent_context) + assert sentry_trace_data["trace_id"] == "1234567890abcdef1234567890abcdef" + assert sentry_trace_data["span_id"] == "1234567890abcdef" + assert sentry_trace_data["parent_span_id"] == "abcdef1234567890" + assert sentry_trace_data["parent_sampled"] + assert sentry_trace_data["baggage"] == baggage + + +def test_update_span_with_otel_data_http_method(): + sentry_span = Span() + + otel_span = MagicMock() + otel_span.name = "Test OTel Span" + otel_span.attributes = { + "http.method": "GET", + "http.status_code": 429, + "http.status_text": "xxx", + "http.user_agent": "curl/7.64.1", + "net.peer.name": "example.com", + "http.target": "/", + } + + span_processor = SentrySpanProcessor() + span_processor._update_span_with_otel_data(sentry_span, otel_span) + + assert sentry_span.op == "http.get" + assert sentry_span.description == "GET example.com /" + assert sentry_span._tags["http.status_code"] == "429" + assert sentry_span.status == "resource_exhausted" + + assert sentry_span._data["http.method"] == "GET" + assert sentry_span._data["http.status_code"] == 429 + assert sentry_span._data["http.status_text"] == "xxx" + assert sentry_span._data["http.user_agent"] == "curl/7.64.1" + assert sentry_span._data["net.peer.name"] == "example.com" + assert sentry_span._data["http.target"] == "/" + + +def test_update_span_with_otel_data_db_query(): + sentry_span = Span() + + otel_span = MagicMock() + otel_span.name = "Test OTel Span" + otel_span.attributes = { + "db.system": "postgresql", + "db.statement": "SELECT * FROM table where pwd = '123456'", + } + + span_processor = SentrySpanProcessor() + span_processor._update_span_with_otel_data(sentry_span, otel_span) + + assert sentry_span.op == "db" + assert sentry_span.description == "SELECT * FROM table where pwd = '123456'" + + assert sentry_span._data["db.system"] == "postgresql" + assert ( + sentry_span._data["db.statement"] == "SELECT * FROM table where pwd = '123456'" + ) + + +def test_on_start_transaction(): + otel_span = MagicMock() + otel_span.name = "Sample OTel Span" + otel_span.start_time = time.time_ns() + otel_span.context = MagicMock() + otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16) + otel_span.context.span_id = int("1234567890abcdef", 16) + otel_span.parent = MagicMock() + otel_span.parent.span_id = int("abcdef1234567890", 16) + + parent_context = {} + + fakeHub = MagicMock() + fakeHub.current.return_value = MagicMock() + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fakeHub + ): + span_processor = SentrySpanProcessor() + span_processor.on_start(otel_span, parent_context) + + fakeHub.current.start_transaction.assert_called_once_with( + name="Sample OTel Span", + span_id="1234567890abcdef", + parent_span_id="abcdef1234567890", + trace_id="1234567890abcdef1234567890abcdef", + baggage=None, + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), + instrumenter="sentry", + ) + + assert len(span_processor.otel_span_map.keys()) == 1 + assert list(span_processor.otel_span_map.keys())[0] == "1234567890abcdef" + + +def test_on_start_child(): + otel_span = MagicMock() + otel_span.name = "Sample OTel Span" + otel_span.start_time = time.time_ns() + otel_span.context = MagicMock() + otel_span.context.trace_id = int("1234567890abcdef1234567890abcdef", 16) + otel_span.context.span_id = int("1234567890abcdef", 16) + otel_span.parent = MagicMock() + otel_span.parent.span_id = int("abcdef1234567890", 16) + + parent_context = {} + + fakeHub = MagicMock() + fakeHub.current.return_value = MagicMock() + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fakeHub + ): + fakeSpan = MagicMock() + + span_processor = SentrySpanProcessor() + span_processor.otel_span_map["abcdef1234567890"] = fakeSpan + span_processor.on_start(otel_span, parent_context) + + fakeSpan.start_child.assert_called_once_with( + span_id="1234567890abcdef", + description="Sample OTel Span", + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), + instrumenter="sentry", + ) + + assert len(span_processor.otel_span_map.keys()) == 2 + assert "abcdef1234567890" in span_processor.otel_span_map.keys() + assert "1234567890abcdef" in span_processor.otel_span_map.keys() + + +def test_on_end(): + assert "TODO" == "THIS NEEDS TO BE IMPLEMENTED" From 5da4ffe7ed68c3421de96e4bcb95df9fe6061ede Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 25 Nov 2022 16:14:32 +0100 Subject: [PATCH 27/46] Made variable name actually pythonic. --- .../opentelemetry/test_span_processor.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index b1c97385c3..96427cd645 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -195,16 +195,16 @@ def test_on_start_transaction(): parent_context = {} - fakeHub = MagicMock() - fakeHub.current.return_value = MagicMock() + fake_hub = MagicMock() + fake_hub.current.return_value = MagicMock() with mock.patch( - "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fakeHub + "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub ): span_processor = SentrySpanProcessor() span_processor.on_start(otel_span, parent_context) - fakeHub.current.start_transaction.assert_called_once_with( + fake_hub.current.start_transaction.assert_called_once_with( name="Sample OTel Span", span_id="1234567890abcdef", parent_span_id="abcdef1234567890", @@ -230,11 +230,11 @@ def test_on_start_child(): parent_context = {} - fakeHub = MagicMock() - fakeHub.current.return_value = MagicMock() + fake_hub = MagicMock() + fake_hub.current.return_value = MagicMock() with mock.patch( - "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fakeHub + "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub ): fakeSpan = MagicMock() From edca5b011f07093ca430402687a175fbeb10bece Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 25 Nov 2022 16:16:32 +0100 Subject: [PATCH 28/46] Added opentelementry test config --- .../test-integration-opentelemetry.yml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 .github/workflows/test-integration-opentelemetry.yml diff --git a/.github/workflows/test-integration-opentelemetry.yml b/.github/workflows/test-integration-opentelemetry.yml new file mode 100644 index 0000000000..987d92b95c --- /dev/null +++ b/.github/workflows/test-integration-opentelemetry.yml @@ -0,0 +1,62 @@ +name: Test opentelemetry + +on: + push: + branches: + - master + - release/** + + pull_request: + +# Cancel in progress workflows on pull_requests. +# https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +permissions: + contents: read + +env: + BUILD_CACHE_KEY: ${{ github.sha }} + CACHED_BUILD_PATHS: | + ${{ github.workspace }}/dist-serverless + +jobs: + test: + name: opentelemetry, python ${{ matrix.python-version }}, ${{ matrix.os }} + runs-on: ${{ matrix.os }} + timeout-minutes: 45 + continue-on-error: true + + strategy: + matrix: + python-version: ["3.7","3.8","3.9","3.10"] + os: [ubuntu-latest] + + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Setup Test Env + env: + PGHOST: localhost + PGPASSWORD: sentry + run: | + pip install codecov tox + + - name: Test opentelemetry + env: + CI_PYTHON_VERSION: ${{ matrix.python-version }} + timeout-minutes: 45 + shell: bash + run: | + set -x # print commands that are executed + coverage erase + + ./scripts/runtox.sh "${{ matrix.python-version }}-opentelemetry" --cov=tests --cov=sentry_sdk --cov-report= --cov-branch + coverage combine .coverage* + coverage xml -i + codecov --file coverage.xml From 1f5285e4390212ab161480ba46ca3df8102efd04 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 25 Nov 2022 16:18:45 +0100 Subject: [PATCH 29/46] Made variable name actually pythonic. --- tests/integrations/opentelemetry/test_span_processor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 96427cd645..2dda029a7d 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -236,13 +236,13 @@ def test_on_start_child(): with mock.patch( "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub ): - fakeSpan = MagicMock() + fake_span = MagicMock() span_processor = SentrySpanProcessor() - span_processor.otel_span_map["abcdef1234567890"] = fakeSpan + span_processor.otel_span_map["abcdef1234567890"] = fake_span span_processor.on_start(otel_span, parent_context) - fakeSpan.start_child.assert_called_once_with( + fake_span.start_child.assert_called_once_with( span_id="1234567890abcdef", description="Sample OTel Span", start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), From 1eb729535d3ee04c1c0ac8514c34d42b2ea50c9a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 10:13:26 +0100 Subject: [PATCH 30/46] Tests for on_end --- .../opentelemetry/span_processor.py | 2 +- .../opentelemetry/test_span_processor.py | 81 ++++++++++++++++++- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 209f9fade0..3c1059aba0 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -73,7 +73,7 @@ def on_start(self, otel_span, parent_context=None): def on_end(self, otel_span): # type: (OTelSpan) -> None span_id = format_span_id(otel_span.context.span_id) - sentry_span = self.otel_span_map.pop(span_id) + sentry_span = self.otel_span_map.pop(span_id, None) if not sentry_span: return diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 2dda029a7d..b52f613104 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -3,7 +3,7 @@ import mock import time from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor -from sentry_sdk.tracing import Span +from sentry_sdk.tracing import Span, Transaction def test_get_otel_context(): @@ -254,5 +254,80 @@ def test_on_start_child(): assert "1234567890abcdef" in span_processor.otel_span_map.keys() -def test_on_end(): - assert "TODO" == "THIS NEEDS TO BE IMPLEMENTED" +def test_on_end_no_sentry_span(): + """ + If on_end is called on a span that is not in the otel_span_map, it should be a no-op. + """ + otel_span = MagicMock() + otel_span.name = "Sample OTel Span" + otel_span.end_time = time.time_ns() + otel_span.context = MagicMock() + otel_span.context.span_id = int("1234567890abcdef", 16) + + span_processor = SentrySpanProcessor() + span_processor.otel_span_map = {} + span_processor._get_otel_context = MagicMock() + span_processor._update_span_with_otel_data = MagicMock() + + span_processor.on_end(otel_span) + + span_processor._get_otel_context.assert_not_called() + span_processor._update_span_with_otel_data.assert_not_called() + + +def test_on_end_sentry_transaction(): + """ + Test on_end for a sentry Transaction. + """ + otel_span = MagicMock() + otel_span.name = "Sample OTel Span" + otel_span.end_time = time.time_ns() + otel_span.context = MagicMock() + otel_span.context.span_id = int("1234567890abcdef", 16) + + fake_sentry_span = MagicMock(spec=Transaction) + fake_sentry_span.set_context = MagicMock() + fake_sentry_span.finish = MagicMock() + + span_processor = SentrySpanProcessor() + span_processor._get_otel_context = MagicMock() + span_processor._update_span_with_otel_data = MagicMock() + span_processor.otel_span_map["1234567890abcdef"] = fake_sentry_span + + span_processor.on_end(otel_span) + + fake_sentry_span.set_context.assert_called_once() + span_processor._update_span_with_otel_data.assert_not_called() + fake_sentry_span.finish.assert_called_once_with( + end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) + ) + + +def test_on_end_sentry_Span(): + """ + Test on_end for a sentry Span. + """ + otel_span = MagicMock() + otel_span.name = "Sample OTel Span" + otel_span.end_time = time.time_ns() + otel_span.context = MagicMock() + otel_span.context.span_id = int("1234567890abcdef", 16) + + fake_sentry_span = MagicMock(spec=Span) + fake_sentry_span.set_context = MagicMock() + fake_sentry_span.finish = MagicMock() + + span_processor = SentrySpanProcessor() + span_processor._get_otel_context = MagicMock() + span_processor._update_span_with_otel_data = MagicMock() + span_processor.otel_span_map["1234567890abcdef"] = fake_sentry_span + + span_processor.on_end(otel_span) + + fake_sentry_span.set_context.assert_not_called() + span_processor._update_span_with_otel_data.assert_called_once_with( + fake_sentry_span, otel_span + ) + fake_sentry_span.finish.assert_called_once_with( + end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) + ) From 268f8d0526a6ba5d97f0bb4d21614bb345f0b231 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 10:22:02 +0100 Subject: [PATCH 31/46] Fixed typo --- tests/integrations/opentelemetry/test_span_processor.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index b52f613104..2c9d78cac5 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -299,11 +299,13 @@ def test_on_end_sentry_transaction(): fake_sentry_span.set_context.assert_called_once() span_processor._update_span_with_otel_data.assert_not_called() fake_sentry_span.finish.assert_called_once_with( - end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) + end_timestamp=datetime.fromtimestamp( + otel_span.entest_on_end_sentry_Span_time / 1e9 + ) ) -def test_on_end_sentry_Span(): +def test_on_end_sentry_span(): """ Test on_end for a sentry Span. """ From 17f0346b1ef596b8f4f99da2e4c8b0b53081376f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 10:34:18 +0100 Subject: [PATCH 32/46] Updated assert commands --- .../integrations/opentelemetry/test_span_processor.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 2c9d78cac5..40695acce8 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -298,11 +298,7 @@ def test_on_end_sentry_transaction(): fake_sentry_span.set_context.assert_called_once() span_processor._update_span_with_otel_data.assert_not_called() - fake_sentry_span.finish.assert_called_once_with( - end_timestamp=datetime.fromtimestamp( - otel_span.entest_on_end_sentry_Span_time / 1e9 - ) - ) + fake_sentry_span.finish.assert_called_once_with() def test_on_end_sentry_span(): @@ -330,6 +326,4 @@ def test_on_end_sentry_span(): span_processor._update_span_with_otel_data.assert_called_once_with( fake_sentry_span, otel_span ) - fake_sentry_span.finish.assert_called_once_with( - end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) - ) + fake_sentry_span.finish.assert_called_once() From d1c194256175b7e4e6d4b011f0b95c625f3599d6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 10:38:24 +0100 Subject: [PATCH 33/46] Fixed typo --- tests/integrations/opentelemetry/test_span_processor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 40695acce8..efb1b27679 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -298,7 +298,7 @@ def test_on_end_sentry_transaction(): fake_sentry_span.set_context.assert_called_once() span_processor._update_span_with_otel_data.assert_not_called() - fake_sentry_span.finish.assert_called_once_with() + fake_sentry_span.finish.assert_called_once() def test_on_end_sentry_span(): From 4b662b52f802ad71c6a6f0c930034365dffe740a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 11:50:28 +0100 Subject: [PATCH 34/46] Trigger CI again. From 0c67bb85c3fcd390045236dc460d178205e323a7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 16:01:54 +0100 Subject: [PATCH 35/46] Make some ifs to work same as in other SDKs for consistency --- sentry_sdk/hub.py | 17 +++++++------- .../integrations/opentelemetry/propagator.py | 8 ++----- .../opentelemetry/span_processor.py | 22 ++++++++++++++++--- sentry_sdk/tracing.py | 9 +++----- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 5da32603de..8261c107a6 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -5,7 +5,6 @@ from contextlib import contextmanager from sentry_sdk._compat import with_metaclass -from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.scope import Scope from sentry_sdk.client import Client from sentry_sdk.tracing import NoOpSpan, Span, Transaction @@ -465,12 +464,12 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ - instrumenter = kwargs.get("instrumenter", None) or ( - self.client and self.client.options["instrumenter"] + instrumenter = kwargs.get("instrumenter", None) + configuration_instrumenter = self.client and self.client.options.get( + "instrumenter", None ) - if instrumenter == INSTRUMENTER.OTEL: - # logger.warn("start_child does NOTHING because instrumenter is OTEL") + if instrumenter != configuration_instrumenter: return NoOpSpan() # TODO: consider removing this in a future release. @@ -528,12 +527,12 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ - instrumenter = kwargs.get("instrumenter", None) or ( - self.client and self.client.options["instrumenter"] + instrumenter = kwargs.get("instrumenter", None) + configuration_instrumenter = self.client and self.client.options.get( + "instrumenter", None ) - if instrumenter == INSTRUMENTER.OTEL: - # logger.warn("start_child does NOTHING because instrumenter is OTEL") + if instrumenter != configuration_instrumenter: return NoOpSpan() custom_sampling_context = kwargs.pop("custom_sampling_context", {}) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 3d109eeb11..7b32cb56ed 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -97,13 +97,9 @@ def inject(self, carrier, context=None, setter=default_setter): baggage = sentry_span.get_baggage() if baggage: - setter.set(carrier, BAGGAGE_HEADER_NAME, baggage) + setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize()) @property def fields(self): # type: () -> Set[str] - return { - self.TRACE_ID_KEY, - self.SPAN_ID_KEY, - self.SAMPLED_KEY, - } + return {SENTRY_TRACE_HEADER_NAME, BAGGAGE_HEADER_NAME} diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 3c1059aba0..3e5100f966 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -3,7 +3,13 @@ from opentelemetry.context import get_value # type: ignore from opentelemetry.sdk.trace import SpanProcessor # type: ignore from opentelemetry.semconv.trace import SpanAttributes # type: ignore -from opentelemetry.trace import format_span_id, format_trace_id, SpanContext, Span as OTelSpan # type: ignore +from opentelemetry.trace import ( + format_span_id, + format_trace_id, + SpanContext, + Span as OTelSpan, +) +from sentry_sdk.consts import INSTRUMENTER # type: ignore from sentry_sdk.hub import Hub from sentry_sdk.integrations.opentelemetry.propagator import ( @@ -42,6 +48,9 @@ def on_start(self, otel_span, parent_context=None): if not hub: return + if hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL: + return + trace_data = self._get_trace_data(otel_span, parent_context) parent_span_id = trace_data["parent_span_id"] @@ -55,7 +64,7 @@ def on_start(self, otel_span, parent_context=None): span_id=trace_data["span_id"], description=otel_span.name, start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), - instrumenter="sentry", + instrumenter=INSTRUMENTER.SENTRY, ) else: sentry_span = hub.start_transaction( @@ -65,13 +74,20 @@ def on_start(self, otel_span, parent_context=None): trace_id=trace_data["trace_id"], baggage=trace_data["baggage"], start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), - instrumenter="sentry", + instrumenter=INSTRUMENTER.SENTRY, ) self.otel_span_map[trace_data["span_id"]] = sentry_span def on_end(self, otel_span): # type: (OTelSpan) -> None + hub = Hub.current + if not hub: + return + + if hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL: + return + span_id = format_span_id(otel_span.context.span_id) sentry_span = self.otel_span_map.pop(span_id, None) if not sentry_span: diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index d40f6179b8..3dfdd2a1df 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -6,7 +6,6 @@ from datetime import datetime, timedelta import sentry_sdk -from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.utils import logger from sentry_sdk._types import MYPY @@ -223,12 +222,10 @@ def start_child(self, **kwargs): hub = self.hub or sentry_sdk.Hub.current client = hub.client - instrumenter = kwargs.get("instrumenter", None) or ( - client and client.options["instrumenter"] - ) + instrumenter = kwargs.get("instrumenter", None) + configuration_instrumenter = client and client.options.get("instrumenter", None) - if instrumenter == INSTRUMENTER.OTEL: - # logger.warn("start_child does NOTHING because instrumenter is OTEL") + if instrumenter != configuration_instrumenter: return NoOpSpan() kwargs.setdefault("sampled", self.sampled) From bc316e09a1112cefded5ebd0dd7ad796eb8ddd0b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 16:07:22 +0100 Subject: [PATCH 36/46] Fixed typing --- .../integrations/opentelemetry/propagator.py | 2 +- .../integrations/opentelemetry/span_processor.py | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 7b32cb56ed..fe4a91cf3e 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -1,4 +1,4 @@ -from opentelemetry import trace # type: ignore +from opentelemetry import trace from opentelemetry.context import ( # type: ignore Context, create_key, diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 3e5100f966..a00b726d7f 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -3,13 +3,13 @@ from opentelemetry.context import get_value # type: ignore from opentelemetry.sdk.trace import SpanProcessor # type: ignore from opentelemetry.semconv.trace import SpanAttributes # type: ignore -from opentelemetry.trace import ( +from opentelemetry.trace import ( # type: ignore format_span_id, format_trace_id, SpanContext, Span as OTelSpan, ) -from sentry_sdk.consts import INSTRUMENTER # type: ignore +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.hub import Hub from sentry_sdk.integrations.opentelemetry.propagator import ( @@ -48,7 +48,10 @@ def on_start(self, otel_span, parent_context=None): if not hub: return - if hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL: + if ( + hub.client + and hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL + ): return trace_data = self._get_trace_data(otel_span, parent_context) @@ -85,7 +88,10 @@ def on_end(self, otel_span): if not hub: return - if hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL: + if ( + hub.client + and hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL + ): return span_id = format_span_id(otel_span.context.span_id) From 4d85ecf6cff83ae93f6b044572adba49b610c680 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 16:10:20 +0100 Subject: [PATCH 37/46] Fixed typing again --- sentry_sdk/integrations/opentelemetry/propagator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index fe4a91cf3e..7b32cb56ed 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -1,4 +1,4 @@ -from opentelemetry import trace +from opentelemetry import trace # type: ignore from opentelemetry.context import ( # type: ignore Context, create_key, From 63b477e8d40a49bae8fa7394d9aabd7cea222f8a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 16:14:21 +0100 Subject: [PATCH 38/46] Fixed instrumenter default values. --- sentry_sdk/hub.py | 9 +++++---- sentry_sdk/tracing.py | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index 8261c107a6..f1b024961c 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -5,6 +5,7 @@ from contextlib import contextmanager from sentry_sdk._compat import with_metaclass +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.scope import Scope from sentry_sdk.client import Client from sentry_sdk.tracing import NoOpSpan, Span, Transaction @@ -464,9 +465,9 @@ def start_span( for every incoming HTTP request. Use `start_transaction` to start a new transaction when one is not already in progress. """ - instrumenter = kwargs.get("instrumenter", None) + instrumenter = kwargs.get("instrumenter", INSTRUMENTER.SENTRY) configuration_instrumenter = self.client and self.client.options.get( - "instrumenter", None + "instrumenter", INSTRUMENTER.SENTRY ) if instrumenter != configuration_instrumenter: @@ -527,9 +528,9 @@ def start_transaction( When the transaction is finished, it will be sent to Sentry with all its finished child spans. """ - instrumenter = kwargs.get("instrumenter", None) + instrumenter = kwargs.get("instrumenter", INSTRUMENTER.SENTRY) configuration_instrumenter = self.client and self.client.options.get( - "instrumenter", None + "instrumenter", INSTRUMENTER.SENTRY ) if instrumenter != configuration_instrumenter: diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 3dfdd2a1df..862c57e772 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -6,6 +6,7 @@ from datetime import datetime, timedelta import sentry_sdk +from sentry_sdk.consts import INSTRUMENTER from sentry_sdk.utils import logger from sentry_sdk._types import MYPY @@ -222,8 +223,10 @@ def start_child(self, **kwargs): hub = self.hub or sentry_sdk.Hub.current client = hub.client - instrumenter = kwargs.get("instrumenter", None) - configuration_instrumenter = client and client.options.get("instrumenter", None) + instrumenter = kwargs.get("instrumenter", INSTRUMENTER.SENTRY) + configuration_instrumenter = client and client.options.get( + "instrumenter", INSTRUMENTER.SENTRY + ) if instrumenter != configuration_instrumenter: return NoOpSpan() From 02392644e1804747e8badbd07ffbe364ab470f93 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 16:42:18 +0100 Subject: [PATCH 39/46] Fixed tests --- sentry_sdk/client.py | 4 ++++ sentry_sdk/hub.py | 8 ++------ .../integrations/opentelemetry/span_processor.py | 10 ++-------- sentry_sdk/tracing.py | 4 +--- .../opentelemetry/test_span_processor.py | 16 ++++++++++++++-- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index bf1e483634..8af7003156 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -20,6 +20,7 @@ from sentry_sdk.transport import make_transport from sentry_sdk.consts import ( DEFAULT_OPTIONS, + INSTRUMENTER, VERSION, ClientConstructor, ) @@ -86,6 +87,9 @@ def _get_options(*args, **kwargs): if rv["server_name"] is None and hasattr(socket, "gethostname"): rv["server_name"] = socket.gethostname() + if rv["instrumenter"] is None: + rv["instrumenter"] = INSTRUMENTER.SENTRY + return rv diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py index f1b024961c..b87c88f271 100644 --- a/sentry_sdk/hub.py +++ b/sentry_sdk/hub.py @@ -466,9 +466,7 @@ def start_span( transaction when one is not already in progress. """ instrumenter = kwargs.get("instrumenter", INSTRUMENTER.SENTRY) - configuration_instrumenter = self.client and self.client.options.get( - "instrumenter", INSTRUMENTER.SENTRY - ) + configuration_instrumenter = self.client and self.client.options["instrumenter"] if instrumenter != configuration_instrumenter: return NoOpSpan() @@ -529,9 +527,7 @@ def start_transaction( finished child spans. """ instrumenter = kwargs.get("instrumenter", INSTRUMENTER.SENTRY) - configuration_instrumenter = self.client and self.client.options.get( - "instrumenter", INSTRUMENTER.SENTRY - ) + configuration_instrumenter = self.client and self.client.options["instrumenter"] if instrumenter != configuration_instrumenter: return NoOpSpan() diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index a00b726d7f..b6cb32174d 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -48,10 +48,7 @@ def on_start(self, otel_span, parent_context=None): if not hub: return - if ( - hub.client - and hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL - ): + if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: return trace_data = self._get_trace_data(otel_span, parent_context) @@ -88,10 +85,7 @@ def on_end(self, otel_span): if not hub: return - if ( - hub.client - and hub.client.options.get("instrumenter", None) != INSTRUMENTER.OTEL - ): + if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: return span_id = format_span_id(otel_span.context.span_id) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 862c57e772..79a3acbb6b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -224,9 +224,7 @@ def start_child(self, **kwargs): client = hub.client instrumenter = kwargs.get("instrumenter", INSTRUMENTER.SENTRY) - configuration_instrumenter = client and client.options.get( - "instrumenter", INSTRUMENTER.SENTRY - ) + configuration_instrumenter = client and client.options["instrumenter"] if instrumenter != configuration_instrumenter: return NoOpSpan() diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index efb1b27679..4465fc776b 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -195,8 +195,14 @@ def test_on_start_transaction(): parent_context = {} + fake_client = MagicMock() + fake_client.options = {"instrumenter": "otel"} + + current_hub = MagicMock() + current_hub.client = fake_client + fake_hub = MagicMock() - fake_hub.current.return_value = MagicMock() + fake_hub.current = current_hub with mock.patch( "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub @@ -230,8 +236,14 @@ def test_on_start_child(): parent_context = {} + fake_client = MagicMock() + fake_client.options = {"instrumenter": "otel"} + + current_hub = MagicMock() + current_hub.client = fake_client + fake_hub = MagicMock() - fake_hub.current.return_value = MagicMock() + fake_hub.current = current_hub with mock.patch( "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub From 3ae5e042aea69942c061a96d31fc627ab0901549 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 28 Nov 2022 16:48:24 +0100 Subject: [PATCH 40/46] Fixed test --- sentry_sdk/integrations/opentelemetry/propagator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 7b32cb56ed..cbccbb720e 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -97,7 +97,7 @@ def inject(self, carrier, context=None, setter=default_setter): baggage = sentry_span.get_baggage() if baggage: - setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize()) + setter.set(carrier, BAGGAGE_HEADER_NAME, baggage) @property def fields(self): From 88f446bb06c6682aff7720e4acee4447dd9da32f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Nov 2022 11:24:51 +0100 Subject: [PATCH 41/46] Fixed infinite loop when otel catches sentry http requests. --- sentry_sdk/integrations/opentelemetry/propagator.py | 2 +- .../integrations/opentelemetry/span_processor.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index cbccbb720e..7b32cb56ed 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -97,7 +97,7 @@ def inject(self, carrier, context=None, setter=default_setter): baggage = sentry_span.get_baggage() if baggage: - setter.set(carrier, BAGGAGE_HEADER_NAME, baggage) + setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize()) @property def fields(self): diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index b6cb32174d..38153c3c0c 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -18,6 +18,7 @@ ) from sentry_sdk.tracing import Transaction, Span as SentrySpan from sentry_sdk._types import MYPY +from sentry_sdk.utils import Dsn if MYPY: from typing import Any @@ -64,7 +65,7 @@ def on_start(self, otel_span, parent_context=None): span_id=trace_data["span_id"], description=otel_span.name, start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), - instrumenter=INSTRUMENTER.SENTRY, + instrumenter=INSTRUMENTER.OTEL, ) else: sentry_span = hub.start_transaction( @@ -74,7 +75,7 @@ def on_start(self, otel_span, parent_context=None): trace_id=trace_data["trace_id"], baggage=trace_data["baggage"], start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), - instrumenter=INSTRUMENTER.SENTRY, + instrumenter=INSTRUMENTER.OTEL, ) self.otel_span_map[trace_data["span_id"]] = sentry_span @@ -88,6 +89,11 @@ def on_end(self, otel_span): if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: return + # Break infinite http requests to Sentry are caught by OTel and send again to Sentry. + otel_span_url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) + if otel_span_url and Dsn(hub.client.dsn).netloc in otel_span_url: + return + span_id = format_span_id(otel_span.context.span_id) sentry_span = self.otel_span_map.pop(span_id, None) if not sentry_span: From 1181a632a5eba20d286d36538cca2d38d8f95a27 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Nov 2022 13:53:10 +0100 Subject: [PATCH 42/46] Fixed HTTP request span description. --- .../integrations/opentelemetry/propagator.py | 2 +- .../opentelemetry/span_processor.py | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 7b32cb56ed..e4cde7cd3b 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -95,7 +95,7 @@ def inject(self, carrier, context=None, setter=default_setter): setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_traceparent()) - baggage = sentry_span.get_baggage() + baggage = hasattr(sentry_span, "get_baggage") and sentry_span.get_baggage() if baggage: setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize()) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 38153c3c0c..3c30c0f267 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -8,6 +8,7 @@ format_trace_id, SpanContext, Span as OTelSpan, + SpanKind, ) from sentry_sdk.consts import INSTRUMENTER @@ -20,6 +21,8 @@ from sentry_sdk._types import MYPY from sentry_sdk.utils import Dsn +from urllib3.util import parse_url as urlparse + if MYPY: from typing import Any from typing import Dict @@ -175,8 +178,15 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): db_query = otel_span.attributes.get(SpanAttributes.DB_SYSTEM, None) if http_method: - op = "http.{}".format(http_method.lower()) + op = "http" + + if otel_span.kind == SpanKind.SERVER: + op += ".server" + elif otel_span.kind == SpanKind.CLIENT: + op += ".client" + description = http_method + print(f"~~~~~otel_span.attributes: {otel_span.attributes}") peer_name = otel_span.attributes.get(SpanAttributes.NET_PEER_NAME, None) if peer_name: @@ -186,6 +196,13 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): if target: description += " {}".format(target) + if not peer_name and not target: + url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) + if url: + parsed_url = urlparse(url) + url = f"{parsed_url.scheme}://{parsed_url.netloc}{parsed_url.path}" + description += " {}".format(url) + status_code = otel_span.attributes.get( SpanAttributes.HTTP_STATUS_CODE, None ) From 3d369146599943589e3c7743c3638d2ee5078869 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Nov 2022 14:01:32 +0100 Subject: [PATCH 43/46] Fixed typing --- sentry_sdk/integrations/opentelemetry/span_processor.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 3c30c0f267..d62cbb4482 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -21,7 +21,7 @@ from sentry_sdk._types import MYPY from sentry_sdk.utils import Dsn -from urllib3.util import parse_url as urlparse +from urllib3.util import parse_url as urlparse # type: ignore if MYPY: from typing import Any @@ -94,7 +94,8 @@ def on_end(self, otel_span): # Break infinite http requests to Sentry are caught by OTel and send again to Sentry. otel_span_url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) - if otel_span_url and Dsn(hub.client.dsn).netloc in otel_span_url: + dsn_url = hub.client and Dsn(hub.client.dsn or "").netloc + if otel_span_url and dsn_url in otel_span_url: return span_id = format_span_id(otel_span.context.span_id) From d29d78e895e45e59c04b493c78832665d233eacb Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Nov 2022 14:06:35 +0100 Subject: [PATCH 44/46] Fixed tests --- tests/integrations/opentelemetry/test_span_processor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index 4465fc776b..cda296ef81 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -217,7 +217,7 @@ def test_on_start_transaction(): trace_id="1234567890abcdef1234567890abcdef", baggage=None, start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), - instrumenter="sentry", + instrumenter="otel", ) assert len(span_processor.otel_span_map.keys()) == 1 @@ -258,7 +258,7 @@ def test_on_start_child(): span_id="1234567890abcdef", description="Sample OTel Span", start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), - instrumenter="sentry", + instrumenter="otel", ) assert len(span_processor.otel_span_map.keys()) == 2 From b538f6c90ee5d092238d8569f10b70561ab34185 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Nov 2022 14:17:32 +0100 Subject: [PATCH 45/46] Improved tests --- .../opentelemetry/test_span_processor.py | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index cda296ef81..b932da63da 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -5,6 +5,8 @@ from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor from sentry_sdk.tracing import Span, Transaction +from opentelemetry.trace import SpanKind + def test_get_otel_context(): otel_span = MagicMock() @@ -136,6 +138,7 @@ def test_update_span_with_otel_data_http_method(): otel_span = MagicMock() otel_span.name = "Test OTel Span" + otel_span.kind = SpanKind.CLIENT otel_span.attributes = { "http.method": "GET", "http.status_code": 429, @@ -148,7 +151,7 @@ def test_update_span_with_otel_data_http_method(): span_processor = SentrySpanProcessor() span_processor._update_span_with_otel_data(sentry_span, otel_span) - assert sentry_span.op == "http.get" + assert sentry_span.op == "http.client" assert sentry_span.description == "GET example.com /" assert sentry_span._tags["http.status_code"] == "429" assert sentry_span.status == "resource_exhausted" @@ -161,6 +164,38 @@ def test_update_span_with_otel_data_http_method(): assert sentry_span._data["http.target"] == "/" +def test_update_span_with_otel_data_http_method2(): + sentry_span = Span() + + otel_span = MagicMock() + otel_span.name = "Test OTel Span" + otel_span.kind = SpanKind.SERVER + otel_span.attributes = { + "http.method": "GET", + "http.status_code": 429, + "http.status_text": "xxx", + "http.user_agent": "curl/7.64.1", + "http.url": "https://httpbin.org/status/403?password=123&username=test@example.com&author=User123&auth=1234567890abcdef", + } + + span_processor = SentrySpanProcessor() + span_processor._update_span_with_otel_data(sentry_span, otel_span) + + assert sentry_span.op == "http.server" + assert sentry_span.description == "GET https://httpbin.org/status/403" + assert sentry_span._tags["http.status_code"] == "429" + assert sentry_span.status == "resource_exhausted" + + assert sentry_span._data["http.method"] == "GET" + assert sentry_span._data["http.status_code"] == 429 + assert sentry_span._data["http.status_text"] == "xxx" + assert sentry_span._data["http.user_agent"] == "curl/7.64.1" + assert ( + sentry_span._data["http.url"] + == "https://httpbin.org/status/403?password=123&username=test@example.com&author=User123&auth=1234567890abcdef" + ) + + def test_update_span_with_otel_data_db_query(): sentry_span = Span() From 08d0311cd9dbd5af1af84bce38a81cc02f68535e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 29 Nov 2022 14:40:12 +0100 Subject: [PATCH 46/46] Fixed tests --- .../opentelemetry/test_propagator.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 5cf50021a5..f839d2d486 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -15,6 +15,7 @@ SentryPropagator, ) from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor +from sentry_sdk.tracing_utils import Baggage def test_extract_no_context_no_sentry_trace_header(): @@ -214,12 +215,14 @@ def test_inject_sentry_span_baggage(): sentry_span.to_traceparent = mock.Mock( return_value="1234567890abcdef1234567890abcdef-1234567890abcdef-1" ) - baggage = ( - "sentry-trace_id=771a43a4192642f0b136d5159a501700," - "sentry-public_key=49d0f7386ad645858ae85020e393bef3," - "sentry-sample_rate=0.01337,sentry-user_id=Am%C3%A9lie" - ) - sentry_span.get_baggage = mock.Mock(return_value=baggage) + sentry_items = { + "sentry-trace_id": "771a43a4192642f0b136d5159a501700", + "sentry-public_key": "49d0f7386ad645858ae85020e393bef3", + "sentry-sample_rate": 0.01337, + "sentry-user_id": "Amélie", + } + baggage = Baggage(sentry_items=sentry_items) + sentry_span.get_baggage = MagicMock(return_value=baggage) span_processor = SentrySpanProcessor() span_processor.otel_span_map[span_id] = sentry_span @@ -240,5 +243,5 @@ def test_inject_sentry_span_baggage(): setter.set.assert_any_call( carrier, "baggage", - baggage, + baggage.serialize(), )