From d3164cfe66cc952390a9176823371b17a581550d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 1 Dec 2022 12:23:52 +0100 Subject: [PATCH 01/18] OTel support (added SpanProcessor and Propagator) --- .../test-integration-opentelemetry.yml | 62 +++ .../integrations/opentelemetry/__init__.py | 3 + .../integrations/opentelemetry/propagator.py | 113 ++++++ .../opentelemetry/span_processor.py | 220 ++++++++++ tests/integrations/opentelemetry/__init__.py | 3 + .../opentelemetry/test_propagator.py | 247 ++++++++++++ .../opentelemetry/test_span_processor.py | 376 ++++++++++++++++++ tox.ini | 4 + 8 files changed, 1028 insertions(+) create mode 100644 .github/workflows/test-integration-opentelemetry.yml create mode 100644 sentry_sdk/integrations/opentelemetry/__init__.py create mode 100644 sentry_sdk/integrations/opentelemetry/propagator.py create mode 100644 sentry_sdk/integrations/opentelemetry/span_processor.py 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 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 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/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py new file mode 100644 index 0000000000..141e017a93 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -0,0 +1,113 @@ +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, + TextMapPropagator, + default_getter, + default_setter, +) +from opentelemetry.trace import ( # type: ignore + TraceFlags, + NonRecordingSpan, + SpanContext, +) + +from sentry_sdk.tracing import ( + BAGGAGE_HEADER_NAME, + SENTRY_TRACE_HEADER_NAME, +) +from sentry_sdk.tracing_utils import Baggage, extract_sentrytrace_data +from sentry_sdk._types import MYPY + +if MYPY: + from typing import Optional + from typing import Set + + +SENTRY_TRACE_KEY = create_key("sentry-trace") +SENTRY_BAGGAGE_KEY = create_key("sentry-baggage") + + +class SentryPropagator(TextMapPropagator): # type: ignore + def extract(self, carrier, context=None, getter=default_getter): + # type: (CarrierT, Optional[Context], Getter) -> Context + if context is None: + context = get_current() + + sentry_trace = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) + if not sentry_trace: + return context + + sentrytrace = extract_sentrytrace_data(sentry_trace[0]) + if not sentrytrace: + return context + + sentry_trace_data = ( + sentrytrace.get("trace_id", "0"), + sentrytrace.get("parent_span_id", "0"), + sentrytrace.get("parent_sampled", None), + ) + + context = set_value(SENTRY_TRACE_KEY, sentry_trace_data, context) + + trace_id, span_id, _ = sentry_trace_data + + span_context = SpanContext( + trace_id=int(trace_id, 16), # type: ignore + span_id=int(span_id, 16), # type: ignore + # we simulate a sampled trace on the otel side and leave the sampling to sentry + 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[0]) + 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(sentry_items={}) + + baggage.freeze() + context = set_value(SENTRY_BAGGAGE_KEY, baggage, context) + + span = NonRecordingSpan(span_context) + modified_context = trace.set_span_in_context(span, context) + return modified_context + + def inject(self, carrier, context=None, setter=default_setter): + # type: (CarrierT, Optional[Context], 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) + + 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: + return + + setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_traceparent()) + + baggage = hasattr(sentry_span, "get_baggage") and sentry_span.get_baggage() + if baggage: + setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize()) + + @property + def fields(self): + # type: () -> Set[str] + 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 new file mode 100644 index 0000000000..d62cbb4482 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -0,0 +1,220 @@ +from datetime import datetime + +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 ( # type: ignore + format_span_id, + format_trace_id, + SpanContext, + Span as OTelSpan, + SpanKind, +) +from sentry_sdk.consts import INSTRUMENTER + +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, Span as SentrySpan +from sentry_sdk._types import MYPY +from sentry_sdk.utils import Dsn + +from urllib3.util import parse_url as urlparse # type: ignore + +if MYPY: + from typing import Any + from typing import Dict + from typing import Union + +OPEN_TELEMETRY_CONTEXT = "otel" + + +class SentrySpanProcessor(SpanProcessor): # type: ignore + """ + Converts OTel spans into Sentry spans so they can be sent to the Sentry backend. + """ + + # 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 + + if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: + return + + trace_data = self._get_trace_data(otel_span, parent_context) + + 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 + ) + + sentry_span = None + if sentry_parent_span: + sentry_span = sentry_parent_span.start_child( + span_id=trace_data["span_id"], + description=otel_span.name, + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), + instrumenter=INSTRUMENTER.OTEL, + ) + else: + sentry_span = hub.start_transaction( + name=otel_span.name, + span_id=trace_data["span_id"], + parent_span_id=parent_span_id, + trace_id=trace_data["trace_id"], + baggage=trace_data["baggage"], + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), + instrumenter=INSTRUMENTER.OTEL, + ) + + 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 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) + 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) + sentry_span = self.otel_span_map.pop(span_id, None) + if not sentry_span: + return + + sentry_span.op = otel_span.name + + if isinstance(sentry_span, Transaction): + 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) + + sentry_span.finish( + end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) + ) + + 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 + """ + 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 _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. + """ + 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): + # 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. + """ + for key in otel_span.attributes: + val = otel_span.attributes[key] + sentry_span.set_data(key, val) + + op = otel_span.name + description = otel_span.name + + http_method = otel_span.attributes.get(SpanAttributes.HTTP_METHOD, None) + db_query = otel_span.attributes.get(SpanAttributes.DB_SYSTEM, None) + + if http_method: + 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: + description += " {}".format(peer_name) + + target = otel_span.attributes.get(SpanAttributes.HTTP_TARGET, None) + if target: + description += " {}".format(target) + + if not peer_name and not target: + url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) + 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 + ) + 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/tests/integrations/opentelemetry/__init__.py b/tests/integrations/opentelemetry/__init__.py new file mode 100644 index 0000000000..39ecc610d5 --- /dev/null +++ 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 new file mode 100644 index 0000000000..f839d2d486 --- /dev/null +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -0,0 +1,247 @@ +from mock import MagicMock +import mock + +from opentelemetry.context import get_current +from opentelemetry.trace.propagation import get_current_span +from opentelemetry.trace import ( + set_span_in_context, + TraceFlags, + SpanContext, +) + +from sentry_sdk.integrations.opentelemetry.propagator import ( + SENTRY_BAGGAGE_KEY, + SENTRY_TRACE_KEY, + 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(): + """ + 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 + + modified_context = SentryPropagator().extract(carrier, context, getter) + + assert modified_context == {} + + +def test_extract_context_no_sentry_trace_header(): + """ + 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 + + modified_context = SentryPropagator().extract(carrier, context, getter) + + assert modified_context == context + + +def test_extract_empty_context_sentry_trace_header_no_baggage(): + """ + 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.side_effect = [ + ["1234567890abcdef1234567890abcdef-1234567890abcdef-1"], + None, + ] + + 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(): + """ + 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(): + """ + 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(): + """ + 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(): + """ + 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" + ) + 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 + + 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.serialize(), + ) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py new file mode 100644 index 0000000000..b932da63da --- /dev/null +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -0,0 +1,376 @@ +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, Transaction + +from opentelemetry.trace import SpanKind + + +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.kind = SpanKind.CLIENT + 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.client" + 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_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() + + 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 = {} + + fake_client = MagicMock() + fake_client.options = {"instrumenter": "otel"} + + current_hub = MagicMock() + current_hub.client = fake_client + + fake_hub = MagicMock() + fake_hub.current = current_hub + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub + ): + span_processor = SentrySpanProcessor() + span_processor.on_start(otel_span, parent_context) + + fake_hub.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="otel", + ) + + 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 = {} + + fake_client = MagicMock() + fake_client.options = {"instrumenter": "otel"} + + current_hub = MagicMock() + current_hub.client = fake_client + + fake_hub = MagicMock() + fake_hub.current = current_hub + + with mock.patch( + "sentry_sdk.integrations.opentelemetry.span_processor.Hub", fake_hub + ): + fake_span = MagicMock() + + span_processor = SentrySpanProcessor() + span_processor.otel_span_map["abcdef1234567890"] = fake_span + span_processor.on_start(otel_span, parent_context) + + fake_span.start_child.assert_called_once_with( + span_id="1234567890abcdef", + description="Sample OTel Span", + start_timestamp=datetime.fromtimestamp(otel_span.start_time / 1e9), + instrumenter="otel", + ) + + 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_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() + + +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() 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 98d351f56a886003e0020678128215eb787566a1 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 1 Dec 2022 13:21:50 +0100 Subject: [PATCH 02/18] Added tests for NoOpSpan --- sentry_sdk/tracing.py | 2 +- tests/tracing/test_noop_span.py | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/tracing/test_noop_span.py diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 93d22dc758..fb9efd03fb 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -869,7 +869,7 @@ def __exit__(self, ty, value, tb): def start_child(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs): # type: (str, **Any) -> Any - pass + return NoOpSpan() def new_span(self, **kwargs): # type: (**Any) -> Any diff --git a/tests/tracing/test_noop_span.py b/tests/tracing/test_noop_span.py new file mode 100644 index 0000000000..3dc148f848 --- /dev/null +++ b/tests/tracing/test_noop_span.py @@ -0,0 +1,46 @@ +import sentry_sdk +from sentry_sdk.tracing import NoOpSpan + +# This tests make sure, that the examples from the documentation [1] +# are working when OTel (OpenTelementry) instrumentation is turned on +# and therefore the Senntry tracing should not do anything. +# +# 1: https://docs.sentry.io/platforms/python/performance/instrumentation/custom-instrumentation/ + + +def test_noop_start_transaction(sentry_init): + sentry_init(instrumenter="otel", debug=True) + + transaction = sentry_sdk.start_transaction(op="task", name="test_transaction_name") + assert isinstance(transaction, NoOpSpan) + + transaction.name = "new name" + + +def test_noop_start_span(sentry_init): + sentry_init(instrumenter="otel", debug=True) + + with sentry_sdk.start_span(op="http", description="GET /") as span: + assert isinstance(span, NoOpSpan) + + span.set_tag("http.status_code", "418") + span.set_data("http.entity_type", "teapot") + + +def test_noop_transaction_start_child(sentry_init): + sentry_init(instrumenter="otel", debug=True) + + transaction = sentry_sdk.start_transaction(name="task") + assert isinstance(transaction, NoOpSpan) + + with transaction.start_child(op="child_task") as child: + assert isinstance(child, NoOpSpan) + + +def test_noop_span_start_child(sentry_init): + sentry_init(instrumenter="otel", debug=True) + span = sentry_sdk.start_span(name="task") + assert isinstance(span, NoOpSpan) + + with span.start_child(op="child_task") as child: + assert isinstance(child, NoOpSpan) From 858069224d622662aaf36b36f6a76c4e2463c00a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 1 Dec 2022 13:33:46 +0100 Subject: [PATCH 03/18] Some polishing --- sentry_sdk/integrations/opentelemetry/__init__.py | 1 + sentry_sdk/integrations/opentelemetry/propagator.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/sentry_sdk/integrations/opentelemetry/__init__.py b/sentry_sdk/integrations/opentelemetry/__init__.py index 2ec31af8c9..e4c272a00a 100644 --- a/sentry_sdk/integrations/opentelemetry/__init__.py +++ b/sentry_sdk/integrations/opentelemetry/__init__.py @@ -1,3 +1,4 @@ from sentry_sdk.integrations.opentelemetry.span_processor import ( # noqa: F401 + SentryPropagator, SentrySpanProcessor, ) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 141e017a93..b4054376aa 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -36,6 +36,10 @@ class SentryPropagator(TextMapPropagator): # type: ignore + """ + Propagates tracing headers for Sentry's tracing system in a way OTel understands. + """ + def extract(self, carrier, context=None, getter=default_getter): # type: (CarrierT, Optional[Context], Getter) -> Context if context is None: From 2fe376ef208dfea41c842edf21d8c2eb785cf592 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 1 Dec 2022 13:39:06 +0100 Subject: [PATCH 04/18] Fixed imports --- sentry_sdk/integrations/opentelemetry/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/opentelemetry/__init__.py b/sentry_sdk/integrations/opentelemetry/__init__.py index e4c272a00a..30aa78ef43 100644 --- a/sentry_sdk/integrations/opentelemetry/__init__.py +++ b/sentry_sdk/integrations/opentelemetry/__init__.py @@ -1,4 +1,6 @@ from sentry_sdk.integrations.opentelemetry.span_processor import ( # noqa: F401 - SentryPropagator, SentrySpanProcessor, ) +from sentry_sdk.integrations.opentelemetry.propagator import ( # noqa: F401 + SentryPropagator, +) From 0ba528bee2ddd6741bd8f384afb1beffb0683b4c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 1 Dec 2022 13:47:08 +0100 Subject: [PATCH 05/18] Cleanup --- sentry_sdk/integrations/opentelemetry/span_processor.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index d62cbb4482..615369487d 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -187,7 +187,6 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): 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: From 197dc96ec2f6f5078e50722898623660e21d9920 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 14:39:18 +0100 Subject: [PATCH 06/18] Added opentelemetry to the setup.py extras_require --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 687111566b..318c9dc837 100644 --- a/setup.py +++ b/setup.py @@ -63,6 +63,7 @@ def get_file_text(file_name): "starlette": ["starlette>=0.19.1"], "fastapi": ["fastapi>=0.79.0"], "pymongo": ["pymongo>=3.1"], + "opentelemetry": ["opentelemetry-distro>=0.350b0"], }, classifiers=[ "Development Status :: 5 - Production/Stable", From 61601b58de1f997ababd2a01dd8ba89106b7933b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 14:53:09 +0100 Subject: [PATCH 07/18] Added proper types to NoOpSpan --- sentry_sdk/tracing.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index fb9efd03fb..dc65ea5fd7 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -856,43 +856,43 @@ def _set_initial_sampling_decision(self, sampling_context): class NoOpSpan(Span): def __repr__(self): - # type: () -> Any + # type: () -> str return self.__class__.__name__ def __enter__(self): - # type: () -> Any + # type: () -> NoOpSpan return self def __exit__(self, ty, value, tb): - # type: (Any, Any, Any) -> Any + # type: (Optional[Any], Optional[Any], Optional[Any]) -> None pass def start_child(self, instrumenter=INSTRUMENTER.SENTRY, **kwargs): - # type: (str, **Any) -> Any + # type: (str, **Any) -> NoOpSpan return NoOpSpan() def new_span(self, **kwargs): - # type: (**Any) -> Any + # type: (**Any) -> NoOpSpan pass def set_tag(self, key, value): - # type: (Any, Any) -> Any + # type: (str, Any) -> None pass def set_data(self, key, value): - # type: (Any, Any) -> Any + # type: (str, Any) -> None pass def set_status(self, value): - # type: (Any) -> Any + # type: (str) -> None pass def set_http_status(self, http_status): - # type: (Any) -> Any + # type: (int) -> None pass def finish(self, hub=None, end_timestamp=None): - # type: (Any, Any) -> Any + # type: (Optional[sentry_sdk.Hub], Optional[datetime]) -> Optional[str] pass From 9e3d9d64a9c28cecc70acbd8582d82f5a23e2100 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:08:53 +0100 Subject: [PATCH 08/18] Refactored infinite loop breaking code --- .../opentelemetry/span_processor.py | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 615369487d..5cfdf9296e 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -55,6 +55,9 @@ def on_start(self, otel_span, parent_context=None): if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: return + if self._is_sentry_span(hub, otel_span): + return + trace_data = self._get_trace_data(otel_span, parent_context) parent_span_id = trace_data["parent_span_id"] @@ -92,12 +95,6 @@ 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) - 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) sentry_span = self.otel_span_map.pop(span_id, None) if not sentry_span: @@ -118,6 +115,20 @@ def on_end(self, otel_span): end_timestamp=datetime.fromtimestamp(otel_span.end_time / 1e9) ) + def _is_sentry_span(self, hub, otel_span): + # type: (Hub, OTelSpan) -> bool + """ + Break infinite loop: + HTTP requests to Sentry are caught by OTel and send again to Sentry. + """ + otel_span_url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None) + dsn_url = hub.client and Dsn(hub.client.dsn or "").netloc + + if otel_span_url and dsn_url in otel_span_url: + return True + + return False + def _get_otel_context(self, otel_span): # type: (OTelSpan) -> Dict[str, Any] """ From 5f2b105bfec81eccae719c52fc921a428c1dd743 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:13:22 +0100 Subject: [PATCH 09/18] Made for loop simpler --- sentry_sdk/integrations/opentelemetry/span_processor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 5cfdf9296e..a71fc0551d 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -179,8 +179,7 @@ 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] + for key, val in otel_span.attributes.items(): sentry_span.set_data(key, val) op = otel_span.name From c3c3f46693abb1139a33bee10f7aa9974cd94809 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:33:36 +0100 Subject: [PATCH 10/18] Fixed tests --- .../opentelemetry/test_span_processor.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/integrations/opentelemetry/test_span_processor.py b/tests/integrations/opentelemetry/test_span_processor.py index b932da63da..6d151c9cfe 100644 --- a/tests/integrations/opentelemetry/test_span_processor.py +++ b/tests/integrations/opentelemetry/test_span_processor.py @@ -8,6 +8,33 @@ from opentelemetry.trace import SpanKind +def test_is_sentry_span(): + otel_span = MagicMock() + + hub = MagicMock() + hub.client = None + + span_processor = SentrySpanProcessor() + assert not span_processor._is_sentry_span(hub, otel_span) + + client = MagicMock() + client.options = {"instrumenter": "otel"} + client.dsn = "https://1234567890abcdef@o123456.ingest.sentry.io/123456" + + hub.client = client + assert not span_processor._is_sentry_span(hub, otel_span) + + otel_span.attributes = { + "http.url": "https://example.com", + } + assert not span_processor._is_sentry_span(hub, otel_span) + + otel_span.attributes = { + "http.url": "https://o123456.ingest.sentry.io/api/123/envelope", + } + assert span_processor._is_sentry_span(hub, otel_span) + + def test_get_otel_context(): otel_span = MagicMock() otel_span.attributes = {"foo": "bar"} @@ -232,6 +259,7 @@ def test_on_start_transaction(): fake_client = MagicMock() fake_client.options = {"instrumenter": "otel"} + fake_client.dsn = "https://1234567890abcdef@o123456.ingest.sentry.io/123456" current_hub = MagicMock() current_hub.client = fake_client @@ -273,6 +301,7 @@ def test_on_start_child(): fake_client = MagicMock() fake_client.options = {"instrumenter": "otel"} + fake_client.dsn = "https://1234567890abcdef@o123456.ingest.sentry.io/123456" current_hub = MagicMock() current_hub.client = fake_client From f93886b5897e7ea15d4d51c0d40b1ca0dee5f80f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:33:53 +0100 Subject: [PATCH 11/18] Added otel span kind --- sentry_sdk/integrations/opentelemetry/span_processor.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index a71fc0551d..2c5c4f7229 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -182,6 +182,8 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): for key, val in otel_span.attributes.items(): sentry_span.set_data(key, val) + sentry_span.set_data("otel.kind", otel_span.kind) + op = otel_span.name description = otel_span.name From fb1afe31200ad5327354bf25f1f72220478a824c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:41:57 +0100 Subject: [PATCH 12/18] Simplified code. --- .../integrations/opentelemetry/propagator.py | 10 ++------- .../opentelemetry/test_propagator.py | 21 ++++++++++--------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index b4054376aa..980f37fee3 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -53,15 +53,9 @@ def extract(self, carrier, context=None, getter=default_getter): if not sentrytrace: return context - sentry_trace_data = ( - sentrytrace.get("trace_id", "0"), - sentrytrace.get("parent_span_id", "0"), - sentrytrace.get("parent_sampled", None), - ) - - context = set_value(SENTRY_TRACE_KEY, sentry_trace_data, context) + context = set_value(SENTRY_TRACE_KEY, sentrytrace, context) - trace_id, span_id, _ = sentry_trace_data + trace_id, span_id = sentrytrace["trace_id"], sentrytrace["parent_span_id"] span_context = SpanContext( trace_id=int(trace_id, 16), # type: ignore diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index f839d2d486..5ea338facf 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -65,11 +65,11 @@ def test_extract_empty_context_sentry_trace_header_no_baggage(): assert len(modified_context.keys()) == 3 - assert modified_context[SENTRY_TRACE_KEY] == ( - "1234567890abcdef1234567890abcdef", - "1234567890abcdef", - True, - ) + assert modified_context[SENTRY_TRACE_KEY] == { + "trace_id": "1234567890abcdef1234567890abcdef", + "parent_span_id": "1234567890abcdef", + "parent_sampled": True, + } assert modified_context[SENTRY_BAGGAGE_KEY].serialize() == "" span_context = get_current_span(modified_context).get_span_context() @@ -100,11 +100,12 @@ def test_extract_context_sentry_trace_header_baggage(): assert len(modified_context.keys()) == 4 - assert modified_context[SENTRY_TRACE_KEY] == ( - "1234567890abcdef1234567890abcdef", - "1234567890abcdef", - True, - ) + assert modified_context[SENTRY_TRACE_KEY] == { + "trace_id": "1234567890abcdef1234567890abcdef", + "parent_span_id": "1234567890abcdef", + "parent_sampled": True, + } + assert modified_context[SENTRY_BAGGAGE_KEY].serialize() == ( "sentry-trace_id=771a43a4192642f0b136d5159a501700," "sentry-public_key=49d0f7386ad645858ae85020e393bef3," From 3fe5ec83c0a7c205d7a8b12b30a7906e33e4984a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:46:49 +0100 Subject: [PATCH 13/18] Added checks for valid context --- sentry_sdk/integrations/opentelemetry/span_processor.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index 2c5c4f7229..022c56d7c5 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -55,6 +55,9 @@ def on_start(self, otel_span, parent_context=None): if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: return + if not otel_span.context.is_valid: + return + if self._is_sentry_span(hub, otel_span): return @@ -95,6 +98,9 @@ def on_end(self, otel_span): if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL: return + if not otel_span.context.is_valid: + 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 dbb22e4b634c2506d719c3ceaf71c188b872cb1a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 15:59:17 +0100 Subject: [PATCH 14/18] Fixed circular imports --- .../integrations/opentelemetry/__init__.py | 1 + sentry_sdk/integrations/opentelemetry/consts.py | 6 ++++++ .../integrations/opentelemetry/propagator.py | 16 +++++++--------- .../integrations/opentelemetry/span_processor.py | 5 ++--- .../opentelemetry/test_propagator.py | 6 +++--- 5 files changed, 19 insertions(+), 15 deletions(-) create mode 100644 sentry_sdk/integrations/opentelemetry/consts.py diff --git a/sentry_sdk/integrations/opentelemetry/__init__.py b/sentry_sdk/integrations/opentelemetry/__init__.py index 30aa78ef43..e0020204d5 100644 --- a/sentry_sdk/integrations/opentelemetry/__init__.py +++ b/sentry_sdk/integrations/opentelemetry/__init__.py @@ -1,6 +1,7 @@ from sentry_sdk.integrations.opentelemetry.span_processor import ( # noqa: F401 SentrySpanProcessor, ) + from sentry_sdk.integrations.opentelemetry.propagator import ( # noqa: F401 SentryPropagator, ) diff --git a/sentry_sdk/integrations/opentelemetry/consts.py b/sentry_sdk/integrations/opentelemetry/consts.py new file mode 100644 index 0000000000..79663dd670 --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/consts.py @@ -0,0 +1,6 @@ +from opentelemetry.context import ( # type: ignore + create_key, +) + +SENTRY_TRACE_KEY = create_key("sentry-trace") +SENTRY_BAGGAGE_KEY = create_key("sentry-baggage") diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 980f37fee3..7c4839bc12 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -1,7 +1,6 @@ from opentelemetry import trace # type: ignore from opentelemetry.context import ( # type: ignore Context, - create_key, get_current, set_value, ) @@ -18,6 +17,13 @@ NonRecordingSpan, SpanContext, ) +from sentry_sdk.integrations.opentelemetry.consts import ( + SENTRY_BAGGAGE_KEY, + SENTRY_TRACE_KEY, +) +from sentry_sdk.integrations.opentelemetry.span_processor import ( + SentrySpanProcessor, +) from sentry_sdk.tracing import ( BAGGAGE_HEADER_NAME, @@ -31,10 +37,6 @@ from typing import Set -SENTRY_TRACE_KEY = create_key("sentry-trace") -SENTRY_BAGGAGE_KEY = create_key("sentry-baggage") - - class SentryPropagator(TextMapPropagator): # type: ignore """ Propagates tracing headers for Sentry's tracing system in a way OTel understands. @@ -90,10 +92,6 @@ def inject(self, carrier, context=None, setter=default_setter): 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 022c56d7c5..0ec9c620af 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -11,15 +11,14 @@ SpanKind, ) from sentry_sdk.consts import INSTRUMENTER - from sentry_sdk.hub import Hub -from sentry_sdk.integrations.opentelemetry.propagator import ( +from sentry_sdk.integrations.opentelemetry.consts import ( SENTRY_BAGGAGE_KEY, SENTRY_TRACE_KEY, ) from sentry_sdk.tracing import Transaction, Span as SentrySpan -from sentry_sdk._types import MYPY from sentry_sdk.utils import Dsn +from sentry_sdk._types import MYPY from urllib3.util import parse_url as urlparse # type: ignore diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 5ea338facf..5c5c9216e9 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -8,12 +8,12 @@ TraceFlags, SpanContext, ) - -from sentry_sdk.integrations.opentelemetry.propagator import ( +from sentry_sdk.integrations.opentelemetry.consts import ( SENTRY_BAGGAGE_KEY, SENTRY_TRACE_KEY, - SentryPropagator, ) + +from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor from sentry_sdk.tracing_utils import Baggage From b09d094302003593cc5961ffb1980d80c863da96 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 16:00:45 +0100 Subject: [PATCH 15/18] Checking if context is valid. --- sentry_sdk/integrations/opentelemetry/propagator.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 7c4839bc12..1b091dfa90 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -90,6 +90,10 @@ def inject(self, carrier, context=None, setter=default_setter): context = get_current() current_span = trace.get_current_span(context) + + if not current_span.context.is_valid: + return + span_id = trace.format_span_id(current_span.context.span_id) span_map = SentrySpanProcessor().otel_span_map From 031e79add43236e561589fb75deafb6a207a0600 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 6 Dec 2022 16:09:39 +0100 Subject: [PATCH 16/18] Made code easier to follow --- sentry_sdk/integrations/opentelemetry/propagator.py | 2 +- tests/integrations/opentelemetry/test_propagator.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/opentelemetry/propagator.py b/sentry_sdk/integrations/opentelemetry/propagator.py index 1b091dfa90..7b2a88e347 100644 --- a/sentry_sdk/integrations/opentelemetry/propagator.py +++ b/sentry_sdk/integrations/opentelemetry/propagator.py @@ -103,7 +103,7 @@ def inject(self, carrier, context=None, setter=default_setter): setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_traceparent()) - baggage = hasattr(sentry_span, "get_baggage") and sentry_span.get_baggage() + baggage = sentry_span.containing_transaction.get_baggage() if baggage: setter.set(carrier, BAGGAGE_HEADER_NAME, baggage.serialize()) diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 5c5c9216e9..529aa99c09 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -172,7 +172,7 @@ def test_inject_sentry_span_no_baggage(): sentry_span.to_traceparent = mock.Mock( return_value="1234567890abcdef1234567890abcdef-1234567890abcdef-1" ) - sentry_span.get_baggage = mock.Mock(return_value=None) + sentry_span.containing_transaction.get_baggage = mock.Mock(return_value=None) span_processor = SentrySpanProcessor() span_processor.otel_span_map[span_id] = sentry_span @@ -223,7 +223,7 @@ def test_inject_sentry_span_baggage(): "sentry-user_id": "Amélie", } baggage = Baggage(sentry_items=sentry_items) - sentry_span.get_baggage = MagicMock(return_value=baggage) + sentry_span.containing_transaction.get_baggage = MagicMock(return_value=baggage) span_processor = SentrySpanProcessor() span_processor.otel_span_map[span_id] = sentry_span From 301a5c4d049dc61e7f3b7df9f61b064defb6bc99 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 14 Dec 2022 15:11:56 +0100 Subject: [PATCH 17/18] Better readability --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 6e03f9fcc4..d2e87cb1f7 100644 --- a/tox.ini +++ b/tox.ini @@ -128,6 +128,7 @@ envlist = # Boto3 {py2.7,py3.6,py3.7,py3.8}-boto3-v{1.9,1.10,1.11,1.12,1.13,1.14,1.15,1.16} + # OpenTelemetry (OTel) {py3.7,py3.8,py3.9,py3.10}-opentelemetry [testenv] From 69dc721696c19301aa3ead63b9cb14b74d8fb40d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 14 Dec 2022 15:12:25 +0100 Subject: [PATCH 18/18] Updated ci config --- .../test-integration-opentelemetry.yml | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test-integration-opentelemetry.yml b/.github/workflows/test-integration-opentelemetry.yml index 987d92b95c..73a16098e4 100644 --- a/.github/workflows/test-integration-opentelemetry.yml +++ b/.github/workflows/test-integration-opentelemetry.yml @@ -27,12 +27,16 @@ jobs: name: opentelemetry, python ${{ matrix.python-version }}, ${{ matrix.os }} runs-on: ${{ matrix.os }} timeout-minutes: 45 - continue-on-error: true strategy: + fail-fast: false matrix: python-version: ["3.7","3.8","3.9","3.10"] - os: [ubuntu-latest] + # python3.6 reached EOL and is no longer being supported on + # new versions of hosted runners on Github Actions + # ubuntu-20.04 is the last version that supported python3.6 + # see https://github.com/actions/setup-python/issues/544#issuecomment-1332535877 + os: [ubuntu-20.04] steps: - uses: actions/checkout@v3 @@ -41,15 +45,10 @@ jobs: python-version: ${{ matrix.python-version }} - name: Setup Test Env - env: - PGHOST: localhost - PGPASSWORD: sentry run: | - pip install codecov tox + pip install codecov "tox>=3,<4" - name: Test opentelemetry - env: - CI_PYTHON_VERSION: ${{ matrix.python-version }} timeout-minutes: 45 shell: bash run: | @@ -60,3 +59,15 @@ jobs: coverage combine .coverage* coverage xml -i codecov --file coverage.xml + + check_required_tests: + name: All opentelemetry tests passed or skipped + needs: test + # Always run this, even if a dependent job failed + if: always() + runs-on: ubuntu-20.04 + steps: + - name: Check for failures + if: contains(needs.test.result, 'failure') + run: | + echo "One of the dependent jobs have failed. You may need to re-run it." && exit 1