fix: Response class doesn't have exception attribute. Fixes: #819

Signed-off-by: Cagri Yonca <[email protected]>

Author: CagriYonca

src/instana/instrumentation/pyramid.py

@@ -1,28 +1,28 @@
 # (c) Copyright IBM Corp. 2021
 # (c) Copyright Instana Inc. 2020
 
+
 try:
+    from typing import TYPE_CHECKING, Any, Callable, Dict, Optional, Tuple, Union
+
+    import wrapt
+    from opentelemetry.semconv.trace import SpanAttributes
+    from pyramid.config import Configurator
     from pyramid.httpexceptions import HTTPException
     from pyramid.path import caller_package
     from pyramid.settings import aslist
     from pyramid.tweens import EXCVIEW
-    from pyramid.config import Configurator
-    from typing import TYPE_CHECKING, Dict, Any, Callable, Tuple
-    import wrapt
-
-    from opentelemetry.semconv.trace import SpanAttributes
-    from opentelemetry.trace import SpanKind
 
     from instana.log import logger
-    from instana.singletons import tracer, agent
+    from instana.propagators.format import Format
+    from instana.singletons import agent, tracer
     from instana.util.secrets import strip_secrets_from_query
     from instana.util.traceutils import extract_custom_headers
-    from instana.propagators.format import Format
 
     if TYPE_CHECKING:
+        from pyramid.registry import Registry
         from pyramid.request import Request
         from pyramid.response import Response
-        from pyramid.registry import Registry
 
     class InstanaTweenFactory(object):
         """A factory that provides Instana instrumentation tween for Pyramid apps"""
@@ -32,11 +32,11 @@ def __init__(
         ) -> None:
             self.handler = handler
 
-        def __call__(self, request: "Request") -> "Response":
+        def __call__(self, request: "Request") -> Optional["Response"]:
             ctx = tracer.extract(Format.HTTP_HEADERS, dict(request.headers))
 
             with tracer.start_as_current_span("wsgi", span_context=ctx) as span:
-                span.set_attribute("http.host", request.host)
+                span.set_attribute(SpanAttributes.HTTP_HOST, request.host)
                 span.set_attribute(SpanAttributes.HTTP_METHOD, request.method)
                 span.set_attribute(SpanAttributes.HTTP_URL, request.path)
 
@@ -57,9 +57,7 @@ def __call__(self, request: "Request") -> "Response":
                         span.set_attribute(
                             "http.path_tpl", request.matched_route.pattern
                         )
-
                     extract_custom_headers(span, response.headers)
-
                     tracer.inject(span.context, Format.HTTP_HEADERS, response.headers)
                 except HTTPException as e:
                     response = e
@@ -69,7 +67,6 @@ def __call__(self, request: "Request") -> "Response":
                 except BaseException as e:
                     span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, 500)
                     span.record_exception(e)
-
                     logger.debug(
                         "Pyramid InstanaTweenFactory BaseException: ", exc_info=True
                     )
@@ -78,16 +75,18 @@ def __call__(self, request: "Request") -> "Response":
                         span.set_attribute(
                             SpanAttributes.HTTP_STATUS_CODE, response.status_int
                         )
-
-                        if 500 <= response.status_int:
-                            if response.exception:
-                                span.record_exception(response.exception)
-                            span.assure_errored()
-
+                        if response.status_code >= 500:
+                            handle_exception(span, response)
                 return response
 
     INSTANA_TWEEN = __name__ + ".InstanaTweenFactory"
 
+    def handle_exception(span, response: Union["Response", HTTPException]) -> None:
+        if isinstance(response, HTTPException):
+            span.record_exception(response.exception)
+        else:
+            span.record_exception(response.body)
+
     # implicit tween ordering
     def includeme(config: Configurator) -> None:
         logger.debug("Instrumenting pyramid")

tests/apps/pyramid/pyramid_app/app.py

@@ -1,12 +1,12 @@
 # (c) Copyright IBM Corp. 2021
 # (c) Copyright Instana Inc. 2020
 
-from wsgiref.simple_server import make_server
-from pyramid.config import Configurator
 import logging
+from wsgiref.simple_server import make_server
 
-from pyramid.response import Response
 import pyramid.httpexceptions as exc
+from pyramid.config import Configurator
+from pyramid.response import Response
 
 from tests.helpers import testenv
 
@@ -25,6 +25,10 @@ def please_fail(request):
     raise exc.HTTPInternalServerError("internal error")
 
 
+def fail_with_http_exception(request):
+    raise exc.HTTPException("bad request")
+
+
 def tableflip(request):
     raise BaseException("fake exception")
 
@@ -39,6 +43,10 @@ def hello_user(request):
     return Response(f"Hello {user}!")
 
 
