diff --git a/src/instana/instrumentation/pyramid.py b/src/instana/instrumentation/pyramid.py index 6faed9db..a16f4d88 100644 --- a/src/instana/instrumentation/pyramid.py +++ b/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") diff --git a/tests/apps/pyramid/pyramid_app/app.py b/tests/apps/pyramid/pyramid_app/app.py index 867b2e7c..88763bbc 100644 --- a/tests/apps/pyramid/pyramid_app/app.py +++ b/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) diff --git a/tests/frameworks/test_pyramid.py b/tests/frameworks/test_pyramid.py index f2a8a640..72f934c5 100644 --- a/tests/frameworks/test_pyramid.py +++ b/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