Add falcon version 1.4.1 support to opentelemetry-instrumentation-falcon

Author: rjduffner

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.10.0-0.29b0...HEAD)
 
+- `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/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py

@@ -91,6 +91,7 @@ def response_hook(span, req, resp):
 """
 
 from logging import getLogger
+from packaging import version as package_version
 from sys import exc_info
 from typing import Collection
 
@@ -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,3 +1,4 @@
+from packaging import version as package_version
 import falcon
 
 # pylint:disable=R0201,W0613,E0602
@@ -34,12 +35,17 @@ def on_get(self, req, 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

@@ -81,7 +81,7 @@ def test_head(self):
 
     def _test_method(self, method):
         self.client().simulate_request(
-            method=method, path="/hello", remote_addr="127.0.0.1"
+            method=method, path="/hello"
         )
         spans = self.memory_exporter.get_finished_spans()
         self.assertEqual(len(spans), 1)
@@ -111,7 +111,7 @@ def _test_method(self, method):
         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]
@@ -135,7 +135,7 @@ def test_404(self):
 
     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()
@@ -187,7 +187,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")
@@ -197,7 +197,7 @@ 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]
         )
@@ -211,7 +211,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)
@@ -257,7 +257,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")