+def return_error_response(request):
+    return Response("Error", status=500)
+
+
 app = None
 settings = {
     "pyramid.tweens": "tests.apps.pyramid.pyramid_utils.tweens.timing_tween_factory",
@@ -48,12 +56,16 @@ def hello_user(request):
     config.add_view(hello_world, route_name="hello")
     config.add_route("fail", "/500")
     config.add_view(please_fail, route_name="fail")
+    config.add_route("fail_with_http_exception", "/fail_with_http_exception")
+    config.add_view(fail_with_http_exception, route_name="fail_with_http_exception")
     config.add_route("crash", "/exception")
     config.add_view(tableflip, route_name="crash")
     config.add_route("response_headers", "/response_headers")
     config.add_view(response_headers, route_name="response_headers")
     config.add_route("hello_user", "/hello_user/{user}")
     config.add_view(hello_user, route_name="hello_user")
+    config.add_route(name="return_error_response", pattern="/return_error_response")
+    config.add_view(return_error_response, route_name="return_error_response")
     app = config.make_wsgi_app()
 
 pyramid_server = make_server("127.0.0.1", testenv["pyramid_port"], app)

tests/frameworks/test_pyramid.py

@@ -1,15 +1,16 @@
 # (c) Copyright IBM Corp. 2021
 # (c) Copyright Instana Inc. 2020
 
+from typing import Generator
+
 import pytest
 import urllib3
-from typing import Generator
 
+import tests.apps.pyramid.pyramid_app  # noqa: F401
+from instana.singletons import agent, tracer
+from instana.span.span import get_current_span
 from instana.util.ids import hex_id
-import tests.apps.pyramid.pyramid_app
 from tests.helpers import testenv
-from instana.singletons import tracer, agent
-from instana.span.span import get_current_span
 
 
 class TestPyramid:
@@ -77,7 +78,9 @@ def test_get_request(self) -> None:
 
         # wsgi
         assert pyramid_span.n == "wsgi"
-        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
         assert pyramid_span.data["http"]["url"] == "/"
         assert pyramid_span.data["http"]["method"] == "GET"
         assert pyramid_span.data["http"]["status"] == 200
@@ -161,7 +164,9 @@ def test_500(self) -> None:
 
         # wsgi
         assert pyramid_span.n == "wsgi"
-        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
         assert pyramid_span.data["http"]["url"] == "/500"
         assert pyramid_span.data["http"]["method"] == "GET"
         assert pyramid_span.data["http"]["status"] == 500
@@ -178,6 +183,56 @@ def test_500(self) -> None:
         assert type(urllib3_span.stack) is list
         assert len(urllib3_span.stack) > 1
 
+    def test_return_error_response(self) -> None:
+        with tracer.start_as_current_span("test"):
+            response = self.http.request(
+                "GET", testenv["pyramid_server"] + "/return_error_response"
+            )
+
+        spans = self.recorder.queued_spans()
+        assert len(spans) == 3
+
+        pyramid_span = spans[0]
+
+        assert response.status == 500
+
+        assert pyramid_span.n == "wsgi"
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
+        assert pyramid_span.data["http"]["url"] == "/return_error_response"
+        assert pyramid_span.data["http"]["method"] == "GET"
+        assert pyramid_span.data["http"]["status"] == 500
+        assert pyramid_span.data["http"]["error"] == "b'Error'"
+        assert pyramid_span.data["http"]["path_tpl"] == "/return_error_response"
+
+        assert pyramid_span.ec == 1
+
+    def test_fail_with_http_exception(self) -> None:
+        with tracer.start_as_current_span("test"):
+            response = self.http.request(
+                "GET", testenv["pyramid_server"] + "/fail_with_http_exception"
+            )
+
+        spans = self.recorder.queued_spans()
+        assert len(spans) == 3
+
+        pyramid_span = spans[0]
+
+        assert response.status == 520
+
+        assert pyramid_span.n == "wsgi"
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
+        assert pyramid_span.data["http"]["url"] == "/fail_with_http_exception"
+        assert pyramid_span.data["http"]["method"] == "GET"
+        assert pyramid_span.data["http"]["status"] == 520
+        assert pyramid_span.data["http"]["error"] == "bad request"
+        assert pyramid_span.data["http"]["path_tpl"] == "/fail_with_http_exception"
+
+        assert pyramid_span.ec == 1
+
     def test_exception(self) -> None:
         with tracer.start_as_current_span("test"):
             response = self.http.request(
@@ -211,7 +266,9 @@ def test_exception(self) -> None:
 
         # wsgi
         assert pyramid_span.n == "wsgi"
-        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
         assert pyramid_span.data["http"]["url"] == "/exception"
         assert pyramid_span.data["http"]["method"] == "GET"
         assert pyramid_span.data["http"]["status"] == 500
@@ -270,7 +327,9 @@ def test_response_header_capture(self) -> None:
 
         # wsgi
         assert pyramid_span.n == "wsgi"
-        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
         assert pyramid_span.data["http"]["url"] == "/response_headers"
         assert pyramid_span.data["http"]["method"] == "GET"
         assert pyramid_span.data["http"]["status"] == 200
@@ -341,7 +400,9 @@ def test_request_header_capture(self) -> None:
 
         # wsgi
         assert pyramid_span.n == "wsgi"
-        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
         assert pyramid_span.data["http"]["url"] == "/"
         assert pyramid_span.data["http"]["method"] == "GET"
         assert pyramid_span.data["http"]["status"] == 200
@@ -419,7 +480,9 @@ def test_scrub_secret_path_template(self) -> None:
 
         # wsgi
         assert pyramid_span.n == "wsgi"
-        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(testenv["pyramid_port"])
+        assert pyramid_span.data["http"]["host"] == "127.0.0.1:" + str(
+            testenv["pyramid_port"]
+        )
         assert pyramid_span.data["http"]["url"] == "/hello_user/oswald"
         assert pyramid_span.data["http"]["method"] == "GET"
         assert pyramid_span.data["http"]["status"] == 200