-
Notifications
You must be signed in to change notification settings - Fork 568
OTLPIntegration #4877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
OTLPIntegration #4877
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,7 @@ | |
| "gcp", | ||
| "gevent", | ||
| "opentelemetry", | ||
| "otlp", | ||
| "potel", | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,6 +150,7 @@ | |
| "Misc": [ | ||
| "loguru", | ||
| "opentelemetry", | ||
| "otlp", | ||
| "potel", | ||
| "pure_eval", | ||
| "trytond", | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||||||||||
| from sentry_sdk.integrations import Integration, DidNotEnable | ||||||||||||||
| from sentry_sdk.scope import register_external_propagation_context | ||||||||||||||
| from sentry_sdk.utils import logger, Dsn | ||||||||||||||
| from sentry_sdk.consts import VERSION, EndpointType | ||||||||||||||
|
|
||||||||||||||
| try: | ||||||||||||||
| from opentelemetry import trace | ||||||||||||||
| from opentelemetry.propagate import set_global_textmap | ||||||||||||||
| from opentelemetry.sdk.trace import TracerProvider | ||||||||||||||
| from opentelemetry.sdk.trace.export import BatchSpanProcessor | ||||||||||||||
| from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter | ||||||||||||||
|
|
||||||||||||||
| from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator | ||||||||||||||
| except ImportError: | ||||||||||||||
| raise DidNotEnable("opentelemetry-distro[otlp] is not installed") | ||||||||||||||
|
|
||||||||||||||
| from typing import TYPE_CHECKING | ||||||||||||||
|
|
||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||
| from typing import Optional, Dict, Any, Tuple | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def otel_propagation_context(): | ||||||||||||||
| # type: () -> Optional[Tuple[str, str]] | ||||||||||||||
| """ | ||||||||||||||
| Get the (trace_id, span_id) from opentelemetry if exists. | ||||||||||||||
| """ | ||||||||||||||
| ctx = trace.get_current_span().get_span_context() | ||||||||||||||
|
|
||||||||||||||
| if ctx.trace_id == trace.INVALID_TRACE_ID or ctx.span_id == trace.INVALID_SPAN_ID: | ||||||||||||||
| return None | ||||||||||||||
|
|
||||||||||||||
| return (trace.format_trace_id(ctx.trace_id), trace.format_span_id(ctx.span_id)) | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| def setup_otlp_exporter(dsn=None): | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Not sure if I am missing something because I am not as familiar with OTel. I would have thought if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. they can still manually set env variables as follows: which will be picked up by otel internally when initializing the exporter. Leaving it open as an option just in case someone has a complicated ops setup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we'll iterate on the DX here once we get some feedback/complaints/confusion |
||||||||||||||
| # type: (Optional[str]) -> None | ||||||||||||||
| tracer_provider = trace.get_tracer_provider() | ||||||||||||||
|
|
||||||||||||||
| if not isinstance(tracer_provider, TracerProvider): | ||||||||||||||
| logger.debug("[OTLP] No TracerProvider configured by user, creating a new one") | ||||||||||||||
| tracer_provider = TracerProvider() | ||||||||||||||
| trace.set_tracer_provider(tracer_provider) | ||||||||||||||
|
|
||||||||||||||
| endpoint = None | ||||||||||||||
| headers = None | ||||||||||||||
| if dsn: | ||||||||||||||
| auth = Dsn(dsn).to_auth(f"sentry.python/{VERSION}") | ||||||||||||||
| endpoint = auth.get_api_url(EndpointType.OTLP_TRACES) | ||||||||||||||
| headers = {"X-Sentry-Auth": auth.to_header()} | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we set two headers in the transport and only one here? See sentry-python/sentry_sdk/transport.py Lines 325 to 330 in 659bd84
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good question! user agent is already set by otlp, which I think is fine to distinguish it from a normal (sentry-traces-only) user |
||||||||||||||
| logger.debug(f"[OTLP] Sending traces to {endpoint}") | ||||||||||||||
|
|
||||||||||||||
| otlp_exporter = OTLPSpanExporter(endpoint=endpoint, headers=headers) | ||||||||||||||
| span_processor = BatchSpanProcessor(otlp_exporter) | ||||||||||||||
| tracer_provider.add_span_processor(span_processor) | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| class OTLPIntegration(Integration): | ||||||||||||||
| identifier = "otlp" | ||||||||||||||
|
|
||||||||||||||
| def __init__(self, setup_otlp_exporter=True, setup_propagator=True): | ||||||||||||||
| # type: (bool, bool) -> None | ||||||||||||||
| self.setup_otlp_exporter = setup_otlp_exporter | ||||||||||||||
| self.setup_propagator = setup_propagator | ||||||||||||||
|
|
||||||||||||||
| @staticmethod | ||||||||||||||
| def setup_once(): | ||||||||||||||
| # type: () -> None | ||||||||||||||
| logger.debug("[OTLP] Setting up trace linking for all events") | ||||||||||||||
| register_external_propagation_context(otel_propagation_context) | ||||||||||||||
sl0thentr0py marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
|
|
||||||||||||||
| def setup_once_with_options(self, options=None): | ||||||||||||||
| # type: (Optional[Dict[str, Any]]) -> None | ||||||||||||||
| if self.setup_otlp_exporter: | ||||||||||||||
| logger.debug("[OTLP] Setting up OTLP exporter") | ||||||||||||||
| dsn = options.get("dsn") if options else None # type: Optional[str] | ||||||||||||||
| setup_otlp_exporter(dsn) | ||||||||||||||
|
|
||||||||||||||
| if self.setup_propagator: | ||||||||||||||
| logger.debug("[OTLP] Setting up propagator for distributed tracing") | ||||||||||||||
| # TODO-neel better propagator support, chain with existing ones if possible instead of replacing | ||||||||||||||
| set_global_textmap(SentryPropagator()) | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import pytest | ||
|
|
||
| pytest.importorskip("opentelemetry") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| import pytest | ||
| import responses | ||
|
|
||
| from opentelemetry import trace | ||
| from opentelemetry.trace import ( | ||
| get_tracer_provider, | ||
| set_tracer_provider, | ||
| ProxyTracerProvider, | ||
| format_span_id, | ||
| format_trace_id, | ||
| ) | ||
| from opentelemetry.propagate import get_global_textmap, set_global_textmap | ||
| from opentelemetry.util._once import Once | ||
| from opentelemetry.sdk.trace import TracerProvider | ||
| from opentelemetry.sdk.trace.export import BatchSpanProcessor | ||
| from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter | ||
|
|
||
| from sentry_sdk.integrations.otlp import OTLPIntegration | ||
| from sentry_sdk.integrations.opentelemetry import SentryPropagator | ||
| from sentry_sdk.scope import get_external_propagation_context | ||
|
|
||
|
|
||
| original_propagator = get_global_textmap() | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def reset_otlp(uninstall_integration): | ||
| trace._TRACER_PROVIDER_SET_ONCE = Once() | ||
| trace._TRACER_PROVIDER = None | ||
|
|
||
| set_global_textmap(original_propagator) | ||
|
|
||
| uninstall_integration("otlp") | ||
|
|
||
|
|
||
| def test_sets_new_tracer_provider_with_otlp_exporter(sentry_init): | ||
| existing_tracer_provider = get_tracer_provider() | ||
| assert isinstance(existing_tracer_provider, ProxyTracerProvider) | ||
|
|
||
| sentry_init( | ||
| dsn="https://mysecret@bla.ingest.sentry.io/12312012", | ||
| integrations=[OTLPIntegration()], | ||
| ) | ||
|
|
||
| tracer_provider = get_tracer_provider() | ||
| assert tracer_provider is not existing_tracer_provider | ||
| assert isinstance(tracer_provider, TracerProvider) | ||
|
|
||
| (span_processor,) = tracer_provider._active_span_processor._span_processors | ||
| assert isinstance(span_processor, BatchSpanProcessor) | ||
|
|
||
| exporter = span_processor.span_exporter | ||
| assert isinstance(exporter, OTLPSpanExporter) | ||
| assert ( | ||
| exporter._endpoint | ||
| == "https://bla.ingest.sentry.io/api/12312012/integration/otlp/v1/traces/" | ||
| ) | ||
| assert "X-Sentry-Auth" in exporter._headers | ||
| assert ( | ||
| "Sentry sentry_key=mysecret, sentry_version=7, sentry_client=sentry.python/" | ||
| in exporter._headers["X-Sentry-Auth"] | ||
| ) | ||
|
|
||
|
|
||
| def test_uses_existing_tracer_provider_with_otlp_exporter(sentry_init): | ||
| existing_tracer_provider = TracerProvider() | ||
| set_tracer_provider(existing_tracer_provider) | ||
|
|
||
| sentry_init( | ||
| dsn="https://mysecret@bla.ingest.sentry.io/12312012", | ||
| integrations=[OTLPIntegration()], | ||
| ) | ||
|
|
||
| tracer_provider = get_tracer_provider() | ||
| assert tracer_provider == existing_tracer_provider | ||
| assert isinstance(tracer_provider, TracerProvider) | ||
|
|
||
| (span_processor,) = tracer_provider._active_span_processor._span_processors | ||
| assert isinstance(span_processor, BatchSpanProcessor) | ||
|
|
||
| exporter = span_processor.span_exporter | ||
| assert isinstance(exporter, OTLPSpanExporter) | ||
| assert ( | ||
| exporter._endpoint | ||
| == "https://bla.ingest.sentry.io/api/12312012/integration/otlp/v1/traces/" | ||
| ) | ||
| assert "X-Sentry-Auth" in exporter._headers | ||
| assert ( | ||
| "Sentry sentry_key=mysecret, sentry_version=7, sentry_client=sentry.python/" | ||
| in exporter._headers["X-Sentry-Auth"] | ||
| ) | ||
|
|
||
|
|
||
| def test_does_not_setup_exporter_when_disabled(sentry_init): | ||
| existing_tracer_provider = get_tracer_provider() | ||
| assert isinstance(existing_tracer_provider, ProxyTracerProvider) | ||
|
|
||
| sentry_init( | ||
| dsn="https://mysecret@bla.ingest.sentry.io/12312012", | ||
| integrations=[OTLPIntegration(setup_otlp_exporter=False)], | ||
| ) | ||
|
|
||
| tracer_provider = get_tracer_provider() | ||
| assert tracer_provider is existing_tracer_provider | ||
|
|
||
|
|
||
| def test_sets_propagator(sentry_init): | ||
| sentry_init( | ||
| dsn="https://mysecret@bla.ingest.sentry.io/12312012", | ||
| integrations=[OTLPIntegration()], | ||
| ) | ||
|
|
||
| propagator = get_global_textmap() | ||
| assert isinstance(get_global_textmap(), SentryPropagator) | ||
| assert propagator is not original_propagator | ||
|
|
||
|
|
||
| def test_does_not_set_propagator_if_disabled(sentry_init): | ||
| sentry_init( | ||
| dsn="https://mysecret@bla.ingest.sentry.io/12312012", | ||
| integrations=[OTLPIntegration(setup_propagator=False)], | ||
| ) | ||
|
|
||
| propagator = get_global_textmap() | ||
| assert not isinstance(propagator, SentryPropagator) | ||
| assert propagator is original_propagator | ||
|
|
||
|
|
||
| @responses.activate | ||
| def test_otel_propagation_context(sentry_init): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When running this test I see the message below. I'm trying to find where this is coming from. Failed to export span batch code: 404, reason: You should be really using o*.ingest.sentry.io domains. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mocked |
||
| responses.add( | ||
| responses.POST, | ||
| url="https://bla.ingest.sentry.io/api/12312012/integration/otlp/v1/traces/", | ||
| status=200, | ||
| ) | ||
|
|
||
| sentry_init( | ||
| dsn="https://mysecret@bla.ingest.sentry.io/12312012", | ||
| integrations=[OTLPIntegration()], | ||
| ) | ||
|
|
||
| tracer = trace.get_tracer(__name__) | ||
| with tracer.start_as_current_span("foo") as root_span: | ||
| with tracer.start_as_current_span("bar") as span: | ||
| external_propagation_context = get_external_propagation_context() | ||
|
|
||
| # Force flush to ensure spans are exported while mock is active | ||
| get_tracer_provider().force_flush() | ||
|
|
||
| assert external_propagation_context is not None | ||
| (trace_id, span_id) = external_propagation_context | ||
| assert trace_id == format_trace_id(root_span.get_span_context().trace_id) | ||
| assert trace_id == format_trace_id(span.get_span_context().trace_id) | ||
| assert span_id == format_span_id(span.get_span_context().span_id) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last thing that'd be good before merging.
Since
pip install "opentelemetry-distro[otlp]"also pulls ingrpcio, andgrpciodoes not distribute free-threading wheels, these new tests take like 17 mins to run in CI on free-threading 3.14.https://github.com/getsentry/sentry-python/actions/runs/19139690916/job/54701021577?pr=4877
So I suggest we do not run the new test suite on
3.14t.Also, as an fyi: for running
populate_tox.py, I add auvcommand for running it with the new pre-requisites to the README in https://github.com/getsentry/sentry-python/pull/5076/filesThat said, in this case you can just edit
tox.jinjaandtox.inimanually, since it's just removing a single version.