From 1a2d7a5bc0f1553ab41c838a280899ed63c7a5e0 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Tue, 15 Mar 2022 09:27:57 -0700 Subject: [PATCH 01/10] pyramid: handle BaseException --- .../opentelemetry/instrumentation/pyramid/callbacks.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index cc424eb0d9..aaebc9feb3 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -13,9 +13,12 @@ # limitations under the License. from logging import getLogger +from xmlrpc.client import ResponseError + from pyramid.events import BeforeTraversal from pyramid.httpexceptions import HTTPException +from pyramid.response import Response from pyramid.settings import asbool from pyramid.tweens import EXCVIEW @@ -148,6 +151,12 @@ def trace_tween(request): # response types response_or_exception = exc raise + except BaseException as exc: + # In the case that a non-HTTPException is bubbled up we + # should catch it and encapsulate it in an HTTPException + response = Response() if response is None else response + response_or_exception = HTTPException(response, exc) + raise finally: span = request.environ.get(_ENVIRON_SPAN_KEY) enabled = request.environ.get(_ENVIRON_ENABLED_KEY) From a427deb531cb0d3739e4919af1b985ba5661bdd1 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Tue, 15 Mar 2022 12:44:24 -0700 Subject: [PATCH 02/10] pyramid: handle BaseException --- .../instrumentation/pyramid/callbacks.py | 35 ++++++++++++------- .../tests/pyramid_base_test.py | 2 ++ .../tests/test_programmatic.py | 20 +++++++++++ 3 files changed, 44 insertions(+), 13 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index aaebc9feb3..284eaee1a1 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -12,12 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +from lib2to3.pytree import Base from logging import getLogger from xmlrpc.client import ResponseError from pyramid.events import BeforeTraversal -from pyramid.httpexceptions import HTTPException +from pyramid.httpexceptions import HTTPException, HTTPInternalServerError from pyramid.response import Response from pyramid.settings import asbool from pyramid.tweens import EXCVIEW @@ -140,22 +141,24 @@ def trace_tween(request): request.environ[_ENVIRON_ENABLED_KEY] = True request.environ[_ENVIRON_STARTTIME_KEY] = _time_ns() + response = None + status = None + headerList = 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 as exc: + except BaseException: # In the case that a non-HTTPException is bubbled up we - # should catch it and encapsulate it in an HTTPException - response = Response() if response is None else response - response_or_exception = HTTPException(response, exc) + # should infer a internal server error and raise + status = "500 InternalServerError" raise finally: span = request.environ.get(_ENVIRON_SPAN_KEY) @@ -167,10 +170,16 @@ def trace_tween(request): "PyramidInstrumentor().instrument_config(config) is called" ) elif enabled: + if response: + if hasattr(response, "status"): + status = response.status + if hasattr(response, "headerList"): + headerList = response.headerList + otel_wsgi.add_response_attributes( span, - response_or_exception.status, - response_or_exception.headerlist, + status, + headerList, ) propagator = get_global_response_propagator() @@ -179,11 +188,11 @@ def trace_tween(request): 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) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/pyramid_base_test.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/pyramid_base_test.py index 70ab268c23..b1c5ad09a2 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/pyramid_base_test.py +++ b/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): diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py index ac67809362..e0b1d5a8ea 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from typing import final from unittest.mock import Mock, patch from pyramid.config import Configurator @@ -167,6 +168,25 @@ 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) as context: + 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}) From 7082e762aabd8dc55b251cdd0b7b5a2d5857c4dd Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Tue, 15 Mar 2022 12:54:35 -0700 Subject: [PATCH 03/10] append changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3449b5e72b..c7525dbefc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ 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-pyramid` Handle non-HTTPException exceptions + ([#1001](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1001)) + ## [1.10.0-0.29b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.10.0-0.29b0) - 2022-03-10 From 90968138683a6c68c748114b159ceebf9daeb68c Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Tue, 15 Mar 2022 13:15:25 -0700 Subject: [PATCH 04/10] cleanup unused imports --- .../src/opentelemetry/instrumentation/pyramid/callbacks.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index 284eaee1a1..1acba27b92 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -12,14 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -from lib2to3.pytree import Base from logging import getLogger -from xmlrpc.client import ResponseError - from pyramid.events import BeforeTraversal -from pyramid.httpexceptions import HTTPException, HTTPInternalServerError -from pyramid.response import Response +from pyramid.httpexceptions import HTTPException from pyramid.settings import asbool from pyramid.tweens import EXCVIEW From 629748336249fb66494ce527119eb491d2782c20 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Tue, 15 Mar 2022 13:17:51 -0700 Subject: [PATCH 05/10] cleanup unused imports --- .../tests/test_programmatic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py index e0b1d5a8ea..76dc917b4b 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import final from unittest.mock import Mock, patch from pyramid.config import Configurator From c94e8b7acae2eeeadaeb8b40f5144c28f0c6f0a7 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Tue, 15 Mar 2022 14:29:37 -0700 Subject: [PATCH 06/10] cleanup whitespace to make linter happy --- .../tests/test_programmatic.py | 1 - 1 file changed, 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py index 76dc917b4b..5a14cd9f34 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py @@ -175,7 +175,6 @@ def test_internal_exception(self): SpanAttributes.HTTP_STATUS_CODE: 500, } ) - with self.assertRaises(NotImplementedError) as context: resp = self.client.get("/hello/900") resp.close() From 10abfbba134126771fe2a6c7a45c8fa8e003b767 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Thu, 17 Mar 2022 10:17:38 -0700 Subject: [PATCH 07/10] fix exception assertion to satisfy linter --- .../tests/test_programmatic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py index 5a14cd9f34..414e6ff28e 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py @@ -175,7 +175,7 @@ def test_internal_exception(self): SpanAttributes.HTTP_STATUS_CODE: 500, } ) - with self.assertRaises(NotImplementedError) as context: + with self.assertRaises(NotImplementedError): resp = self.client.get("/hello/900") resp.close() From 28a0757ca59e0e6a228bdb1fb73c46d9f2ad0a52 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Thu, 31 Mar 2022 12:47:28 -0700 Subject: [PATCH 08/10] more defensive --- .../instrumentation/pyramid/callbacks.py | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index 1acba27b92..437d343b33 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -139,8 +139,7 @@ def trace_tween(request): response = None status = None - headerList = None - + try: response = handler(request) except HTTPException as exc: @@ -166,21 +165,19 @@ def trace_tween(request): "PyramidInstrumentor().instrument_config(config) is called" ) elif enabled: - if response: - if hasattr(response, "status"): - status = response.status - if hasattr(response, "headerList"): - headerList = response.headerList - - otel_wsgi.add_response_attributes( - span, - status, - 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: - propagator.inject(response.headers) + if hasattr(response, "headers"): + propagator = get_global_response_propagator() + if propagator: + propagator.inject(response.headers) activation = request.environ.get(_ENVIRON_ACTIVATION_KEY) From 0c8284192b5c675b0b7396b256189a28eeaff70e Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Thu, 31 Mar 2022 12:59:13 -0700 Subject: [PATCH 09/10] whitespace --- .../src/opentelemetry/instrumentation/pyramid/callbacks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index 437d343b33..22f624321c 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -139,7 +139,7 @@ def trace_tween(request): response = None status = None - + try: response = handler(request) except HTTPException as exc: From 74e15c5bf1cfc7684c0122f8ead26ce65d61cc78 Mon Sep 17 00:00:00 2001 From: Greg Buehler Date: Thu, 31 Mar 2022 16:45:33 -0700 Subject: [PATCH 10/10] condense conditional --- .../src/opentelemetry/instrumentation/pyramid/callbacks.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py index 22f624321c..4dcdd96312 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/callbacks.py @@ -174,10 +174,9 @@ def trace_tween(request): getattr(response, "headerList", None), ) - if hasattr(response, "headers"): - propagator = get_global_response_propagator() - if propagator: - propagator.inject(response.headers) + propagator = get_global_response_propagator() + if propagator and hasattr(response, "headers"): + propagator.inject(response.headers) activation = request.environ.get(_ENVIRON_ACTIVATION_KEY)