Skip to content

fix(logging): preserve Django request context in async code - #18820

Open
AliAbouelazm wants to merge 3 commits into
googleapis:mainfrom
AliAbouelazm:fix/logging-django-async-context-15413
Open

AliAbouelazm wants to merge 3 commits into
googleapis:mainfrom
AliAbouelazm:fix/logging-django-async-context-15413

Conversation

@AliAbouelazm

Copy link
Copy Markdown
Contributor

Fixes #15413.

Use asgiref.local.Local when available so Django request context is isolated between async tasks and follows sync/async adapters. Keep the threading.local fallback without adding a dependency.

Add regressions for concurrent tasks, both adapter directions, ordinary thread isolation, and the optional-import fallback. Isolate existing request-data tests without clearing the storage object's internals.

Validation:

  • 1,386 unit tests pass on Python 3.12 with both protobuf implementations.
  • Ruff and Flake8 pass. Changed middleware and new tests have 100% line and branch coverage; aggregate coverage does not decrease from the baseline.
  • Real ASGI/WSGI async-view probes pass on Django 5.2 and 6.1.
  • No live Google Cloud resources were used.

AI-assisted implementation and validation.

@AliAbouelazm
AliAbouelazm marked this pull request as ready for review October 22, 2026 21:49
@AliAbouelazm
AliAbouelazm requested a review from a team as a code owner October 22, 2026 21:49

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Django request middleware to use asgiref.local.Local when available, enabling support for both thread-local and async-local contexts, while falling back to threading.local if asgiref is not installed. It also adds comprehensive unit tests for async and thread-local behaviors. Feedback suggests refactoring a thread-local test to group assertions and middleware execution within a single worker function to prevent potential test flakiness from thread reuse assumptions in ThreadPoolExecutor.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

@daniel-sanche daniel-sanche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, but it looks like there are some failing tests to resolve

from asgiref.local import Local
except ImportError:
# Django is optional, so do not require asgiref for other integrations.
from threading import local as Local

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it;s possible to use contextvars directly for this instead, that would be ideal. We try to avoid external dependencies where possible

But that said, this file is specifically for django middleware, and IIUC, django requires asgiref. And you handled the fall-back case. So I think this is fine as-is

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Async Support for the threading Local Request store

2 participants