django: handle cancelled requests more cleanly #4102
Conversation
| max(duration_s, 0), | ||
| duration_attrs_new, | ||
| ) | ||
| self._active_request_counter.add(-1, active_requests_count_attrs) |
There was a problem hiding this comment.
counter decrement, activation.exit, and detach(token)
are now handled in call finally block to ensure cleanup
even when requests are cancelled
| if propagator: | ||
| propagator.inject(response) | ||
|
|
||
| # record any exceptions raised while processing the request |
There was a problem hiding this comment.
exception handling moved to call finally block
| activation = request.META.pop(self._environ_activation_key, None) | ||
| activation = request.META.get(self._environ_activation_key) | ||
| span = request.META.pop(self._environ_span_key, None) | ||
| active_requests_count_attrs = request.META.pop( |
There was a problem hiding this comment.
active_requests_count_attrs is popped in call finally block
| if token: | ||
| request.META[self._environ_token] = token | ||
|
|
||
| if _DjangoMiddleware._otel_request_hook: |
There was a problem hiding this comment.
request_hook is called in call after activation.enter()
| span.set_attribute(key, value) | ||
|
|
||
| activation = use_span(span, end_on_exit=True) | ||
| activation.__enter__() # pylint: disable=E1101 |
There was a problem hiding this comment.
activation.enter() is called in call to ensure
proper cleanup via finally block even on request cancellation
github-actions
bot
assigned
codefromthecrypt,
karthikscale3,
lmolkova,
lzchen and
nirga
|
Lint 0 / instrumentation-google-genai (pull_request) This one broke because of this |
|
Please drop the google genai changes |
|
@xrmx done! |
|
I would appreciate someone testing it on their setup if possible. Seems to be fine on mine |
7 participants
Description
This is a duplicate of: #3848 but without the django version removal.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Only the newly created unit test for now. Can test more later
Does This PR Require a Core Repo Change?
Checklist: