Add falcon version 1.4.1 support to opentelemetry-instrumentation-falcon

Author: rjduffner

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

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

@@ -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

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

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

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,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]