sentry-docs icon indicating copy to clipboard operation
sentry-docs copied to clipboard

Don't suggest using settings file for Django integration

Open xiongchiamiov opened this issue 3 years ago • 1 comments

Core or SDK?

Platform/SDK

Which part? Which one?

Python SDK in Django

Description

We've been initializing Sentry in the settings files, which is how Sentry recommends it. However, this produces problems with our inheritance structure, because the init is run on import. We've worked around this thus far by defining a wrapper function in base.py and calling it at the end of every settings file. But this is a bit clumsy. And with second-layer imports, we can't override Sentry-related settings to be different than the imported file. Due to these sorts of problems, generally it's advised to avoid putting app initialization code in the settings files.

Example inheritance structure for settings that causes issues:

base.py
│
├─►development.py
├─►staging.py
├  ├─►other_staging.py
├─►production.py

Suggested Solution

In Django 1.7 they added a ready hook for apps to provide a way to do setup code without doing it in your settings file, and so I've created a new sentry app in our project to do this hooking. This is also slightly nicer in that we don't need to remember to call the function when temporarily adding Sentry to dev for debugging purposes. The app is very simple, having only an apps.py like this:

import sentry_sdk
from django.apps import AppConfig
from django.conf import settings
from sentry_sdk.integrations.celery import CeleryIntegration
from sentry_sdk.integrations.django import DjangoIntegration


class SentryConfig(AppConfig):
    name = 'sentry'

    def ready(self):
        if not settings.ENABLE_SENTRY:
            return

        sentry_sdk.init(
            dsn=settings.SENTRY_DSN,
            environment=settings.SENTRY_ENVIRONMENT,
            integrations=[DjangoIntegration(), CeleryIntegration()],
            send_default_pii=True,
            traces_sample_rate=settings.SENTRY_TRACE_SAMPLE_RATE)

and then this app is added to the top of INSTALLED_APPS.

xiongchiamiov avatar Jul 22 '22 19:07 xiongchiamiov

Routing to @getsentry/team-web-sdk-backend for triage. ⏲️

getsentry-release avatar Jul 22 '22 21:07 getsentry-release

Since we have only received one complaint about the currently recommended setup, and since changing the instructions for setting up a commonly used integration could potentially cause issues for even larger numbers of users, we will keep the default instructions the same.

However, we agree that it would be a good idea to add the ready hook setup to the documentation, as well. We will specify it as an alternative method for setting up the Django integration when the recommended settings.py method is causing import issues.

szokeasaurusrex avatar Mar 05 '24 10:03 szokeasaurusrex

However, we agree that it would be a good idea to add the ready hook setup to the documentation, as well. We will specify it as an alternative method for setting up the Django integration when the recommended settings.py method is causing import issues

Upon further reflection, I believe we should not mention this alternative setup method in the docs. This setup method directly contradicts our configuration docs, which clearly state that the SDK initialization should occur "as early as possible in [the] application's lifecycle." Per Django's docs,AppConfig.ready is only called later in the app's lifecycle (specifically, after "the registry is fully populated"). Furthermore, the current method of setting up the SDK in Django clearly works for the vast majority of users; by comparison, this new method would be largely untested, since most users likely use our currently recommended setup.

I will close this issue for now. We can revisit this decision if more users report the same problem with the current setup instructions.

szokeasaurusrex avatar Apr 29 '24 16:04 szokeasaurusrex