diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cc0ef087..eccc719960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-urllib3` Updated `_RequestHookT` with two additional fields - the request body and the request headers ([#660](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/660)) +### Changed +- Tests for Falcon 3 support + ([#644](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/644)) + ### Added - `opentelemetry-instrumentation-urllib3`, `opentelemetry-instrumentation-requests` diff --git a/instrumentation/README.md b/instrumentation/README.md index 8c59661d55..fba704c81d 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -11,7 +11,7 @@ | [opentelemetry-instrumentation-dbapi](./opentelemetry-instrumentation-dbapi) | dbapi | | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | -| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon ~= 2.0 | +| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 2.0.0, < 4.0.0 | | [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0, < 3.0 | | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | diff --git a/instrumentation/opentelemetry-instrumentation-falcon/README.rst b/instrumentation/opentelemetry-instrumentation-falcon/README.rst index 24cb1cf45a..c192a2ae78 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/README.rst +++ b/instrumentation/opentelemetry-instrumentation-falcon/README.rst @@ -9,8 +9,6 @@ OpenTelemetry Falcon Tracing This library builds on the OpenTelemetry WSGI middleware to track web requests in Falcon applications. -Currently, only Falcon v2 is supported. - Installation ------------ 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 c8a802fdfb..dc39e03bf3 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -126,6 +126,13 @@ def response_hook(span, req, resp): _response_propagation_setter = FuncSetter(falcon.Response.append_header) +if hasattr(falcon, "App"): + # Falcon 3 + _instrument_app = "App" +else: + # Falcon 2 + _instrument_app = "API" + class FalconInstrumentor(BaseInstrumentor): # pylint: disable=protected-access,attribute-defined-outside-init @@ -138,14 +145,16 @@ def instrumentation_dependencies(self) -> Collection[str]: return _instruments def _instrument(self, **kwargs): - self._original_falcon_api = falcon.API - falcon.API = partial(_InstrumentedFalconAPI, **kwargs) + self._original_falcon_api = getattr(falcon, _instrument_app) + setattr( + falcon, _instrument_app, partial(_InstrumentedFalconAPI, **kwargs) + ) def _uninstrument(self, **kwargs): - falcon.API = self._original_falcon_api + setattr(falcon, _instrument_app, self._original_falcon_api) -class _InstrumentedFalconAPI(falcon.API): +class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): def __init__(self, *args, **kwargs): # inject trace middleware middlewares = kwargs.pop("middleware", []) @@ -169,6 +178,15 @@ def __init__(self, *args, **kwargs): self._excluded_urls = get_excluded_urls("FALCON") super().__init__(*args, **kwargs) + def _handle_exception( + self, req, resp, ex, params + ): # pylint: disable=C0103 + # Falcon 3 does not execute middleware within the context of the exception + # so we capture the exception here and save it into the env dict + _, exc, _ = exc_info() + req.env[_ENVIRON_EXC] = exc + return super()._handle_exception(req, resp, ex, params) + def __call__(self, env, start_response): # pylint: disable=E1101 if self._excluded_urls.url_disabled(env.get("PATH_INFO", "/")): @@ -193,7 +211,6 @@ def __call__(self, env, start_response): env[_ENVIRON_ACTIVATION_KEY] = activation def _start_response(status, response_headers, *args, **kwargs): - otel_wsgi.add_response_attributes(span, status, response_headers) response = start_response( status, response_headers, *args, **kwargs ) @@ -264,22 +281,23 @@ def process_response( if resource is None: status = "404" reason = "NotFound" - - exc_type, exc, _ = exc_info() - if exc_type and not req_succeeded: - if "HTTPNotFound" in exc_type.__name__: - status = "404" - reason = "NotFound" + else: + if _ENVIRON_EXC in req.env: + exc = req.env[_ENVIRON_EXC] + exc_type = type(exc) else: - status = "500" - reason = "{}: {}".format(exc_type.__name__, exc) + exc_type, exc = None, None + if exc_type and not req_succeeded: + if "HTTPNotFound" in exc_type.__name__: + status = "404" + reason = "NotFound" + else: + status = "500" + reason = "{}: {}".format(exc_type.__name__, exc) status = status.split(" ")[0] try: status_code = int(status) - except ValueError: - pass - finally: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) span.set_status( Status( @@ -287,6 +305,8 @@ def process_response( description=reason, ) ) + except ValueError: + pass propagator = get_global_response_propagator() if propagator: diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py index 7e554264ff..9cdd0f17cd 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/package.py @@ -13,4 +13,4 @@ # limitations under the License. -_instruments = ("falcon ~= 2.0",) +_instruments = ("falcon >= 2.0.0, < 4.0.0",) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py index dcbfe11b49..839c535fc8 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -34,7 +34,12 @@ def on_get(self, req, resp): def make_app(): - app = falcon.API() + if hasattr(falcon, "App"): + # Falcon 3 + app = falcon.App() + else: + # Falcon 2 + app = falcon.API() app.add_route("/hello", HelloWorldResource()) app.add_route("/ping", HelloWorldResource()) app.add_route("/error", ErrorResource()) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 4ffa4ee0bd..3f9c2975de 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -78,7 +78,9 @@ def test_head(self): self._test_method("HEAD") def _test_method(self, method): - self.client().simulate_request(method=method, path="/hello") + self.client().simulate_request( + method=method, path="/hello", remote_addr="127.0.0.1" + ) spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] @@ -86,6 +88,9 @@ def _test_method(self, method): span.name, "HelloWorldResource.on_{0}".format(method.lower()) ) self.assertEqual(span.status.status_code, StatusCode.UNSET) + self.assertEqual( + span.status.description, None, + ) self.assertSpanHasAttributes( span, { @@ -105,12 +110,15 @@ def _test_method(self, method): self.memory_exporter.clear() def test_404(self): - self.client().simulate_get("/does-not-exist") + self.client().simulate_get("/does-not-exist", remote_addr="127.0.0.1") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] self.assertEqual(span.name, "HTTP GET") self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual( + span.status.description, "NotFound", + ) self.assertSpanHasAttributes( span, { @@ -129,7 +137,7 @@ def test_404(self): def test_500(self): try: - self.client().simulate_get("/error") + self.client().simulate_get("/error", remote_addr="127.0.0.1") except NameError: pass spans = self.memory_exporter.get_finished_spans() diff --git a/pytest.ini b/pytest.ini index 013d2c555c..136015a6b0 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,3 +2,4 @@ addopts = -rs -v log_cli = true log_cli_level = warning +testpaths = instrumentation/opentelemetry-instrumentation-falcon/tests/ diff --git a/tox.ini b/tox.ini index b608b99aec..274a3f9264 100644 --- a/tox.ini +++ b/tox.ini @@ -38,8 +38,8 @@ envlist = pypy3-test-instrumentation-elasticsearch{2,5,6} ; opentelemetry-instrumentation-falcon - py3{4,5,6,7,8,9}-test-instrumentation-falcon - pypy3-test-instrumentation-falcon + py3{4,5,6,7,8,9}-test-instrumentation-falcon{2,3} + pypy3-test-instrumentation-falcon{2,3} ; opentelemetry-instrumentation-fastapi ; fastapi only supports 3.6 and above. @@ -175,6 +175,8 @@ deps = ; FIXME: Elasticsearch >=7 causes CI workflow tests to hang, see open-telemetry/opentelemetry-python-contrib#620 ; elasticsearch7: elasticsearch-dsl>=7.0,<8.0 ; elasticsearch7: elasticsearch>=7.0,<8.0 + falcon2: falcon >=2.0.0,<3.0.0 + falcon3: falcon >=3.0.0,<4.0.0 sqlalchemy11: sqlalchemy>=1.1,<1.2 sqlalchemy14: aiosqlite sqlalchemy14: sqlalchemy~=1.4 @@ -193,7 +195,7 @@ changedir = test-instrumentation-dbapi: instrumentation/opentelemetry-instrumentation-dbapi/tests test-instrumentation-django: instrumentation/opentelemetry-instrumentation-django/tests test-instrumentation-elasticsearch{2,5,6}: instrumentation/opentelemetry-instrumentation-elasticsearch/tests - test-instrumentation-falcon: instrumentation/opentelemetry-instrumentation-falcon/tests + test-instrumentation-falcon{2,3}: instrumentation/opentelemetry-instrumentation-falcon/tests test-instrumentation-fastapi: instrumentation/opentelemetry-instrumentation-fastapi/tests test-instrumentation-flask: instrumentation/opentelemetry-instrumentation-flask/tests test-instrumentation-urllib: instrumentation/opentelemetry-instrumentation-urllib/tests @@ -236,8 +238,8 @@ commands_pre = grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test] - falcon,flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test] - wsgi,falcon,flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + falcon{2,3},flask,django,pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test] + wsgi,falcon{2,3},flask,django,pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] asgi,starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] asyncpg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asyncpg[test] @@ -245,7 +247,7 @@ commands_pre = boto: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-botocore[test] boto: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-boto[test] - falcon: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-falcon[test] + falcon{2,3}: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-falcon[test] flask: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-flask[test]