Skip to content

refactor(django): correct debug unpatch target#17233

Draft
KowalskiThomas wants to merge 1 commit intomainfrom
dd/kowalski/fix/django-unpatch-views-debug
Draft

refactor(django): correct debug unpatch target#17233
KowalskiThomas wants to merge 1 commit intomainfrom
dd/kowalski/fix/django-unpatch-views-debug

Conversation

@KowalskiThomas
Copy link
Copy Markdown
Contributor

@KowalskiThomas KowalskiThomas commented Mar 31, 2026

Description

This fixes a Django integration unpatch bug where _unpatch referenced django.view.debug (non-existent), which can raise AttributeError during unpatch and prevent proper cleanup.

  • Updated ddtrace/contrib/internal/django/patch.py to unwrap django.views.debug.technical_500_response.
  • Added a regression test in tests/contrib/django/test_django_patch.py to verify _unpatch targets django.views.debug.technical_500_response.

@cit-pr-commenter-54b7da
Copy link
Copy Markdown

Codeowners resolved as

ddtrace/contrib/internal/django/patch.py                                @DataDog/apm-core-python @DataDog/apm-idm-python
tests/contrib/django/test_django_patch.py                               @DataDog/apm-core-python @DataDog/apm-idm-python

Co-authored-by: KowalskiThomas <14239160+KowalskiThomas@users.noreply.github.com>
@KowalskiThomas KowalskiThomas force-pushed the dd/kowalski/fix/django-unpatch-views-debug branch from a24433e to 2a0afbc Compare April 1, 2026 06:43
@KowalskiThomas KowalskiThomas added the changelog/no-changelog A changelog entry is not required for this PR. label Apr 1, 2026
@KowalskiThomas
Copy link
Copy Markdown
Contributor Author

@codex review

@KowalskiThomas KowalskiThomas changed the title fix(django): correct debug unpatch target refactor(django): correct debug unpatch target Apr 1, 2026
@KowalskiThomas KowalskiThomas added changelog/no-changelog A changelog entry is not required for this PR. and removed changelog/no-changelog A changelog entry is not required for this PR. labels Apr 1, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a0afbc0df

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

trace_utils.unwrap(django.contrib.auth.login, "login")
trace_utils.unwrap(django.contrib.auth.authenticate, "authenticate")
trace_utils.unwrap(django.view.debug.technical_500_response, "technical_500_response")
trace_utils.unwrap(django.views.debug.technical_500_response, "technical_500_response")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Unwrap the debug module instead of the wrapped function

trace_utils.unwrap expects a container object and an attribute name (see ddtrace/internal/utils/wrappers.py), but this call passes django.views.debug.technical_500_response itself as the container. In real unpatch flows this leads to getattr(function, "technical_500_response") and can still raise AttributeError, so unpatch cleanup remains broken despite the viewviews fix. The first argument should be the module (django.views.debug), not the function object.

Useful? React with 👍 / 👎.

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

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant