From 561520141995341c71817836a7cf66f91942b58b Mon Sep 17 00:00:00 2001 From: Robert Duffner Date: Thu, 3 Mar 2022 16:26:46 -0800 Subject: [PATCH] Add falcon version 1.4.1 support to opentelemetry-instrumentation-falcon --- CHANGELOG.md | 2 ++ instrumentation/README.md | 2 +- .../instrumentation/falcon/__init__.py | 34 ++++++++++++++++--- .../instrumentation/falcon/package.py | 2 +- .../tests/app.py | 10 ++++-- .../tests/test_falcon.py | 33 +++++++++++------- .../instrumentation/bootstrap_gen.py | 2 +- tox.ini | 13 +++---- 8 files changed, 71 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c62672bf7..4d6d92aa8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-psycopg2` extended the sql commenter support of dbapi into psycopg2 ([#940](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/940)) +- `opentelemetry-instrumentation-falcon` Add support for falcon==1.4.1 + ([#1000])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1000) - `opentelemetry-instrumentation-flask` Fix non-recording span bug ([#999])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/999) - `opentelemetry-instrumentation-tornado` Fix non-recording span bug diff --git a/instrumentation/README.md b/instrumentation/README.md index 1421653a70..b2c2fc93db 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -12,7 +12,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.0, < 4.0.0 | +| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 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/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 4ee643412f..2e7c9e997f 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -95,6 +95,7 @@ def response_hook(span, req, resp): from typing import Collection import falcon +from packaging import version as package_version import opentelemetry.instrumentation.wsgi as otel_wsgi from opentelemetry import context, trace @@ -126,12 +127,19 @@ def response_hook(span, req, resp): _response_propagation_setter = FuncSetter(falcon.Response.append_header) -if hasattr(falcon, "App"): +_parsed_falcon_version = package_version.parse(falcon.__version__) +if _parsed_falcon_version >= package_version.parse("3.0.0"): # Falcon 3 _instrument_app = "App" -else: + _falcon_version = 3 +elif _parsed_falcon_version >= package_version.parse("2.0.0"): # Falcon 2 _instrument_app = "API" + _falcon_version = 2 +else: + # Falcon 1 + _instrument_app = "API" + _falcon_version = 1 class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): @@ -163,13 +171,31 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def _handle_exception( - self, req, resp, ex, params + self, arg1, arg2, arg3, arg4 ): # 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 + + # Translation layer for handling the changed arg position of "ex" in Falcon > 2 vs + # Falcon < 2 + if _falcon_version == 1: + ex = arg1 + req = arg2 + resp = arg3 + params = arg4 + else: + req = arg1 + resp = arg2 + ex = arg3 + params = arg4 + _, exc, _ = exc_info() req.env[_ENVIRON_EXC] = exc - return super()._handle_exception(req, resp, ex, params) + + if _falcon_version == 1: + return super()._handle_exception(ex, req, resp, params) + else: + return super()._handle_exception(req, resp, ex, params) def __call__(self, env, start_response): # pylint: disable=E1101 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 9cdd0f17cd..bb705f6da1 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.0, < 4.0.0",) +_instruments = ("falcon >= 1.4.1, < 4.0.0",) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py index c173d5915e..fee47855fb 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/app.py @@ -1,4 +1,5 @@ import falcon +from packaging import version as package_version # pylint:disable=R0201,W0613,E0602 @@ -46,12 +47,17 @@ def on_get(self, _, resp): def make_app(): - if hasattr(falcon, "App"): + _parsed_falcon_version = package_version.parse(falcon.__version__) + if _parsed_falcon_version >= package_version.parse("3.0.0"): # Falcon 3 app = falcon.App() - else: + elif _parsed_falcon_version >= package_version.parse("2.0.0"): # Falcon 2 app = falcon.API() + else: + # Falcon 1 + 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 9e1a525bab..39830e15ea 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -84,9 +84,7 @@ def test_head(self): self._test_method("HEAD") def _test_method(self, method): - self.client().simulate_request( - method=method, path="/hello", remote_addr="127.0.0.1" - ) + self.client().simulate_request(method=method, path="/hello") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] @@ -105,17 +103,20 @@ def _test_method(self, method): SpanAttributes.NET_HOST_PORT: 80, SpanAttributes.HTTP_HOST: "falconframework.org", SpanAttributes.HTTP_TARGET: "/", - SpanAttributes.NET_PEER_IP: "127.0.0.1", SpanAttributes.NET_PEER_PORT: "65133", SpanAttributes.HTTP_FLAVOR: "1.1", "falcon.resource": "HelloWorldResource", SpanAttributes.HTTP_STATUS_CODE: 201, }, ) + if SpanAttributes.NET_PEER_IP in span.attributes: + self.assertEqual( + span.attributes[SpanAttributes.NET_PEER_IP], "127.0.0.1" + ) self.memory_exporter.clear() def test_404(self): - self.client().simulate_get("/does-not-exist", remote_addr="127.0.0.1") + self.client().simulate_get("/does-not-exist") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 1) span = spans[0] @@ -130,16 +131,19 @@ def test_404(self): SpanAttributes.NET_HOST_PORT: 80, SpanAttributes.HTTP_HOST: "falconframework.org", SpanAttributes.HTTP_TARGET: "/", - SpanAttributes.NET_PEER_IP: "127.0.0.1", SpanAttributes.NET_PEER_PORT: "65133", SpanAttributes.HTTP_FLAVOR: "1.1", SpanAttributes.HTTP_STATUS_CODE: 404, }, ) + if SpanAttributes.NET_PEER_IP in span.attributes: + self.assertEqual( + span.attributes[SpanAttributes.NET_PEER_IP], "127.0.0.1" + ) def test_500(self): try: - self.client().simulate_get("/error", remote_addr="127.0.0.1") + self.client().simulate_get("/error") except NameError: pass spans = self.memory_exporter.get_finished_spans() @@ -161,12 +165,15 @@ def test_500(self): SpanAttributes.NET_HOST_PORT: 80, SpanAttributes.HTTP_HOST: "falconframework.org", SpanAttributes.HTTP_TARGET: "/", - SpanAttributes.NET_PEER_IP: "127.0.0.1", SpanAttributes.NET_PEER_PORT: "65133", SpanAttributes.HTTP_FLAVOR: "1.1", SpanAttributes.HTTP_STATUS_CODE: 500, }, ) + if SpanAttributes.NET_PEER_IP in span.attributes: + self.assertEqual( + span.attributes[SpanAttributes.NET_PEER_IP], "127.0.0.1" + ) def test_uninstrument(self): self.client().simulate_get(path="/hello") @@ -191,7 +198,7 @@ def test_exclude_lists(self): self.assertEqual(len(span_list), 1) def test_traced_request_attributes(self): - self.client().simulate_get(path="/hello?q=abc") + self.client().simulate_get(path="/hello", query_string="q=abc") span = self.memory_exporter.get_finished_spans()[0] self.assertIn("query_string", span.attributes) self.assertEqual(span.attributes["query_string"], "q=abc") @@ -201,7 +208,9 @@ def test_trace_response(self): orig = get_global_response_propagator() set_global_response_propagator(TraceResponsePropagator()) - response = self.client().simulate_get(path="/hello?q=abc") + response = self.client().simulate_get( + path="/hello", query_string="q=abc" + ) self.assertTraceResponseHeaderMatchesSpan( response.headers, self.memory_exporter.get_finished_spans()[0] ) @@ -215,7 +224,7 @@ def test_traced_not_recording(self): mock_tracer.start_span.return_value = mock_span with patch("opentelemetry.trace.get_tracer") as tracer: tracer.return_value = mock_tracer - self.client().simulate_get(path="/hello?q=abc") + self.client().simulate_get(path="/hello", query_string="q=abc") self.assertFalse(mock_span.is_recording()) self.assertTrue(mock_span.is_recording.called) self.assertFalse(mock_span.set_attribute.called) @@ -261,7 +270,7 @@ def response_hook(self, span, req, resp): span.update_name("set from hook") def test_hooks(self): - self.client().simulate_get(path="/hello?q=abc") + self.client().simulate_get(path="/hello", query_string="q=abc") span = self.memory_exporter.get_finished_spans()[0] self.assertEqual(span.name, "set from hook") diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index a159d3bae9..ff4837f5fd 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py @@ -53,7 +53,7 @@ "instrumentation": "opentelemetry-instrumentation-elasticsearch==0.29b0", }, "falcon": { - "library": "falcon >= 2.0.0, < 4.0.0", + "library": "falcon >= 1.4.1, < 4.0.0", "instrumentation": "opentelemetry-instrumentation-falcon==0.29b0", }, "fastapi": { diff --git a/tox.ini b/tox.ini index d2e1dd0f04..1e1d88615f 100644 --- a/tox.ini +++ b/tox.ini @@ -59,8 +59,8 @@ envlist = pypy3-test-instrumentation-elasticsearch5 ; opentelemetry-instrumentation-falcon - py3{6,7,8,9,10}-test-instrumentation-falcon{2,3} - pypy3-test-instrumentation-falcon{2,3} + py3{6,7,8,9,10}-test-instrumentation-falcon{1,2,3} + pypy3-test-instrumentation-falcon{1,2,3} ; opentelemetry-instrumentation-fastapi ; fastapi only supports 3.6 and above. @@ -215,6 +215,7 @@ 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 + falcon1: falcon ==1.4.1 falcon2: falcon >=2.0.0,<3.0.0 falcon3: falcon >=3.0.0,<4.0.0 sqlalchemy11: sqlalchemy>=1.1,<1.2 @@ -250,7 +251,7 @@ changedir = test-instrumentation-dbapi: instrumentation/opentelemetry-instrumentation-dbapi/tests test-instrumentation-django{1,2,3,4}: instrumentation/opentelemetry-instrumentation-django/tests test-instrumentation-elasticsearch{2,5,6}: instrumentation/opentelemetry-instrumentation-elasticsearch/tests - test-instrumentation-falcon{2,3}: instrumentation/opentelemetry-instrumentation-falcon/tests + test-instrumentation-falcon{1,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 @@ -303,8 +304,8 @@ commands_pre = grpc: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-grpc[test] - falcon{2,3},flask,django{1,2,3,4},pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test] - wsgi,falcon{2,3},flask,django{1,2,3,4},pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] + falcon{1,2,3},flask,django{1,2,3,4},pyramid,tornado,starlette,fastapi,aiohttp,asgi,requests,urllib,urllib3,wsgi: pip install {toxinidir}/util/opentelemetry-util-http[test] + wsgi,falcon{1,2,3},flask,django{1,2,3,4},pyramid: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-wsgi[test] asgi,django{3,4},starlette,fastapi: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asgi[test] asyncpg: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-asyncpg[test] @@ -314,7 +315,7 @@ commands_pre = boto: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-botocore[test] boto: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-boto[test] - falcon{2,3}: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-falcon[test] + falcon{1,2,3}: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-falcon[test] flask: pip install {toxinidir}/instrumentation/opentelemetry-instrumentation-flask[test]