Skip to content

Commit

Permalink
ref(otel): Remove experimental autoinstrumentation (#3239)
Browse files Browse the repository at this point in the history
  • Loading branch information
sentrivana authored Jul 30, 2024
1 parent 6bb2081 commit fc5db4f
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 331 deletions.
66 changes: 0 additions & 66 deletions sentry_sdk/integrations/opentelemetry/distro.py

This file was deleted.

156 changes: 24 additions & 132 deletions sentry_sdk/integrations/opentelemetry/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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},
}


Expand All @@ -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)
56 changes: 1 addition & 55 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
76 changes: 0 additions & 76 deletions tests/integrations/opentelemetry/test_experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Loading

0 comments on commit fc5db4f

Please sign in to comment.