Skip to content

Commit 9b5fdd0

Browse files
123liuzimingCopilotemdnetoMikeGoldsmithxrmx
authored
flask: Clear context at the end of the request in non-streaming scenarios. (#4341)
* Initial plan * Clean up _ENVIRON_ACTIVATION_KEY and _ENVIRON_TOKEN in _teardown_request to prevent duplicate execution Add a finally block in _teardown_request to pop _ENVIRON_ACTIVATION_KEY and _ENVIRON_TOKEN from flask.request.environ after teardown completes. This prevents issues when _teardown_request is called multiple times, as the second call will see activation as None and return early. Co-authored-by: 123liuziming <32130965+123liuziming@users.noreply.github.com> * Improve test to explicitly verify duplicate teardown calls don't cause errors Co-authored-by: 123liuziming <32130965+123liuziming@users.noreply.github.com> * fix lint Change-Id: Iea7711d86dcd6f4da7a3af92cb68db2fe3db345f Co-developed-by: Cursor <noreply@cursor.com> * polish CHANGELOG Change-Id: Ie89cac244ade8d04768503112db8673f91d81756 Co-developed-by: Cursor <noreply@cursor.com> * Refactor request teardown to avoid duplicate cleanup Remove redundant environment variable cleanup in finally block. * Update test_flask_compatibility.py * Uncomment FlaskInstrumentor import in test file * Initial plan * Fix ruff format errors in docker test files Co-authored-by: 123liuziming <32130965+123liuziming@users.noreply.github.com> Agent-Logs-Url: https://github.com/123liuziming/opentelemetry-python-contrib/sessions/9165702c-5648-474d-9291-dfa57008b0b6 * Initial plan * Revert docker-test formatting changes that undo merged #4339 cleanup Agent-Logs-Url: https://github.com/123liuziming/opentelemetry-python-contrib/sessions/25f73137-1606-4050-b879-ffc01d7ab803 Co-authored-by: 123liuziming <32130965+123liuziming@users.noreply.github.com> * Update CHANGELOG.md --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com> Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
1 parent 0158026 commit 9b5fdd0

3 files changed

Lines changed: 62 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2424
([#4461](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4461))
2525
- `opentelemetry-instrumentation-dbapi` Use `ObjectProxy` instead of `BaseObjectProxy` for `TracedCursorProxy` to restore iterability with wrapt 2.x
2626
([#4427](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4427))
27+
- `opentelemetry-instrumentation-flask`: Clean up environ keys in `_teardown_request` to prevent duplicate execution
28+
([#4341](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4341))
2729

2830
### Breaking changes
2931

instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,8 @@ def _teardown_request(exc):
636636

637637
if token:
638638
context.detach(token)
639+
flask.request.environ.pop(_ENVIRON_ACTIVATION_KEY, None)
640+
flask.request.environ.pop(_ENVIRON_TOKEN, None)
639641

640642
except (RuntimeError, AttributeError, ValueError) as teardown_exc:
641643
# Log the error but don't raise it to avoid breaking the request handling

instrumentation/opentelemetry-instrumentation-flask/tests/test_flask_compatibility.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626

2727
from opentelemetry import trace
2828
from opentelemetry.instrumentation.flask import (
29+
_ENVIRON_ACTIVATION_KEY,
30+
_ENVIRON_TOKEN,
2931
FlaskInstrumentor,
3032
_request_ctx_ref,
3133
)
@@ -354,3 +356,59 @@ def generate():
354356
# This ensures OpenTelemetry contexts are properly cleaned up for streaming responses
355357
# following Logfire's recommendations (see open-telemetry/opentelemetry-python#2606)
356358
# If we reach this point, the Flask 3.1+ streaming context cleanup is working
359+
360+
def test_duplicate_teardown_request_does_not_cause_errors(self):
361+
"""Test that _teardown_request can be called multiple times without errors.
362+
363+
Flask may call teardown_request multiple times in some scenarios.
364+
After the first call, _ENVIRON_ACTIVATION_KEY and _ENVIRON_TOKEN
365+
should be cleaned up so subsequent calls are no-ops.
366+
"""
367+
app = flask.Flask(__name__)
368+
369+
# Register the check handler BEFORE instrumenting so it runs AFTER
370+
# the instrumentation's teardown (Flask uses LIFO order).
371+
cleaned_up = {}
372+
call_count = {"teardown_calls": 0}
373+
374+
@app.teardown_request
375+
def check_cleanup(exc):
376+
cleaned_up["activation_present"] = (
377+
_ENVIRON_ACTIVATION_KEY in flask.request.environ
378+
)
379+
cleaned_up["token_present"] = (
380+
_ENVIRON_TOKEN in flask.request.environ
381+
)
382+
383+
FlaskInstrumentor().instrument_app(app)
384+
385+
# Wrap the instrumentation's teardown to count calls and invoke it
386+
# a second time to simulate duplicate teardown.
387+
instrumentation_teardown = app.teardown_request_funcs[None][-1]
388+
389+
def counting_teardown(exc):
390+
call_count["teardown_calls"] += 1
391+
instrumentation_teardown(exc)
392+
# Call it again to simulate duplicate teardown - should not raise
393+
instrumentation_teardown(exc)
394+
395+
app.teardown_request_funcs[None][-1] = counting_teardown
396+
397+
@app.route("/test")
398+
def test_endpoint():
399+
return "OK"
400+
401+
client = app.test_client()
402+
response = client.get("/test")
403+
self.assertEqual(response.status_code, 200)
404+
# Verify the teardown was actually called
405+
self.assertGreater(call_count["teardown_calls"], 0)
406+
# Verify env keys are cleaned up after teardown
407+
self.assertFalse(
408+
cleaned_up.get("activation_present", True),
409+
"_ENVIRON_ACTIVATION_KEY should be cleaned up after teardown",
410+
)
411+
self.assertFalse(
412+
cleaned_up.get("token_present", True),
413+
"_ENVIRON_TOKEN should be cleaned up after teardown",
414+
)

0 commit comments

Comments
 (0)