Add falcon version 1.4.1 support to opentelemetry-instrumentation-falcon

CHANGELOG.md

@@ -23,6 +23,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`

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 |

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

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",)

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())

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):

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": {

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]