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

SentryWrappingMiddleware does not expose details of wrapped middleware #1145

Closed
MattFisher opened this issue Jul 7, 2021 · 6 comments
Closed
Labels
Help wanted Extra attention is needed

Comments

@MattFisher
Copy link
Contributor

Environment

How do you use Sentry?
Sentry SaaS (sentry.io)

Which SDK and version?
Python sentry-sdk==1.1.0

Steps to Reproduce

Use the python sentry sdk alongside the python NewRelic agent (newrelic-python-agent v6.4.4.161).
Wait for slow transaction traces to appear in NewRelic UI.

Expected Result

The middleware entries in transaction traces should show the names of the middlewares being wrapped.

Actual Result

All middlewares in transaction traces appear listed as sentry_sdk.integrations.django_middleware:_wrap_middleware.<locals>.SentryWrappingMiddleware, as shown in the screenshot below.
Screen Shot 2021-07-07 at 11 29 21 am

I've done some investigation, and the NewRelic agent determines the strings to be displayed
via a callable_name function, that bottoms out in a function called _object_context_py3. This function returns the module name (via object.__module__) and object.__qualname__ attributes of the passed object.

In the sentry_sdk.integrations.django.middleware._wrap_middleware function, the SentryWrappingMiddleware is given the wrapped middleware's __name__ attribute.

	if hasattr(middleware, "__name__"):
        SentryWrappingMiddleware.__name__ = middleware.__name__

In the standard library's functools.wraps(), the wrapping entity is given ('__module__', '__name__', '__qualname__', '__doc__', '__annotations__') of the wrapped object. This implies it should be pretty safe to do the same with SentryWrappingMiddleware, like so:

    if hasattr(middleware, "__name__"):
        SentryWrappingMiddleware.__name__ = middleware.__name__
    if hasattr(middleware, "__module__"):
        SentryWrappingMiddleware.__module__ = middleware.__module__
    if hasattr(middleware, "__qualname__"):
        SentryWrappingMiddleware.__qualname__ = middleware.__qualname__

or potentially

    for attr in ('__name__', '__module__', '__qualname__', '__doc__', '__annotations__'):
        if hasattr(middleware, attr):
            setattr(SentryWrappingMiddleware, attr, getattr(middleware, attr))

After manually making the first code change listed above, the results were what was expected:
Screen Shot 2021-07-07 at 11 28 21 am

@DmytroLitvinov
Copy link

I have faced the same issue 👀

@HazAT
Copy link
Member

HazAT commented Aug 2, 2021

@MattFisher thanks for the awesome issue!
Would you mind making a PR out of this since it seems you solved it yourself?
We are happy to merge it and make it better for everyone, wdyt?

@HazAT HazAT added the Help wanted Extra attention is needed label Aug 2, 2021
MattFisher added a commit to MattFisher/sentry-python that referenced this issue Sep 22, 2021
Include __name__, __module__, __qualname__, __doc__, __annotations__
Addresses getsentry#1145
@github-actions
Copy link

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@DmytroLitvinov
Copy link

Hi @HazAT ,
Could you please review related PR #1202 for that?

@nicon89
Copy link

nicon89 commented Mar 9, 2022

We have same problem. Is there any plan for fixing it?

@sl0thentr0py
Copy link
Member

fixed by #1202, will be in next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants