From 3d2517d8d1635e69b4188521013cb16149da19d4 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 28 Aug 2023 11:10:46 +0200 Subject: [PATCH] Allow to use OTel for performance instrumentation (experimental) (#2272) To enable this experimental feature, install `sentry_sdk[opentelemetry-experimental]` and initialize the SDK with `_experiments={"otel_powered_performance": True}`. This sets up performance powered by OTel for a handful of the most popular Python frameworks/libraries like Django, Flask, FastAPI, requests. Note that this is a proof of concept which we might end up utilizing or not -- depending on how successful this attempt is at addressing the various issues we've identified with regards to our compatibility with OTel. As the goal was to make this work automatically without requiring the user to set anything up, the autoinstrumentation builds on what the official opentelemetry-instrument tool does, but without having to actually use it to run a program (opentelemetry-instrument python app.py). --- sentry_sdk/client.py | 11 +- sentry_sdk/consts.py | 1 + sentry_sdk/integrations/__init__.py | 70 +++---- .../integrations/opentelemetry/__init__.py | 4 + .../integrations/opentelemetry/integration.py | 174 ++++++++++++++++++ .../opentelemetry/span_processor.py | 19 ++ setup.py | 48 +++-- .../opentelemetry/test_experimental.py | 34 ++++ tests/test_basics.py | 6 +- 9 files changed, 312 insertions(+), 55 deletions(-) create mode 100644 sentry_sdk/integrations/opentelemetry/integration.py create mode 100644 tests/integrations/opentelemetry/test_experimental.py diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 75e44dd206..1a4b044abe 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -27,7 +27,7 @@ VERSION, ClientConstructor, ) -from sentry_sdk.integrations import setup_integrations +from sentry_sdk.integrations import _DEFAULT_INTEGRATIONS, setup_integrations from sentry_sdk.utils import ContextVar from sentry_sdk.sessions import SessionFlusher from sentry_sdk.envelope import Envelope @@ -237,6 +237,15 @@ def _capture_envelope(envelope): ) ) + if self.options["_experiments"].get("otel_powered_performance", False): + logger.debug( + "[OTel] Enabling experimental OTel-powered performance monitoring." + ) + self.options["instrumenter"] = INSTRUMENTER.OTEL + _DEFAULT_INTEGRATIONS.append( + "sentry_sdk.integrations.opentelemetry.OpenTelemetryIntegration", + ) + self.integrations = setup_integrations( self.options["integrations"], with_defaults=self.options["default_integrations"], diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 057e4b2196..3989e857e0 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -39,6 +39,7 @@ # TODO: Remove these 2 profiling related experiments "profiles_sample_rate": Optional[float], "profiler_mode": Optional[ProfilerMode], + "otel_powered_performance": Optional[bool], }, total=False, ) diff --git a/sentry_sdk/integrations/__init__.py b/sentry_sdk/integrations/__init__.py index 9870471623..0fe958d217 100644 --- a/sentry_sdk/integrations/__init__.py +++ b/sentry_sdk/integrations/__init__.py @@ -1,12 +1,10 @@ -"""This package""" from __future__ import absolute_import - from threading import Lock from sentry_sdk._compat import iteritems +from sentry_sdk._types import TYPE_CHECKING from sentry_sdk.utils import logger -from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: from typing import Callable @@ -14,7 +12,6 @@ from typing import Iterator from typing import List from typing import Set - from typing import Tuple from typing import Type @@ -22,8 +19,11 @@ _installed_integrations = set() # type: Set[str] -def _generate_default_integrations_iterator(integrations, auto_enabling_integrations): - # type: (Tuple[str, ...], Tuple[str, ...]) -> Callable[[bool], Iterator[Type[Integration]]] +def _generate_default_integrations_iterator( + integrations, # type: List[str] + auto_enabling_integrations, # type: List[str] +): + # type: (...) -> Callable[[bool], Iterator[Type[Integration]]] def iter_default_integrations(with_auto_enabling_integrations): # type: (bool) -> Iterator[Type[Integration]] @@ -51,38 +51,40 @@ def iter_default_integrations(with_auto_enabling_integrations): return iter_default_integrations -_AUTO_ENABLING_INTEGRATIONS = ( - "sentry_sdk.integrations.django.DjangoIntegration", - "sentry_sdk.integrations.flask.FlaskIntegration", - "sentry_sdk.integrations.starlette.StarletteIntegration", - "sentry_sdk.integrations.fastapi.FastApiIntegration", +_DEFAULT_INTEGRATIONS = [ + # stdlib/base runtime integrations + "sentry_sdk.integrations.argv.ArgvIntegration", + "sentry_sdk.integrations.atexit.AtexitIntegration", + "sentry_sdk.integrations.dedupe.DedupeIntegration", + "sentry_sdk.integrations.excepthook.ExcepthookIntegration", + "sentry_sdk.integrations.logging.LoggingIntegration", + "sentry_sdk.integrations.modules.ModulesIntegration", + "sentry_sdk.integrations.stdlib.StdlibIntegration", + "sentry_sdk.integrations.threading.ThreadingIntegration", +] + +_AUTO_ENABLING_INTEGRATIONS = [ + "sentry_sdk.integrations.aiohttp.AioHttpIntegration", + "sentry_sdk.integrations.boto3.Boto3Integration", "sentry_sdk.integrations.bottle.BottleIntegration", - "sentry_sdk.integrations.falcon.FalconIntegration", - "sentry_sdk.integrations.sanic.SanicIntegration", "sentry_sdk.integrations.celery.CeleryIntegration", + "sentry_sdk.integrations.django.DjangoIntegration", + "sentry_sdk.integrations.falcon.FalconIntegration", + "sentry_sdk.integrations.fastapi.FastApiIntegration", + "sentry_sdk.integrations.flask.FlaskIntegration", + "sentry_sdk.integrations.httpx.HttpxIntegration", + "sentry_sdk.integrations.pyramid.PyramidIntegration", + "sentry_sdk.integrations.redis.RedisIntegration", "sentry_sdk.integrations.rq.RqIntegration", - "sentry_sdk.integrations.aiohttp.AioHttpIntegration", - "sentry_sdk.integrations.tornado.TornadoIntegration", + "sentry_sdk.integrations.sanic.SanicIntegration", "sentry_sdk.integrations.sqlalchemy.SqlalchemyIntegration", - "sentry_sdk.integrations.redis.RedisIntegration", - "sentry_sdk.integrations.pyramid.PyramidIntegration", - "sentry_sdk.integrations.boto3.Boto3Integration", - "sentry_sdk.integrations.httpx.HttpxIntegration", -) + "sentry_sdk.integrations.starlette.StarletteIntegration", + "sentry_sdk.integrations.tornado.TornadoIntegration", +] iter_default_integrations = _generate_default_integrations_iterator( - integrations=( - # stdlib/base runtime integrations - "sentry_sdk.integrations.logging.LoggingIntegration", - "sentry_sdk.integrations.stdlib.StdlibIntegration", - "sentry_sdk.integrations.excepthook.ExcepthookIntegration", - "sentry_sdk.integrations.dedupe.DedupeIntegration", - "sentry_sdk.integrations.atexit.AtexitIntegration", - "sentry_sdk.integrations.modules.ModulesIntegration", - "sentry_sdk.integrations.argv.ArgvIntegration", - "sentry_sdk.integrations.threading.ThreadingIntegration", - ), + integrations=_DEFAULT_INTEGRATIONS, auto_enabling_integrations=_AUTO_ENABLING_INTEGRATIONS, ) @@ -93,8 +95,10 @@ def setup_integrations( integrations, with_defaults=True, with_auto_enabling_integrations=False ): # type: (List[Integration], bool, bool) -> Dict[str, Integration] - """Given a list of integration instances this installs them all. When - `with_defaults` is set to `True` then all default integrations are added + """ + Given a list of integration instances, this installs them all. + + When `with_defaults` is set to `True` all default integrations are added unless they were already provided before. """ integrations = dict( diff --git a/sentry_sdk/integrations/opentelemetry/__init__.py b/sentry_sdk/integrations/opentelemetry/__init__.py index e0020204d5..158f49a658 100644 --- a/sentry_sdk/integrations/opentelemetry/__init__.py +++ b/sentry_sdk/integrations/opentelemetry/__init__.py @@ -1,3 +1,7 @@ +from sentry_sdk.integrations.opentelemetry.integration import ( # noqa: F401 + OpenTelemetryIntegration, +) + from sentry_sdk.integrations.opentelemetry.span_processor import ( # noqa: F401 SentrySpanProcessor, ) diff --git a/sentry_sdk/integrations/opentelemetry/integration.py b/sentry_sdk/integrations/opentelemetry/integration.py new file mode 100644 index 0000000000..20dc4625df --- /dev/null +++ b/sentry_sdk/integrations/opentelemetry/integration.py @@ -0,0 +1,174 @@ +""" +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. +""" +import sys +from importlib import import_module + +from sentry_sdk.integrations import DidNotEnable, Integration +from sentry_sdk.integrations.opentelemetry.span_processor import SentrySpanProcessor +from sentry_sdk.integrations.opentelemetry.propagator import SentryPropagator +from sentry_sdk.integrations.modules import _get_installed_modules +from sentry_sdk.utils import logger +from sentry_sdk._types import TYPE_CHECKING + +try: + from opentelemetry import trace # type: ignore + from opentelemetry.instrumentation.auto_instrumentation._load import ( # type: ignore + _load_distro, + _load_instrumentors, + ) + from opentelemetry.propagate import set_global_textmap # type: ignore + from opentelemetry.sdk.trace import TracerProvider # type: ignore +except ImportError: + raise DidNotEnable("opentelemetry not installed") + +if TYPE_CHECKING: + from typing import Dict + + +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", +} + + +class OpenTelemetryIntegration(Integration): + identifier = "opentelemetry" + + @staticmethod + def setup_once(): + # type: () -> None + logger.warning( + "[OTel] Initializing highly experimental OpenTelemetry support. " + "Use at your own risk." + ) + + original_classes = _record_unpatched_classes() + + try: + distro = _load_distro() + distro.configure() + _load_instrumentors(distro) + except Exception: + logger.exception("[OTel] Failed to auto-initialize OpenTelemetry") + + 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() + + 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 _import_by_path(path): + # type: (str) -> type + parts = path.rsplit(".", maxsplit=1) + return getattr(import_module(parts[0]), parts[-1]) + + +def _setup_sentry_tracing(): + # type: () -> None + provider = TracerProvider() + + provider.add_span_processor(SentrySpanProcessor()) + + trace.set_tracer_provider(provider) + + set_global_textmap(SentryPropagator()) diff --git a/sentry_sdk/integrations/opentelemetry/span_processor.py b/sentry_sdk/integrations/opentelemetry/span_processor.py index bb53da198e..9dd15bfb3e 100644 --- a/sentry_sdk/integrations/opentelemetry/span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/span_processor.py @@ -169,6 +169,7 @@ def on_end(self, otel_span): sentry_span.set_context( OPEN_TELEMETRY_CONTEXT, self._get_otel_context(otel_span) ) + self._update_transaction_with_otel_data(sentry_span, otel_span) else: self._update_span_with_otel_data(sentry_span, otel_span) @@ -306,3 +307,21 @@ def _update_span_with_otel_data(self, sentry_span, otel_span): sentry_span.op = op sentry_span.description = description + + def _update_transaction_with_otel_data(self, sentry_span, otel_span): + # type: (SentrySpan, OTelSpan) -> None + http_method = otel_span.attributes.get(SpanAttributes.HTTP_METHOD) + + if http_method: + status_code = otel_span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) + if status_code: + sentry_span.set_http_status(status_code) + + op = "http" + + if otel_span.kind == SpanKind.SERVER: + op += ".server" + elif otel_span.kind == SpanKind.CLIENT: + op += ".client" + + sentry_span.op = op diff --git a/setup.py b/setup.py index 1f83681959..dc07ac4fef 100644 --- a/setup.py +++ b/setup.py @@ -40,35 +40,45 @@ def get_file_text(file_name): install_requires=[ 'urllib3>=1.25.7; python_version<="3.4"', 'urllib3>=1.26.9; python_version=="3.5"', - 'urllib3>=1.26.11; python_version >="3.6"', + 'urllib3>=1.26.11; python_version>="3.6"', "certifi", ], extras_require={ - "flask": ["flask>=0.11", "blinker>=1.1", "markupsafe"], - "quart": ["quart>=0.16.1", "blinker>=1.1"], + "aiohttp": ["aiohttp>=3.5"], + "arq": ["arq>=0.23"], + "beam": ["apache-beam>=2.12"], "bottle": ["bottle>=0.12.13"], - "falcon": ["falcon>=1.4"], - "django": ["django>=1.8"], - "sanic": ["sanic>=0.8"], "celery": ["celery>=3"], + "chalice": ["chalice>=1.16.0"], + "django": ["django>=1.8"], + "falcon": ["falcon>=1.4"], + "fastapi": ["fastapi>=0.79.0"], + "flask": ["flask>=0.11", "blinker>=1.1", "markupsafe"], + "grpcio": ["grpcio>=1.21.1"], + "httpx": ["httpx>=0.16.0"], "huey": ["huey>=2"], - "beam": ["apache-beam>=2.12"], - "arq": ["arq>=0.23"], + "loguru": ["loguru>=0.5"], + "opentelemetry": ["opentelemetry-distro>=0.35b0"], + "opentelemetry-experimental": [ + "opentelemetry-distro~=0.40b0", + "opentelemetry-instrumentation-aiohttp-client~=0.40b0", + "opentelemetry-instrumentation-django~=0.40b0", + "opentelemetry-instrumentation-fastapi~=0.40b0", + "opentelemetry-instrumentation-flask~=0.40b0", + "opentelemetry-instrumentation-requests~=0.40b0", + "opentelemetry-instrumentation-sqlite3~=0.40b0", + "opentelemetry-instrumentation-urllib~=0.40b0", + ], + "pure_eval": ["pure_eval", "executing", "asttokens"], + "pymongo": ["pymongo>=3.1"], + "pyspark": ["pyspark>=2.4.4"], + "quart": ["quart>=0.16.1", "blinker>=1.1"], "rq": ["rq>=0.6"], - "aiohttp": ["aiohttp>=3.5"], - "tornado": ["tornado>=5"], + "sanic": ["sanic>=0.8"], "sqlalchemy": ["sqlalchemy>=1.2"], - "pyspark": ["pyspark>=2.4.4"], - "pure_eval": ["pure_eval", "executing", "asttokens"], - "chalice": ["chalice>=1.16.0"], - "httpx": ["httpx>=0.16.0"], "starlette": ["starlette>=0.19.1"], "starlite": ["starlite>=1.48"], - "fastapi": ["fastapi>=0.79.0"], - "pymongo": ["pymongo>=3.1"], - "opentelemetry": ["opentelemetry-distro>=0.35b0"], - "grpcio": ["grpcio>=1.21.1"], - "loguru": ["loguru>=0.5"], + "tornado": ["tornado>=5"], }, classifiers=[ "Development Status :: 5 - Production/Stable", diff --git a/tests/integrations/opentelemetry/test_experimental.py b/tests/integrations/opentelemetry/test_experimental.py new file mode 100644 index 0000000000..77286330a5 --- /dev/null +++ b/tests/integrations/opentelemetry/test_experimental.py @@ -0,0 +1,34 @@ +try: + # python 3.3 and above + from unittest.mock import MagicMock +except ImportError: + # python < 3.3 + from mock import MagicMock + +from sentry_sdk.integrations.opentelemetry.integration import OpenTelemetryIntegration + + +def test_integration_enabled_if_option_is_on(sentry_init): + OpenTelemetryIntegration.setup_once = MagicMock() + sentry_init( + _experiments={ + "otel_powered_performance": True, + } + ) + OpenTelemetryIntegration.setup_once.assert_called_once() + + +def test_integration_not_enabled_if_option_is_off(sentry_init): + OpenTelemetryIntegration.setup_once = MagicMock() + sentry_init( + _experiments={ + "otel_powered_performance": False, + } + ) + OpenTelemetryIntegration.setup_once.assert_not_called() + + +def test_integration_not_enabled_if_option_is_missing(sentry_init): + OpenTelemetryIntegration.setup_once = MagicMock() + sentry_init() + OpenTelemetryIntegration.setup_once.assert_not_called() diff --git a/tests/test_basics.py b/tests/test_basics.py index 751b0a617b..b2b8846eb9 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -52,14 +52,16 @@ def error_processor(event, exc_info): def test_auto_enabling_integrations_catches_import_error(sentry_init, caplog): caplog.set_level(logging.DEBUG) - REDIS = 12 # noqa: N806 + redis_index = _AUTO_ENABLING_INTEGRATIONS.index( + "sentry_sdk.integrations.redis.RedisIntegration" + ) # noqa: N806 sentry_init(auto_enabling_integrations=True, debug=True) for import_string in _AUTO_ENABLING_INTEGRATIONS: # Ignore redis in the test case, because it is installed as a # dependency for running tests, and therefore always enabled. - if _AUTO_ENABLING_INTEGRATIONS[REDIS] == import_string: + if _AUTO_ENABLING_INTEGRATIONS[redis_index] == import_string: continue assert any(