Add falcon version 1.4.1 support to opentelemetry-instrumentation-falcon (#1000)
This commit is contained in:
		
							parent
							
								
									4ad7592256
								
							
						
					
					
						commit
						abdd25f644
					
				|  | @ -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 `<target>` and puts the value in attribute `elasticsearch.target` | ||||
|  |  | |||
|  | @ -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 | | ||||
|  |  | |||
|  | @ -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 = | ||||
|  |  | |||
|  | @ -146,6 +146,7 @@ from sys import exc_info | |||
| 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 @@ _ENVIRON_EXC = "opentelemetry-falcon.exc" | |||
| 
 | ||||
| _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 @@ class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): | |||
|         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 @@ class _TraceMiddleware: | |||
| 
 | ||||
|     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 @@ class _TraceMiddleware: | |||
|                     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 | ||||
|  |  | |||
|  | @ -13,4 +13,4 @@ | |||
| # limitations under the License. | ||||
| 
 | ||||
| 
 | ||||
| _instruments = ("falcon >= 2.0.0, < 4.0.0",) | ||||
| _instruments = ("falcon >= 1.4.1, < 4.0.0",) | ||||
|  |  | |||
|  | @ -1,4 +1,5 @@ | |||
| import falcon | ||||
| from packaging import version as package_version | ||||
| 
 | ||||
| # pylint:disable=R0201,W0613,E0602 | ||||
| 
 | ||||
|  | @ -46,12 +47,14 @@ class CustomResponseHeaderResource: | |||
| 
 | ||||
| 
 | ||||
| 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()) | ||||
|  |  | |||
|  | @ -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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|         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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|                 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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|                 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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|                 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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|         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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|         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 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): | |||
|         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 @@ class TestFalconInstrumentationHooks(TestFalconBase): | |||
|         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 @@ class TestCustomRequestResponseHeaders(TestFalconBase): | |||
|             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 @@ class TestCustomRequestResponseHeaders(TestFalconBase): | |||
|         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): | ||||
|  |  | |||
|  | @ -53,7 +53,7 @@ libraries = { | |||
|         "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": { | ||||
|  |  | |||
							
								
								
									
										13
									
								
								tox.ini
								
								
								
								
							
							
						
						
									
										13
									
								
								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] | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue