ref: Integration options now live on the client (#101)

* ref: Integration options now live on the client

* ref: Use an indirection for working with integrations

* ref: Refactored integration code again

* fix: Fixed some tests

* fix: Fixed the celery integration

* tests: Added test for integration separation

* fix: Restore old `install` for backwards compat

* fix: styling

* ref: No classmethod for setup_once

* fix: Fix missing transaction in celery

* fix: Fix sanic crash

* feat: Added warning to disabled integrations due to lack of fall through

* feat: Emit a warning into the debug log when an integration does not fall through

* fix: linters

Author: untitaker

sentry_sdk/_compat.py

@@ -12,6 +12,7 @@
     string_types = (str, text_type)
     number_types = (int, long, float)  # noqa
     int_types = (int, long)  # noqa
+    iteritems = lambda x: x.iteritems()
 
     def implements_str(cls):
         cls.__unicode__ = cls.__str__
@@ -34,6 +35,7 @@ def implements_iterator(cls):
     string_types = (text_type,)
     number_types = (int, float)
     int_types = (int,)  # noqa
+    iteritems = lambda x: x.items()
 
     def _identity(x):
         return x

sentry_sdk/api.py

@@ -1,14 +1,16 @@
 import inspect
 from contextlib import contextmanager
 
-from sentry_sdk.hub import Hub
+from sentry_sdk.hub import Hub, init
 from sentry_sdk.scope import Scope
 from sentry_sdk.transport import Transport, HttpTransport
-from sentry_sdk.client import Client, get_options
-from sentry_sdk.integrations import setup_integrations
+from sentry_sdk.client import Client
 
 
-__all__ = ["Hub", "Scope", "Client", "Transport", "HttpTransport"]
+__all__ = ["Hub", "Scope", "Client", "Transport", "HttpTransport", "init"]
+
+
+_initial_client = None
 
 
 def public(f):
@@ -24,45 +26,6 @@ def hubmethod(f):
     return public(f)
 
 
-class _InitGuard(object):
-    def __init__(self, client):
-        self._client = client
-
-    def __enter__(self):
-        return self
-
-    def __exit__(self, exc_type, exc_value, tb):
-        c = self._client
-        if c is not None:
-            c.close()
-
-
-def _init_on_hub(hub, args, kwargs):
-    options = get_options(*args, **kwargs)
-    client = Client(options)
-    hub.bind_client(client)
-    setup_integrations(
-        options["integrations"] or [], with_defaults=options["default_integrations"]
-    )
-    return _InitGuard(client)
-
-
-@public
-def init(*args, **kwargs):
-    """Initializes the SDK and optionally integrations.
-
-    This takes the same arguments as the client constructor.
-    """
-    return _init_on_hub(Hub.main, args, kwargs)
-
-
-def _init_on_current(*args, **kwargs):
-    # This function only exists to support unittests.  Do not call it as
-    # initializing integrations on anything but the main hub is not going
-    # to yield the results you expect.
-    return _init_on_hub(Hub.current, args, kwargs)
-
-
 @hubmethod
 def capture_event(event, hint=None):
     hub = Hub.current

sentry_sdk/client.py

@@ -16,6 +16,7 @@
 )
 from sentry_sdk.transport import make_transport
 from sentry_sdk.consts import DEFAULT_OPTIONS, SDK_INFO
+from sentry_sdk.integrations import setup_integrations
 
 
 def get_options(*args, **kwargs):
@@ -60,6 +61,10 @@ def __init__(self, *args, **kwargs):
                 )
             )
 
+        self.integrations = setup_integrations(
+            options["integrations"], with_defaults=options["default_integrations"]
+        )
+
     @property
     def dsn(self):
         """Returns the configured DSN as string."""
@@ -93,7 +98,9 @@ def _prepare_event(self, event, hint, scope):
             if event.get(key) is None:
                 event[key] = self.options[key]
         if event.get("sdk") is None:
-            event["sdk"] = SDK_INFO
+            sdk_info = dict(SDK_INFO)
+            sdk_info["integrations"] = sorted(self.integrations.keys())
+            event["sdk"] = sdk_info
 
         if event.get("platform") is None:
             event["platform"] = "python"

