Pyramid handle non-HTTPException (#1001)

Author: gregbuehler

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([#999])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/999)
 - `opentelemetry-instrumentation-tornado` Fix non-recording span bug
   ([#999])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/999)
+- `opentelemetry-instrumentation-pyramid` Handle non-HTTPException exceptions
+  ([#1001](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1001))
 - `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/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py

@@ -137,16 +137,23 @@ def trace_tween(request):
         request.environ[_ENVIRON_ENABLED_KEY] = True
         request.environ[_ENVIRON_STARTTIME_KEY] = _time_ns()
 
+        response = None
+        status = None
+
         try:
             response = handler(request)
-            response_or_exception = response
         except HTTPException as exc:
             # If the exception is a pyramid HTTPException,
             # that's still valuable information that isn't necessarily
             # a 500. For instance, HTTPFound is a 302.
             # As described in docs, Pyramid exceptions are all valid
             # response types
-            response_or_exception = exc
+            response = exc
+            raise
+        except BaseException:
+            # In the case that a non-HTTPException is bubbled up we
+            # should infer a internal server error and raise
+            status = "500 InternalServerError"
             raise
         finally:
             span = request.environ.get(_ENVIRON_SPAN_KEY)
@@ -158,23 +165,26 @@ def trace_tween(request):
                     "PyramidInstrumentor().instrument_config(config) is called"
                 )
             elif enabled:
-                otel_wsgi.add_response_attributes(
-                    span,
-                    response_or_exception.status,
-                    response_or_exception.headerlist,
-                )
+                status = getattr(response, "status", status)
+
+                if status is not None:
+                    otel_wsgi.add_response_attributes(
+                        span,
+                        status,
+                        getattr(response, "headerList", None),
+                    )
 
                 propagator = get_global_response_propagator()
-                if propagator:
+                if propagator and hasattr(response, "headers"):
                     propagator.inject(response.headers)
 
                 activation = request.environ.get(_ENVIRON_ACTIVATION_KEY)
 
-                if isinstance(response_or_exception, HTTPException):
+                if isinstance(response, HTTPException):
                     activation.__exit__(
-                        type(response_or_exception),
-                        response_or_exception,
-                        getattr(response_or_exception, "__traceback__", None),
+                        type(response),
+                        response,
+                        getattr(response, "__traceback__", None),
                     )
                 else:
                     activation.__exit__(None, None, None)

instrumentation/opentelemetry-instrumentation-pyramid/tests/pyramid_base_test.py

@@ -24,6 +24,8 @@ def _hello_endpoint(request):
         helloid = int(request.matchdict["helloid"])
         if helloid == 500:
             raise exc.HTTPInternalServerError()
+        if helloid == 900:
+            raise NotImplementedError()
         return Response("Hello: " + str(helloid))
 
     def _common_initialization(self, config):

instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py

@@ -167,6 +167,24 @@ def test_internal_error(self):
         self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER)
         self.assertEqual(span_list[0].attributes, expected_attrs)
 
+    def test_internal_exception(self):
+        expected_attrs = expected_attributes(
+            {
+                SpanAttributes.HTTP_TARGET: "/hello/900",
+                SpanAttributes.HTTP_ROUTE: "/hello/{helloid}",
+                SpanAttributes.HTTP_STATUS_CODE: 500,
+            }
+        )
+        with self.assertRaises(NotImplementedError):
+            resp = self.client.get("/hello/900")
+            resp.close()
+
+        span_list = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(span_list), 1)
+        self.assertEqual(span_list[0].name, "/hello/{helloid}")
+        self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER)
+        self.assertEqual(span_list[0].attributes, expected_attrs)
+
     def test_tween_list(self):
         tween_list = "opentelemetry.instrumentation.pyramid.trace_tween_factory\npyramid.tweens.excview_tween_factory"
         config = Configurator(settings={"pyramid.tweens": tween_list})