Added recording of exceptions in Pyramid (open-telemetry#2622)

Author: rbagd

Diff for: CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `opentelemetry-instrumentation-pyramid` Record exceptions raised when serving a request
+  ([#2622](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2622))
 - `opentelemetry-sdk-extension-aws` Add AwsXrayLambdaPropagator
   ([#2573](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2573))
 

Diff for: instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py

@@ -31,6 +31,7 @@
 from opentelemetry.metrics import get_meter
 from opentelemetry.semconv.metrics import MetricInstruments
 from opentelemetry.semconv.trace import SpanAttributes
+from opentelemetry.trace.status import Status, StatusCode
 from opentelemetry.util.http import get_excluded_urls
 
 TWEEN_NAME = "opentelemetry.instrumentation.pyramid.trace_tween_factory"
@@ -180,6 +181,7 @@ def trace_tween(request):
 
         response = None
         status = None
+        recordable_exc = None
 
         try:
             response = handler(request)
@@ -190,11 +192,14 @@ def trace_tween(request):
             # As described in docs, Pyramid exceptions are all valid
             # response types
             response = exc
+            if isinstance(exc, HTTPServerError):
+                recordable_exc = exc
             raise
-        except BaseException:
+        except BaseException as exc:
             # In the case that a non-HTTPException is bubbled up we
             # should infer a internal server error and raise
             status = "500 InternalServerError"
+            recordable_exc = exc
             raise
         finally:
             duration = max(round((default_timer() - start) * 1000), 0)
@@ -222,6 +227,12 @@ def trace_tween(request):
                         getattr(response, "headerlist", None),
                     )
 
+                    if recordable_exc is not None:
+                        span.set_status(
+                            Status(StatusCode.ERROR, str(recordable_exc))
+                        )
+                        span.record_exception(recordable_exc)
+
                 if span.is_recording() and span.kind == trace.SpanKind.SERVER:
                     custom_attributes = (
                         otel_wsgi.collect_custom_response_headers_attributes(

Diff for: instrumentation/opentelemetry-instrumentation-pyramid/tests/pyramid_base_test.py

@@ -35,7 +35,7 @@ def _hello_endpoint(request):
         if helloid == 204:
             raise exc.HTTPNoContent()
         if helloid == 900:
-            raise NotImplementedError()
+            raise NotImplementedError("error message")
         return Response("Hello: " + str(helloid))
 
     @staticmethod

Diff for: instrumentation/opentelemetry-instrumentation-pyramid/tests/test_automatic.py

@@ -121,6 +121,7 @@ def test_redirect_response_is_not_an_error(self):
         span_list = self.memory_exporter.get_finished_spans()
         self.assertEqual(len(span_list), 1)
         self.assertEqual(span_list[0].status.status_code, StatusCode.UNSET)
+        self.assertEqual(len(span_list[0].events), 0)
 
         PyramidInstrumentor().uninstrument()
 

Diff for: instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py

@@ -23,6 +23,7 @@
     set_global_response_propagator,
 )
 from opentelemetry.instrumentation.pyramid import PyramidInstrumentor
+from opentelemetry.semconv.attributes import exception_attributes
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.test.wsgitestutil import WsgiTestBase
 from opentelemetry.util.http import get_excluded_urls
@@ -149,6 +150,7 @@ def test_404(self):
         self.assertEqual(span_list[0].name, "POST /bye")
         self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER)
         self.assertEqual(span_list[0].attributes, expected_attrs)
+        self.assertEqual(len(span_list[0].events), 0)
 
     def test_internal_error(self):
         expected_attrs = expected_attributes(
@@ -166,6 +168,18 @@ def test_internal_error(self):
         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)
+        self.assertEqual(
+            span_list[0].status.status_code, trace.StatusCode.ERROR
+        )
+        self.assertIn(
+            "HTTPInternalServerError", span_list[0].status.description
+        )
+        self.assertEqual(
+            span_list[0]
+            .events[0]
+            .attributes[exception_attributes.EXCEPTION_TYPE],
+            "pyramid.httpexceptions.HTTPInternalServerError",
+        )
 
     def test_internal_exception(self):
         expected_attrs = expected_attributes(
@@ -184,6 +198,21 @@ def test_internal_exception(self):
         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)
+        self.assertEqual(
+            span_list[0].status.status_code, trace.StatusCode.ERROR
+        )
+        self.assertEqual(span_list[0].status.description, "error message")
+
+        expected_error_event_attrs = {
+            exception_attributes.EXCEPTION_TYPE: "NotImplementedError",
+            exception_attributes.EXCEPTION_MESSAGE: "error message",
+        }
+        self.assertEqual(span_list[0].events[0].name, "exception")
+        # Ensure exception event has specific attributes, but allow additional ones
+        self.assertLess(
+            expected_error_event_attrs.items(),
+            dict(span_list[0].events[0].attributes).items(),
+        )
 
     def test_tween_list(self):
         tween_list = "opentelemetry.instrumentation.pyramid.trace_tween_factory\npyramid.tweens.excview_tween_factory"