sentry_sdk/consts.py

@@ -30,12 +30,8 @@
 }
 
 
-# Modified by sentry_sdk.integrations
-INTEGRATIONS = []
-
 SDK_INFO = {
     "name": "sentry.python",
     "version": VERSION,
     "packages": [{"name": "pypi:sentry-sdk", "version": VERSION}],
-    "integrations": INTEGRATIONS,
 }

sentry_sdk/hub.py

@@ -1,10 +1,13 @@
 import sys
 import copy
+import weakref
 from datetime import datetime
 from contextlib import contextmanager
+from warnings import warn
 
-from sentry_sdk._compat import with_metaclass
+from sentry_sdk._compat import with_metaclass, string_types
 from sentry_sdk.scope import Scope
+from sentry_sdk.client import Client
 from sentry_sdk.utils import (
     exc_info_from_error,
     event_from_exception,
@@ -14,12 +17,7 @@
 
 
 _local = ContextVar("sentry_current_hub")
-
-
-def _get_client_options():
-    hub = Hub.current
-    if hub and hub.client:
-        return hub.client.options
+_initial_client = None
 
 
 def _should_send_default_pii():
@@ -29,6 +27,33 @@ def _should_send_default_pii():
     return client.options["send_default_pii"]
 
 
+class _InitGuard(object):
+    def __init__(self, client):
+        self._client = client
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exc_type, exc_value, tb):
+        c = self._client
+        if c is not None:
+            c.close()
+
+
+def init(*args, **kwargs):
+    """Initializes the SDK and optionally integrations.
+
+    This takes the same arguments as the client constructor.
+    """
+    global _initial_client
+    client = Client(*args, **kwargs)
+    Hub.main.bind_client(client)
+    rv = _InitGuard(client)
+    if client is not None:
+        _initial_client = weakref.ref(client)
+    return rv
+
+
 class HubMeta(type):
     @property
     def current(self):
@@ -107,6 +132,41 @@ def run(self, callback):
         with self:
             return callback()
 
+    def get_integration(self, name_or_class):
+        """Returns the integration for this hub by name or class.  If there
+        is no client bound or the client does not have that integration
+        then `None` is returned.
+
+        If the return value is not `None` the hub is guaranteed to have a
+        client attached.
+        """
+        if not isinstance(name_or_class, string_types):
+            name_or_class = name_or_class.identifier
+        client = self._stack[-1][0]
+        if client is not None:
+            rv = client.integrations.get(name_or_class)
+            if rv is not None:
+                return rv
+
+        initial_client = _initial_client
+        if initial_client is not None:
+            initial_client = initial_client()
+
+        if (
+            initial_client is not None
+            and initial_client is not client
+            and initial_client.integrations.get(name_or_class) is not None
+        ):
+            warning = (
+                "Integration %r attempted to run but it was only "
+                "enabled on init() but not the client that "
+                "was bound to the current flow.  Earlier versions of "
+                "the SDK would consider these integrations enabled but "
+                "this is no longer the case." % (name_or_class,)
+            )
+            warn(Warning(warning), stacklevel=3)
+            logger.warning(warning)
+
     @property
     def client(self):
         """Returns the current client on the hub."""

sentry_sdk/integrations/__init__.py

@@ -1,15 +1,17 @@
 """This package"""
 from threading import Lock
+from collections import namedtuple
 
+from sentry_sdk._compat import iteritems
 from sentry_sdk.utils import logger
-from sentry_sdk.consts import INTEGRATIONS as _installed_integrations
 
 
 _installer_lock = Lock()
+_installed_integrations = set()
 
 
-def get_default_integrations():
-    """Returns an iterator of default integration instances.
+def iter_default_integrations():
+    """Returns an iterator of default integration classes.
 
     This returns the following default integration:
 
@@ -25,54 +27,74 @@ def get_default_integrations():
     from sentry_sdk.integrations.atexit import AtexitIntegration
     from sentry_sdk.integrations.modules import ModulesIntegration
 
