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

feat(integrations): add django signals_denylist to filter signals that are attached to by signals_span #2758

Merged
merged 4 commits into from
Apr 9, 2024

Conversation

lieryan
Copy link
Contributor

@lieryan lieryan commented Feb 23, 2024

This PR adds a way to exclude sentry-sdk from attaching to certain Django signals. This can be used to work around performance issues when sentry attaches to performance sensitive signals like pre_init and post_init.

You pass a list of Django signals that you don't want Sentry to attach to:

import django.db.models.signals
import sentry_sdk

sentry_sdk.init(
    ...
    integrations=[
        DjangoIntegration(
            ...
            signals_denylist=[
                django.db.models.signals.pre_init, 
                django.db.models.signals.post_init,
            ],
        ),
    ],
)

This potentially fixes #1826 or #1739, and jazzband/django-model-utils#556.


General Notes

Thank you for contributing to sentry-python!

Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.

For maintainers

Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.

Before running sensitive test suites, please carefully check the PR. Then, apply the Trigger: tests using secrets label. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.

@wsantos
Copy link

wsantos commented Mar 26, 2024

Loved this, I have some init known signals that I'd like to disable mainly django-imagekit but not disable it all.

@sentrivana
Copy link
Contributor

Hey @lieryan, thanks a lot for the PR and sorry for the wait! I'm looking at it now, will leave a review soon.

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Not sure yet what's going on with the cache tests failing, fighting with that now, but as soon as I fix that I'll merge this. Thank you for contributing!

@sentrivana sentrivana added the Trigger: tests using secrets PR code is safe; run CI label Apr 9, 2024
@sentrivana sentrivana merged commit 11e1f9a into getsentry:master Apr 9, 2024
124 of 127 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Trigger: tests using secrets PR code is safe; run CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible performance regression related to django signals instrumentation
3 participants