diff --git a/CHANGELOG.md b/CHANGELOG.md index c7549c25ef..23c4a79aa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1004])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1004) - `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-falcon` Falcon: Capture custom request/response headers in span attributes ([#1003])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1003) - `opentelemetry-instrumentation-elasticsearch` no longer creates unique span names by including search target, replaces them with `` and puts the value in attribute `elasticsearch.target` diff --git a/instrumentation/README.md b/instrumentation/README.md index d609e42326..20caa144bb 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/setup.cfg b/instrumentation/opentelemetry-instrumentation-falcon/setup.cfg index e5ea642115..f542604a56 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/setup.cfg +++ b/instrumentation/opentelemetry-instrumentation-falcon/setup.cfg @@ -45,6 +45,7 @@ install_requires = opentelemetry-instrumentation == 0.29b0 opentelemetry-api ~= 1.3 opentelemetry-semantic-conventions == 0.29b0 + packaging >= 20.0 [options.extras_require] test = 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 b28ac96e4c..86ebf4822d 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -146,6 +146,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 @@ -177,12 +178,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)): @@ -214,12 +222,30 @@ 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 + + if _falcon_version == 1: + return super()._handle_exception(ex, req, resp, params) + return super()._handle_exception(req, resp, ex, params) def __call__(self, env, start_response): @@ -311,7 +337,7 @@ def process_resource(self, req, resp, resource, params): def process_response( self, req, resp, resource, req_succeeded=None - ): # pylint:disable=R0201 + ): # pylint:disable=R0201,R0912 span = req.env.get(_ENVIRON_SPAN_KEY) if not span or not span.is_recording(): @@ -348,9 +374,16 @@ def process_response( description=reason, ) ) + + # Falcon 1 does not support response headers. So + # send an empty dict. + response_headers = {} + if _falcon_version > 1: + response_headers = resp.headers + if span.is_recording() and span.kind == trace.SpanKind.SERVER: otel_wsgi.add_custom_response_headers( - span, resp.headers.items() + span, response_headers.items() ) except ValueError: pass 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..6cc60faee6 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,14 @@ 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 1 and Falcon 2 + app = falcon.API() + else: # 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 9e1a525bab..5098937b2a 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -11,10 +11,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +# from unittest.mock import Mock, patch +import pytest +from falcon import __version__ as _falcon_verison from falcon import testing +from packaging import version as package_version from opentelemetry import trace from opentelemetry.instrumentation.falcon import FalconInstrumentor @@ -84,9 +87,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 +106,23 @@ 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, }, ) + # In falcon<3, NET_PEER_IP is always set by default to 127.0.0.1 + # In falcon>3, NET_PEER_IP is not set to anything by default to + # https://github.com/falconry/falcon/blob/5233d0abed977d9dab78ebadf305f5abe2eef07c/falcon/testing/helpers.py#L1168-L1172 # noqa + 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 +137,22 @@ 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, }, ) + # In falcon<3, NET_PEER_IP is always set by default to 127.0.0.1 + # In falcon>3, NET_PEER_IP is not set to anything by default to + # https://github.com/falconry/falcon/blob/5233d0abed977d9dab78ebadf305f5abe2eef07c/falcon/testing/helpers.py#L1168-L1172 # noqa + 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 +174,18 @@ 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, }, ) + # In falcon<3, NET_PEER_IP is always set by default to 127.0.0.1 + # In falcon>3, NET_PEER_IP is not set to anything by default to + # https://github.com/falconry/falcon/blob/5233d0abed977d9dab78ebadf305f5abe2eef07c/falcon/testing/helpers.py#L1168-L1172 # noqa + 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 +210,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 +220,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 +236,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 +282,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") @@ -343,6 +364,11 @@ def test_custom_request_header_not_added_in_internal_span(self): for key, _ in not_expected.items(): self.assertNotIn(key, span.attributes) + @pytest.mark.skipif( + condition=package_version.parse(_falcon_verison) + < package_version.parse("2.0.0"), + reason="falcon<2 does not implement custom response headers", + ) def test_custom_response_header_added_in_server_span(self): self.client().simulate_request( method="GET", path="/test_custom_response_headers" @@ -366,6 +392,11 @@ def test_custom_response_header_added_in_server_span(self): for key, _ in not_expected.items(): self.assertNotIn(key, span.attributes) + @pytest.mark.skipif( + condition=package_version.parse(_falcon_verison) + < package_version.parse("2.0.0"), + reason="falcon<2 does not implement custom response headers", + ) def test_custom_response_header_not_added_in_internal_span(self): tracer = trace.get_tracer(__name__) with tracer.start_as_current_span("test", kind=trace.SpanKind.SERVER): diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/bootstrap_gen.py index 66d4980ad6..6cb3a8d252 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 1fd07e51d1..ed3ad86413 100644 --- a/tox.ini +++ b/tox.ini @@ -59,8 +59,10 @@ envlist = pypy3-test-instrumentation-elasticsearch5 ; opentelemetry-instrumentation-falcon + ; py310 does not work with falcon 1 + py3{6,7,8,9}-test-instrumentation-falcon1 py3{6,7,8,9,10}-test-instrumentation-falcon{2,3} - pypy3-test-instrumentation-falcon{2,3} + pypy3-test-instrumentation-falcon{1,2,3} ; opentelemetry-instrumentation-fastapi ; fastapi only supports 3.6 and above. @@ -219,6 +221,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 @@ -258,7 +261,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 @@ -312,8 +315,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] @@ -323,7 +326,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]