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

Using defined span creation on static class method cause TypeError #2525

Closed
GeorgeS1995 opened this issue Nov 23, 2023 · 3 comments · Fixed by #2559
Closed

Using defined span creation on static class method cause TypeError #2525

GeorgeS1995 opened this issue Nov 23, 2023 · 3 comments · Fixed by #2559
Assignees
Labels
Type: Bug Something isn't working

Comments

@GeorgeS1995
Copy link

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.35.0

Steps to Reproduce

Deps:
fastapi==0.87.0

Steps to reproduce

  • Create class with static method and signature like that:
class SentryTracedService:
    @staticmethod
    def f4(a = None):
        return a
  • Create any route that call this method with argument
@app.get("/")
def r():
    sts = SentryTracedService()
    sts.f4(1)
    return {"Hello": "World"}
  • Add to sentry init this method
    sentry_sdk.init(
        ...
        functions_to_trace=[{"qualified_name": "path_to_cls.SentryTracedService.f4"}],
    )
  • Call route with this static method

Expected Result

The route runs back properly without any errors caused by the static method. Additionally, tracing of this method is added to the performance tab.

Actual Result

Got a error:
TypeError: SentryTracedService.f4() takes from 0 to 1 positional arguments but 2 were given

@sentrivana
Copy link
Contributor

Hey @GeorgeS1995, thanks for the report and clear steps to repro! That will make it easier for us to debug.

@szokeasaurusrex szokeasaurusrex self-assigned this Nov 29, 2023
@szokeasaurusrex
Copy link
Member

Hey @GeorgeS1995, I am currently trying to figure out how we can fix this bug. While investigating the bug, I discovered a workaround you can use until a fix is released.

Specifically, you will need to use the sentry_sdk.trace decorator to mark the static method for tracing, instead of using functions_to_trace in the sentry_init method. When using the sentry_sdk.trace decorator, you will also need to ensure it is placed after the staticmethod decorator. So, your example would need to be modified like so:

import sentry_sdk

class SentryTracedService:
    @staticmethod
    @sentry_sdk.trace
    def f4(a = None):
        return a

...

sentry_sdk.init(
    ... # Remove SentryTracedService.f4 entry from functions_to_trace
)

Hope this helps!

@szokeasaurusrex
Copy link
Member

A similar error also occurs with class methods; the same workaround suggested above for static methods also applies to class methods. The sentry_sdk.trace decorator must go after the classmethod decorator, like so:

import sentry_sdk

class SentryTracedService:
    @classmethod
    @sentry_sdk.trace
    def my_classmethod(cls, ...):
        ...

szokeasaurusrex added a commit that referenced this issue Jan 5, 2024
Fixes TypeError that occurred when static or class methods, which were passed in the `functions_to_trace` argument when initializing the SDK, were called on an instance. 

Fixes GH-2525

---------

Co-authored-by: Ivana Kellyerova <ivana.kellyerova@sentry.io>
szokeasaurusrex added a commit to getsentry/sentry-docs that referenced this issue Jan 9, 2024
… decorators (#8831)

This PR updates the Sentry Python SDK tracing docs to inform users that when tracing a static or class method using the @sentry_sdk.trace decorator, the decorator must be placed after the @staticmethod or @classmethod decorator.

Failing to adhere to this ordering will break the function (specifically when it is called on an instance rather than on the class), due to how static and class methods work in the Python language (see getsentry/sentry-python#2525). Unfortunately, because of how static and class methods work in Python it is probably impossible to change the @sentry_sdk.trace decorator to allow a flipped ordering, so alerting users in the documentation is likely the best solution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants