From f794392105ff32eb48dca52b9c6fd4c8c3924449 Mon Sep 17 00:00:00 2001 From: Owais Lone Date: Thu, 3 Feb 2022 06:48:34 +0530 Subject: [PATCH] Safer patching for Falcon API We replace Falcon API class with a partial callable. It is safer to replace it with a sub-class of the base falcon.API class so any other systems making assumptions about falcon don't fail. --- CHANGELOG.md | 4 +- .../instrumentation/falcon/__init__.py | 67 ++++++++++--------- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1fb72c6a3..e141537a4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.9.1-0.28b1...HEAD) -## [1.9.1-0.28b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.9.1-0.28b1) - 2022-01-29 +- `opentelemetry-instrumentation-falcon` Safer patching mechanism + ([#895](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/895)) +## [1.9.1-0.28b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.9.1-0.28b1) - 2022-01-29 ### Fixed diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 4c19f9a1d7..13eed10c25 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -90,7 +90,6 @@ def response_hook(span, req, resp): --- """ -from functools import partial from logging import getLogger from sys import exc_info from typing import Collection @@ -135,48 +134,32 @@ def response_hook(span, req, resp): _instrument_app = "API" -class FalconInstrumentor(BaseInstrumentor): - # pylint: disable=protected-access,attribute-defined-outside-init - """An instrumentor for falcon.API - - See `BaseInstrumentor` - """ - - def instrumentation_dependencies(self) -> Collection[str]: - return _instruments - - def _instrument(self, **kwargs): - self._original_falcon_api = getattr(falcon, _instrument_app) - setattr( - falcon, _instrument_app, partial(_InstrumentedFalconAPI, **kwargs) - ) - - def _uninstrument(self, **kwargs): - setattr(falcon, _instrument_app, self._original_falcon_api) - - class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): def __init__(self, *args, **kwargs): + otel_opts = kwargs.pop("_otel_opts", {}) + # inject trace middleware middlewares = kwargs.pop("middleware", []) - tracer_provider = kwargs.pop("tracer_provider", None) + tracer_provider = otel_opts.pop("tracer_provider", None) if not isinstance(middlewares, (list, tuple)): middlewares = [middlewares] - self._tracer = trace.get_tracer(__name__, __version__, tracer_provider) + self._otel_tracer = trace.get_tracer( + __name__, __version__, tracer_provider + ) trace_middleware = _TraceMiddleware( - self._tracer, - kwargs.pop( + self._otel_tracer, + otel_opts.pop( "traced_request_attributes", get_traced_request_attrs("FALCON") ), - kwargs.pop("request_hook", None), - kwargs.pop("response_hook", None), + otel_opts.pop("request_hook", None), + otel_opts.pop("response_hook", None), ) middlewares.insert(0, trace_middleware) kwargs["middleware"] = middlewares - self._excluded_urls = get_excluded_urls("FALCON") + self._otel_excluded_urls = get_excluded_urls("FALCON") super().__init__(*args, **kwargs) def _handle_exception( @@ -190,13 +173,13 @@ def _handle_exception( def __call__(self, env, start_response): # pylint: disable=E1101 - if self._excluded_urls.url_disabled(env.get("PATH_INFO", "/")): + if self._otel_excluded_urls.url_disabled(env.get("PATH_INFO", "/")): return super().__call__(env, start_response) start_time = _time_ns() span, token = _start_internal_or_server_span( - tracer=self._tracer, + tracer=self._otel_tracer, span_name=otel_wsgi.get_default_span_name(env), start_time=start_time, context_carrier=env, @@ -321,3 +304,27 @@ def process_response( if self._response_hook: self._response_hook(span, req, resp) + + +class FalconInstrumentor(BaseInstrumentor): + # pylint: disable=protected-access,attribute-defined-outside-init + """An instrumentor for falcon.API + + See `BaseInstrumentor` + """ + + def instrumentation_dependencies(self) -> Collection[str]: + return _instruments + + def _instrument(self, **opts): + self._original_falcon_api = getattr(falcon, _instrument_app) + + class FalconAPI(_InstrumentedFalconAPI): + def __init__(self, *args, **kwargs): + kwargs["_otel_opts"] = opts + super().__init__(*args, **kwargs) + + setattr(falcon, _instrument_app, FalconAPI) + + def _uninstrument(self, **kwargs): + setattr(falcon, _instrument_app, self._original_falcon_api)