Loguru integration does not support disabled_integrations?

[alicederyn]

How do you use Sentry?

Self-hosted/on-premise

Version

2.28.0

Steps to Reproduce

As I understand the docs, you would disable Loguru integration like follows:

sentry_sdk.init(..., disabled_integrations=[LoguruIntegration()])

Expected Result

Loguru integration to be disabled.

Actual Result

LoguruIntegration is enabling itself in its __init__ method, not in setup_once:

sentry-python/sentry_sdk/integrations/loguru.py

Lines 90 to 93 in aa0eaab

	@staticmethod
	def setup_once():
	# type: () -> None
	pass # we do everything in __init__

This means the above code does not actually disable Loguru integration at all, it actually enables it earlier. I don't want to have to disable all automatic integrations just to disable this one.

(Also, given sentry_sdk.init is a context manager, I would expect Loguru integration to be disabled once the context manager was exited -- this would help with running test suites :) )

[sentrivana]

Hey @alicederyn, thanks for raising this. When we introduced disabled_integrations we overlooked that Loguru is special in that it doesn't use setup_once. 🙃

We'll need to do the following:

• Instruct folks to provide the class, not an instance of it, in disabled_integrations, so that the class is not instantiated. (This already works now, you can do either, but in the docs we're using instances which won't work in this case.)
• Fix up our integration setup code so that it doesn't instantiate disabled integrations (because right now it does, so the first bullet point is not enough).

Alternatively, maybe it's possible to move the Loguru setup code to setup_once (which would be the preferred solution), but not sure atm.

I'll take a look at this soon.

[alicederyn]

I think Loguru not using setup_once also breaks the ability for users to customize it? Since it'll be registered with their settings, then immediately deregistered by the new version created during sentry_sdk.init. Users would also need to add the class to disabled_integrations. If I'm right, I think having the code moved would be better?

[sentrivana]

Yeah I'm also not happy about the code being in the __init__ -- we have setup_once exactly because the setup code should not run multiple times.

Will def investigate if we can move it. Right now there's a comment that suggests that's not possible, but the LoggingIntegration needs to do similar things in order to get set up and it manages to do them in setup_once, so I'm hoping it's possible.