Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added recording of exceptions in Pyramid #2622

Merged
merged 5 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -180,6 +181,7 @@ def trace_tween(request):

response = None
status = None
recordable_exc = None

try:
response = handler(request)
Expand All @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
xrmx marked this conversation as resolved.
Show resolved Hide resolved
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"
Expand Down
Loading