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

[opentelemetry-instrumentation-django] Handle exceptions from request/response hooks #2153

Merged
merged 10 commits into from
Jul 2, 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
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- `opentelemetry-instrumentation-django` Handle exceptions from request/response hooks
([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153))
- `opentelemetry-instrumentation-asyncio` instrumented `asyncio.wait_for` properly raises `asyncio.TimeoutError` as expected
([#2637](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2637))
- `opentelemetry-instrumentation-aws-lambda` Bugfix: AWS Lambda event source key incorrect for SNS in instrumentation library.
Expand Down Expand Up @@ -151,7 +153,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2136](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2136))
- `opentelemetry-resource-detector-azure` Suppress instrumentation for `urllib` call
([#2178](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2178))
- AwsLambdaInstrumentor handles and re-raises function exception ([#2245](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2245))
- AwsLambdaInstrumentor handles and re-raises function exception
([#2245](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2245))

### Added

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def _get_span_name(request):
return request.method

# pylint: disable=too-many-locals
# pylint: disable=too-many-branches
def process_request(self, request):
# request.META is a dictionary containing all available HTTP headers
# Read more about request.META here:
Expand Down Expand Up @@ -286,9 +287,14 @@ def process_request(self, request):
request.META[self._environ_token] = token

if _DjangoMiddleware._otel_request_hook:
_DjangoMiddleware._otel_request_hook( # pylint: disable=not-callable
span, request
)
try:
_DjangoMiddleware._otel_request_hook( # pylint: disable=not-callable
span, request
)
except Exception: # pylint: disable=broad-exception-caught
# Raising an exception here would leak the request span since process_response
# would not be called. Log the exception instead.
_logger.exception("Exception raised by request_hook")

# pylint: disable=unused-argument
def process_view(self, request, view_func, *args, **kwargs):
Expand Down Expand Up @@ -385,10 +391,14 @@ def process_response(self, request, response):

# record any exceptions raised while processing the request
exception = request.META.pop(self._environ_exception_key, None)

if _DjangoMiddleware._otel_response_hook:
_DjangoMiddleware._otel_response_hook( # pylint: disable=not-callable
span, request, response
)
try:
_DjangoMiddleware._otel_response_hook( # pylint: disable=not-callable
span, request, response
)
except Exception: # pylint: disable=broad-exception-caught
_logger.exception("Exception raised by response_hook")

if exception:
activation.__exit__(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,32 @@ def response_hook(span, request, response):
self.assertIsInstance(response_hook_args[2], HttpResponse)
self.assertEqual(response_hook_args[2], response)

def test_request_hook_exception(self):
def request_hook(span, request):
# pylint: disable=broad-exception-raised
raise Exception("request hook exception")

_DjangoMiddleware._otel_request_hook = request_hook
Client().get("/span_name/1234/")
_DjangoMiddleware._otel_request_hook = None

# ensure that span ended
finished_spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(finished_spans), 1)

def test_response_hook_exception(self):
def response_hook(span, request, response):
# pylint: disable=broad-exception-raised
raise Exception("response hook exception")

_DjangoMiddleware._otel_response_hook = response_hook
Client().get("/span_name/1234/")
_DjangoMiddleware._otel_response_hook = None

# ensure that span ended
finished_spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(finished_spans), 1)

def test_trace_parent(self):
id_generator = RandomIdGenerator()
trace_id = format_trace_id(id_generator.generate_trace_id())
Expand Down