From fc5db4f8c175d6affac6ea22b5041eb8f2de24a1 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Tue, 30 Jul 2024 13:12:15 +0200 Subject: [PATCH] ref(otel): Remove experimental autoinstrumentation (#3239) --- .../integrations/opentelemetry/distro.py | 66 -------- .../integrations/opentelemetry/integration.py | 156 +++--------------- setup.py | 56 +------ .../opentelemetry/test_experimental.py | 76 --------- tox.ini | 2 - 5 files changed, 25 insertions(+), 331 deletions(-) delete mode 100644 sentry_sdk/integrations/opentelemetry/distro.py diff --git a/sentry_sdk/integrations/opentelemetry/distro.py b/sentry_sdk/integrations/opentelemetry/distro.py deleted file mode 100644 index 87a49a09c3..0000000000 --- a/sentry_sdk/integrations/opentelemetry/distro.py +++ /dev/null @@ -1,66 +0,0 @@ -""" -IMPORTANT: The contents of this file are part of a proof of concept and as such -are experimental and not suitable for production use. They may be changed or -removed at any time without prior notice. -""" - -from sentry_sdk.integrations import DidNotEnable -from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator -from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor -from sentry_sdk.utils import logger -from sentry_sdk._types import TYPE_CHECKING - -try: - from opentelemetry import trace - from opentelemetry.instrumentation.distro import BaseDistro # type: ignore[attr-defined] - from opentelemetry.propagate import set_global_textmap - from opentelemetry.sdk.trace import TracerProvider -except ImportError: - raise DidNotEnable("opentelemetry not installed") - -try: - from opentelemetry.instrumentation.django import DjangoInstrumentor # type: ignore -except ImportError: - DjangoInstrumentor = None - -try: - from opentelemetry.instrumentation.flask import FlaskInstrumentor # type: ignore -except ImportError: - FlaskInstrumentor = None - -if TYPE_CHECKING: - # XXX pkg_resources is deprecated, there's a PR to switch to importlib: - # https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2181 - # we should align this when the PR gets merged - from pkg_resources import EntryPoint - from typing import Any - - -CONFIGURABLE_INSTRUMENTATIONS = { - DjangoInstrumentor: {"is_sql_commentor_enabled": True}, - FlaskInstrumentor: {"enable_commenter": True}, -} - - -class _SentryDistro(BaseDistro): # type: ignore[misc] - def _configure(self, **kwargs): - # type: (Any) -> None - provider = TracerProvider() - provider.add_span_processor(SentrySpanProcessor()) - trace.set_tracer_provider(provider) - set_global_textmap(SentryPropagator()) - - def load_instrumentor(self, entry_point, **kwargs): - # type: (EntryPoint, Any) -> None - instrumentor = entry_point.load() - - if instrumentor in CONFIGURABLE_INSTRUMENTATIONS: - for key, value in CONFIGURABLE_INSTRUMENTATIONS[instrumentor].items(): - kwargs[key] = value - - instrumentor().instrument(**kwargs) - logger.debug( - "[OTel] %s instrumented (%s)", - entry_point.name, - ", ".join([f"{k}: {v}" for k, v in kwargs.items()]), - ) diff --git a/sentry_sdk/integrations/opentelemetry/integration.py b/sentry_sdk/integrations/opentelemetry/integration.py index b765703f54..43e0396c16 100644 --- a/sentry_sdk/integrations/opentelemetry/integration.py +++ b/sentry_sdk/integrations/opentelemetry/integration.py @@ -4,32 +4,26 @@ removed at any time without prior notice. """ -import sys -from importlib import import_module - from sentry_sdk.integrations import DidNotEnable, Integration -from sentry_sdk.integrations.opentelemetry.distro import _SentryDistro -from sentry_sdk.utils import logger, _get_installed_modules -from sentry_sdk._types import TYPE_CHECKING +from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator +from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor +from sentry_sdk.utils import logger try: - from opentelemetry.instrumentation.auto_instrumentation._load import ( - _load_instrumentors, - ) + from opentelemetry import trace + from opentelemetry.propagate import set_global_textmap + from opentelemetry.sdk.trace import TracerProvider except ImportError: raise DidNotEnable("opentelemetry not installed") -if TYPE_CHECKING: - from typing import Dict +try: + from opentelemetry.instrumentation.django import DjangoInstrumentor # type: ignore[import-not-found] +except ImportError: + DjangoInstrumentor = None -CLASSES_TO_INSTRUMENT = { - # A mapping of packages to their entry point class that will be instrumented. - # This is used to post-instrument any classes that were imported before OTel - # instrumentation took place. - "fastapi": "fastapi.FastAPI", - "flask": "flask.Flask", - # XXX Add a mapping for all instrumentors that patch by replacing a class +CONFIGURABLE_INSTRUMENTATIONS = { + DjangoInstrumentor: {"is_sql_commentor_enabled": True}, } @@ -44,123 +38,21 @@ def setup_once(): "Use at your own risk." ) - original_classes = _record_unpatched_classes() - - try: - distro = _SentryDistro() - distro.configure() - # XXX This does some initial checks before loading instrumentations - # (checks OTEL_PYTHON_DISABLED_INSTRUMENTATIONS, checks version - # compat). If we don't want this in the future, we can implement our - # own _load_instrumentors (it anyway just iterates over - # opentelemetry_instrumentor entry points). - _load_instrumentors(distro) - except Exception: - logger.exception("[OTel] Failed to auto-initialize OpenTelemetry") - - # XXX: Consider whether this is ok to keep and make default. - # The alternative is asking folks to follow specific import order for - # some integrations (sentry_sdk.init before you even import Flask, for - # instance). - try: - _patch_remaining_classes(original_classes) - except Exception: - logger.exception( - "[OTel] Failed to post-patch instrumented classes. " - "You might have to make sure sentry_sdk.init() is called before importing anything else." - ) + _setup_sentry_tracing() + # _setup_instrumentors() logger.debug("[OTel] Finished setting up OpenTelemetry integration") -def _record_unpatched_classes(): - # type: () -> Dict[str, type] - """ - Keep references to classes that are about to be instrumented. - - Used to search for unpatched classes after the instrumentation has run so - that they can be patched manually. - """ - installed_packages = _get_installed_modules() - - original_classes = {} - - for package, orig_path in CLASSES_TO_INSTRUMENT.items(): - if package in installed_packages: - try: - original_cls = _import_by_path(orig_path) - except (AttributeError, ImportError): - logger.debug("[OTel] Failed to import %s", orig_path) - continue - - original_classes[package] = original_cls - - return original_classes - - -def _patch_remaining_classes(original_classes): - # type: (Dict[str, type]) -> None - """ - Best-effort attempt to patch any uninstrumented classes in sys.modules. - - This enables us to not care about the order of imports and sentry_sdk.init() - in user code. If e.g. the Flask class had been imported before sentry_sdk - was init()ed (and therefore before the OTel instrumentation ran), it would - not be instrumented. This function goes over remaining uninstrumented - occurrences of the class in sys.modules and replaces them with the - instrumented class. - - Since this is looking for exact matches, it will not work in some scenarios - (e.g. if someone is not using the specific class explicitly, but rather - inheriting from it). In those cases it's still necessary to sentry_sdk.init() - before importing anything that's supposed to be instrumented. - """ - # check which classes have actually been instrumented - instrumented_classes = {} - - for package in list(original_classes.keys()): - original_path = CLASSES_TO_INSTRUMENT[package] - - try: - cls = _import_by_path(original_path) - except (AttributeError, ImportError): - logger.debug( - "[OTel] Failed to check if class has been instrumented: %s", - original_path, - ) - del original_classes[package] - continue - - if not cls.__module__.startswith("opentelemetry."): - del original_classes[package] - continue - - instrumented_classes[package] = cls - - if not instrumented_classes: - return - - # replace occurrences of the original unpatched class in sys.modules - for module_name, module in sys.modules.copy().items(): - if ( - module_name.startswith("sentry_sdk") - or module_name in sys.builtin_module_names - ): - continue - - for package, original_cls in original_classes.items(): - for var_name, var in vars(module).copy().items(): - if var == original_cls: - logger.debug( - "[OTel] Additionally patching %s from %s", - original_cls, - module_name, - ) - - setattr(module, var_name, instrumented_classes[package]) +def _setup_sentry_tracing(): + # type: () -> None + provider = TracerProvider() + provider.add_span_processor(SentrySpanProcessor()) + trace.set_tracer_provider(provider) + set_global_textmap(SentryPropagator()) -def _import_by_path(path): - # type: (str) -> type - parts = path.rsplit(".", maxsplit=1) - return getattr(import_module(parts[0]), parts[-1]) +def _setup_instrumentors(): + # type: () -> None + for instrumentor, kwargs in CONFIGURABLE_INSTRUMENTATIONS.items(): + instrumentor().instrument(**kwargs) diff --git a/setup.py b/setup.py index 0cea2dd51d..09b5cb803e 100644 --- a/setup.py +++ b/setup.py @@ -65,61 +65,7 @@ def get_file_text(file_name): "loguru": ["loguru>=0.5"], "openai": ["openai>=1.0.0", "tiktoken>=0.3.0"], "opentelemetry": ["opentelemetry-distro>=0.35b0"], - "opentelemetry-experimental": [ - # There's an umbrella package called - # opentelemetry-contrib-instrumentations that installs all - # available instrumentation packages, however it's broken in recent - # versions (after 0.41b0), see - # https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2053 - "opentelemetry-instrumentation-aio-pika==0.46b0", - "opentelemetry-instrumentation-aiohttp-client==0.46b0", - # "opentelemetry-instrumentation-aiohttp-server==0.46b0", # broken package - "opentelemetry-instrumentation-aiopg==0.46b0", - "opentelemetry-instrumentation-asgi==0.46b0", - "opentelemetry-instrumentation-asyncio==0.46b0", - "opentelemetry-instrumentation-asyncpg==0.46b0", - "opentelemetry-instrumentation-aws-lambda==0.46b0", - "opentelemetry-instrumentation-boto==0.46b0", - "opentelemetry-instrumentation-boto3sqs==0.46b0", - "opentelemetry-instrumentation-botocore==0.46b0", - "opentelemetry-instrumentation-cassandra==0.46b0", - "opentelemetry-instrumentation-celery==0.46b0", - "opentelemetry-instrumentation-confluent-kafka==0.46b0", - "opentelemetry-instrumentation-dbapi==0.46b0", - "opentelemetry-instrumentation-django==0.46b0", - "opentelemetry-instrumentation-elasticsearch==0.46b0", - "opentelemetry-instrumentation-falcon==0.46b0", - "opentelemetry-instrumentation-fastapi==0.46b0", - "opentelemetry-instrumentation-flask==0.46b0", - "opentelemetry-instrumentation-grpc==0.46b0", - "opentelemetry-instrumentation-httpx==0.46b0", - "opentelemetry-instrumentation-jinja2==0.46b0", - "opentelemetry-instrumentation-kafka-python==0.46b0", - "opentelemetry-instrumentation-logging==0.46b0", - "opentelemetry-instrumentation-mysql==0.46b0", - "opentelemetry-instrumentation-mysqlclient==0.46b0", - "opentelemetry-instrumentation-pika==0.46b0", - "opentelemetry-instrumentation-psycopg==0.46b0", - "opentelemetry-instrumentation-psycopg2==0.46b0", - "opentelemetry-instrumentation-pymemcache==0.46b0", - "opentelemetry-instrumentation-pymongo==0.46b0", - "opentelemetry-instrumentation-pymysql==0.46b0", - "opentelemetry-instrumentation-pyramid==0.46b0", - "opentelemetry-instrumentation-redis==0.46b0", - "opentelemetry-instrumentation-remoulade==0.46b0", - "opentelemetry-instrumentation-requests==0.46b0", - "opentelemetry-instrumentation-sklearn==0.46b0", - "opentelemetry-instrumentation-sqlalchemy==0.46b0", - "opentelemetry-instrumentation-sqlite3==0.46b0", - "opentelemetry-instrumentation-starlette==0.46b0", - "opentelemetry-instrumentation-system-metrics==0.46b0", - "opentelemetry-instrumentation-threading==0.46b0", - "opentelemetry-instrumentation-tornado==0.46b0", - "opentelemetry-instrumentation-tortoiseorm==0.46b0", - "opentelemetry-instrumentation-urllib==0.46b0", - "opentelemetry-instrumentation-urllib3==0.46b0", - "opentelemetry-instrumentation-wsgi==0.46b0", - ], + "opentelemetry-experimental": ["opentelemetry-distro"], "pure_eval": ["pure_eval", "executing", "asttokens"], "pymongo": ["pymongo>=3.1"], "pyspark": ["pyspark>=2.4.4"], diff --git a/tests/integrations/opentelemetry/test_experimental.py b/tests/integrations/opentelemetry/test_experimental.py index 856858c599..8e4b703361 100644 --- a/tests/integrations/opentelemetry/test_experimental.py +++ b/tests/integrations/opentelemetry/test_experimental.py @@ -2,28 +2,6 @@ import pytest -try: - from flask import Flask - from fastapi import FastAPI -except ImportError: - pass - - -try: - import opentelemetry.instrumentation.asyncio # noqa: F401 - - # We actually expect all OTel instrumentation packages to be available, but - # for simplicity we just check for one here. - instrumentation_packages_installed = True -except ImportError: - instrumentation_packages_installed = False - - -needs_potel = pytest.mark.skipif( - not instrumentation_packages_installed, - reason="needs OTel instrumentor libraries installed", -) - @pytest.mark.forked def test_integration_enabled_if_option_is_on(sentry_init, reset_integrations): @@ -67,57 +45,3 @@ def test_integration_not_enabled_if_option_is_missing(sentry_init, reset_integra ): sentry_init() mocked_setup_once.assert_not_called() - - -@pytest.mark.forked -@needs_potel -def test_instrumentors_applied(sentry_init, reset_integrations): - flask_instrument_mock = MagicMock() - fastapi_instrument_mock = MagicMock() - - with patch( - "opentelemetry.instrumentation.flask.FlaskInstrumentor.instrument", - flask_instrument_mock, - ): - with patch( - "opentelemetry.instrumentation.fastapi.FastAPIInstrumentor.instrument", - fastapi_instrument_mock, - ): - sentry_init( - _experiments={ - "otel_powered_performance": True, - }, - ) - - flask_instrument_mock.assert_called_once() - fastapi_instrument_mock.assert_called_once() - - -@pytest.mark.forked -@needs_potel -def test_post_patching(sentry_init, reset_integrations): - assert not hasattr( - Flask(__name__), "_is_instrumented_by_opentelemetry" - ), "Flask is not patched at the start" - assert not hasattr( - FastAPI(), "_is_instrumented_by_opentelemetry" - ), "FastAPI is not patched at the start" - - sentry_init( - _experiments={ - "otel_powered_performance": True, - }, - ) - - flask = Flask(__name__) - fastapi = FastAPI() - - assert hasattr( - flask, "_is_instrumented_by_opentelemetry" - ), "Flask has been patched after init()" - assert flask._is_instrumented_by_opentelemetry is True - - assert hasattr( - fastapi, "_is_instrumented_by_opentelemetry" - ), "FastAPI has been patched after init()" - assert fastapi._is_instrumented_by_opentelemetry is True diff --git a/tox.ini b/tox.ini index de9eb0e74a..2b5ef6d8d2 100644 --- a/tox.ini +++ b/tox.ini @@ -505,8 +505,6 @@ deps = # OpenTelemetry Experimental (POTel) potel: -e .[opentelemetry-experimental] - potel: Flask<3 - potel: fastapi # pure_eval pure_eval: pure_eval