-    yield LoggingIntegration()
-    yield StdlibIntegration()
-    yield ExcepthookIntegration()
-    yield DedupeIntegration()
-    yield AtexitIntegration()
-    yield ModulesIntegration()
+    yield LoggingIntegration
+    yield StdlibIntegration
+    yield ExcepthookIntegration
+    yield DedupeIntegration
+    yield AtexitIntegration
+    yield ModulesIntegration
 
 
 def setup_integrations(integrations, with_defaults=True):
     """Given a list of integration instances this installs them all.  When
     `with_defaults` is set to `True` then all default integrations are added
     unless they were already provided before.
     """
-    integrations = list(integrations)
+    integrations = dict(
+        (integration.identifier, integration) for integration in integrations or ()
+    )
+
     if with_defaults:
-        for instance in get_default_integrations():
-            if not any(isinstance(x, type(instance)) for x in integrations):
-                integrations.append(instance)
+        for integration_cls in iter_default_integrations():
+            if integration_cls.identifier not in integrations:
+                instance = integration_cls()
+                integrations[instance.identifier] = instance
+
+    for identifier, integration in iteritems(integrations):
+        with _installer_lock:
+            if identifier not in _installed_integrations:
+                try:
+                    type(integration).setup_once()
+                except NotImplementedError:
+                    if getattr(integration, "install", None) is not None:
+                        logger.warn(
+                            "Integration %s: The install method is "
+                            "deprecated. Use `setup_once`.",
+                            identifier,
+                        )
+                        integration.install()
+                    else:
+                        raise
+                _installed_integrations.add(identifier)
 
-    for integration in integrations:
-        integration()
+    return integrations
+
+
+IntegrationAttachment = namedtuple(
+    "IntegrationAttachment", ["integration", "client", "hub"]
+)
 
 
 class Integration(object):
-    """Baseclass for all integrations."""
+    """Baseclass for all integrations.
 
-    identifier = None
-    """A unique identifying string for the integration.  Integrations must
-    set this as a class attribute.
+    To accept options for an integration, implement your own constructor that
+    saves those options on `self`.
     """
 
-    def install(self):
-        """An integration must implement all its code here.  When the
-        `setup_integrations` function runs it will invoke this unless the
-        integration was already activated elsewhere.
+    install = None
+    """Legacy method, do not implement."""
+
+    @staticmethod
+    def setup_once():
         """
-        raise NotImplementedError()
+        Initialize the integration.
 
-    def __call__(self):
-        assert self.identifier
-        with _installer_lock:
-            if self.identifier in _installed_integrations:
-                logger.warning(
-                    "%s integration for Sentry is already "
-                    "configured. Will ignore second configuration.",
-                    self.identifier,
-                )
-                return
-
-            self.install()
-            _installed_integrations.append(self.identifier)
+        This function is only called once, ever. Configuration is not available
+        at this point, so the only thing to do here is to hook into exception
+        handlers, and perhaps do monkeypatches.
+
+        Inside those hooks `Integration.current` can be used to access the
+        instance again.
+        """
+        raise NotImplementedError()

sentry_sdk/integrations/_wsgi.py

@@ -1,7 +1,7 @@
 import json
 import sys
 
-from sentry_sdk.hub import Hub, _should_send_default_pii, _get_client_options
+from sentry_sdk.hub import Hub, _should_send_default_pii
 from sentry_sdk.utils import (
     AnnotatedValue,
     capture_internal_exceptions,
@@ -44,8 +44,8 @@ def __init__(self, request):
         self.request = request
 
     def extract_into_event(self, event):
-        client_options = _get_client_options()
-        if client_options is None:
+        client = Hub.current.client
+        if client is None:
             return
 
         content_length = self.content_length()
@@ -55,7 +55,7 @@ def extract_into_event(self, event):
         if _should_send_default_pii():
             request_info["cookies"] = dict(self.cookies())
 
-        bodies = client_options["request_bodies"]
+        bodies = client.options["request_bodies"]
         if (
             bodies == "never"
             or (bodies == "small" and content_length > 10 ** 3)