Merge pull request #441 from palazzem/config-system

[core] add a Configuration system for integrations

Author: Emanuele Palazzetti

ddtrace/__init__.py

@@ -2,7 +2,7 @@
 from .pin import Pin
 from .span import Span
 from .tracer import Tracer
-from .configuration import Config
+from .settings import Config
 
 __version__ = '0.11.1'
 

ddtrace/pin.py

@@ -1,42 +1,54 @@
 import logging
-import wrapt
 
+import wrapt
 import ddtrace
 
+
 log = logging.getLogger(__name__)
 
-_DD_PIN_NAME = '_datadog_pin'
 
 # To set attributes on wrapt proxy objects use this prefix:
 # http://wrapt.readthedocs.io/en/latest/wrappers.html
+_DD_PIN_NAME = '_datadog_pin'
 _DD_PIN_PROXY_NAME = '_self_' + _DD_PIN_NAME
 
 
 class Pin(object):
-    """ Pin (a.k.a Patch INfo) is a small class which is used to
-        set tracing metadata on a particular traced connection.
-        This is useful if you wanted to, say, trace two different
-        database clusters.
+    """Pin (a.k.a Patch INfo) is a small class which is used to
+    set tracing metadata on a particular traced connection.
+    This is useful if you wanted to, say, trace two different
+    database clusters.
 
         >>> conn = sqlite.connect("/tmp/user.db")
         >>> # Override a pin for a specific connection
         >>> pin = Pin.override(conn, service="user-db")
         >>> conn = sqlite.connect("/tmp/image.db")
     """
+    __slots__ = ['app', 'app_type', 'tags', 'tracer', '_target', '_config', '_initialized']
 
-    __slots__ = ['app', 'app_type', 'service', 'tags', 'tracer', '_initialized']
-
-    def __init__(self, service, app=None, app_type=None, tags=None, tracer=None):
+    def __init__(self, service, app=None, app_type=None, tags=None, tracer=None, _config=None):
         tracer = tracer or ddtrace.tracer
-        self.service = service
         self.app = app
         self.app_type = app_type
         self.tags = tags
         self.tracer = tracer
+        self._target = None
+        # keep the configuration attribute internal because the
+        # public API to access it is not the Pin class
+        self._config = _config or {}
+        # [Backward compatibility]: service argument updates the `Pin` config
+        self._config['service_name'] = service
         self._initialized = True
 
+    @property
+    def service(self):
+        """Backward compatibility: accessing to `pin.service` returns the underlying
+        configuration value.
+        """
+        return self._config['service_name']
+
     def __setattr__(self, name, value):
-        if hasattr(self, '_initialized'):
+        if getattr(self, '_initialized', False) and name is not '_target':
             raise AttributeError("can't mutate a pin, use override() or clone() instead")
         super(Pin, self).__setattr__(name, value)
 
@@ -46,15 +58,23 @@ def __repr__(self):
 
     @staticmethod
     def get_from(obj):
-        """ Return the pin associated with the given object.
+        """Return the pin associated with the given object. If a pin is attached to
+        `obj` but the instance is not the owner of the pin, a new pin is cloned and
+        attached. This ensures that a pin inherited from a class is a copy for the new
+        instance, avoiding that a specific instance overrides other pins values.
 
             >>> pin = Pin.get_from(conn)
         """
         if hasattr(obj, '__getddpin__'):
             return obj.__getddpin__()
 
         pin_name = _DD_PIN_PROXY_NAME if isinstance(obj, wrapt.ObjectProxy) else _DD_PIN_NAME
-        return getattr(obj, pin_name, None)
+        pin = getattr(obj, pin_name, None)
+        # detect if the PIN has been inherited from a class
+        if pin is not None and pin._target != id(obj):
+            pin = pin.clone()
+            pin.onto(obj)
+        return pin
 
     @classmethod
     def override(cls, obj, service=None, app=None, app_type=None, tags=None, tracer=None):
@@ -63,9 +83,9 @@ def override(cls, obj, service=None, app=None, app_type=None, tags=None, tracer=
         That's the recommended way to customize an already instrumented client, without
         losing existing attributes.
 
-        >>> conn = sqlite.connect("/tmp/user.db")
-        >>> # Override a pin for a specific connection
-        >>> pin = Pin.override(conn, service="user-db")
+            >>> conn = sqlite.connect("/tmp/user.db")
+            >>> # Override a pin for a specific connection
+            >>> Pin.override(conn, service="user-db")
         """
         if not obj:
             return
@@ -79,15 +99,16 @@ def override(cls, obj, service=None, app=None, app_type=None, tags=None, tracer=
             app=app,
             app_type=app_type,
             tags=tags,
-            tracer=tracer).onto(obj)
+            tracer=tracer,
+        ).onto(obj)
 
     def enabled(self):
-        """ Return true if this pin's tracer is enabled. """
+        """Return true if this pin's tracer is enabled. """
         return bool(self.tracer) and self.tracer.enabled
 
     def onto(self, obj, send=True):
-        """ Patch this pin onto the given object. If send is true, it will also
-            queue the metadata to be sent to the server.
+        """Patch this pin onto the given object. If send is true, it will also
+        queue the metadata to be sent to the server.
         """
         # pinning will also queue the metadata for service submission. this
         # feels a bit side-effecty, but bc it's async and pretty clearly
@@ -104,25 +125,39 @@ def onto(self, obj, send=True):
                 return obj.__setddpin__(self)
 
             pin_name = _DD_PIN_PROXY_NAME if isinstance(obj, wrapt.ObjectProxy) else _DD_PIN_NAME
+
+            # set the target reference; any get_from, clones and retarget the new PIN
+            self._target = id(obj)
             return setattr(obj, pin_name, self)
         except AttributeError:
             log.debug("can't pin onto object. skipping", exc_info=True)
 
     def clone(self, service=None, app=None, app_type=None, tags=None, tracer=None):
-        """ Return a clone of the pin with the given attributes replaced. """
+        """Return a clone of the pin with the given attributes replaced."""
+        # do a shallow copy of Pin dicts
         if not tags and self.tags:
-            # do a shallow copy of the tags if needed.
-            tags = {k:v for k, v in self.tags.items()}
+            tags = self.tags.copy()
+
+        # we use a copy instead of a deepcopy because we expect configurations
+        # to have only a root level dictionary without nested objects. Using
+        # deepcopy introduces a big overhead:
+        #
+        # copy: 0.00654911994934082
+        # deepcopy: 0.2787208557128906
+        config = self._config.copy()
 
         return Pin(
             service=service or self.service,
             app=app or self.app,
             app_type=app_type or self.app_type,
             tags=tags,
-            tracer=tracer or self.tracer) # no copy of the tracer
+            tracer=tracer or self.tracer,  # do not clone the Tracer
+            _config=config,
+        )
 
     def _send(self):
         self.tracer.set_service_info(
             service=self.service,
             app=self.app,
-            app_type=self.app_type)
+            app_type=self.app_type,
+        )

ddtrace/configuration.py renamed to ddtrace/settings.py

@@ -1,5 +1,12 @@
+import logging
+
 from copy import deepcopy
 
+from .pin import Pin
+
+
+log = logging.getLogger(__name__)
+
 
 class ConfigException(Exception):
     """Configuration exception when an integration that is not available
@@ -26,6 +33,19 @@ def __getattr__(self, name):
                 'Integration "{}" is not registered in this configuration'.format(e.message)
             )
 
+    def get_from(self, obj):
+        """Retrieves the configuration for the given object.
+        Any object that has an attached `Pin` must have a configuration
+        and if a wrong object is given, an empty `dict` is returned
+        for safety reasons.
+        """
+        pin = Pin.get_from(obj)
+        if pin is None:
+            log.debug('No configuration found for %s', obj)
+            return {}
+
+        return pin._config
+
     def _add(self, integration, settings):
         """Internal API that registers an integration with given default
         settings.

tests/test_configuration.py renamed to tests/test_global_config.py

@@ -3,10 +3,10 @@
 from nose.tools import eq_, ok_, assert_raises
 
 from ddtrace import config as global_config
-from ddtrace.configuration import Config, ConfigException
+from ddtrace.settings import Config, ConfigException
 
 
-class ConfigTestCase(TestCase):
+class GlobalConfigTestCase(TestCase):
     """Test the `Configuration` class that stores integration settings"""
     def setUp(self):
         self.config = Config()

tests/test_instance_config.py

@@ -0,0 +1,102 @@
+from unittest import TestCase
+
+from nose.tools import eq_, ok_
+
+from ddtrace import config
+from ddtrace.pin import Pin
+
+
+class InstanceConfigTestCase(TestCase):
+    """TestCase for the Configuration API that is used to define
+    global settings and for each `Pin` instance.
+    """
+    def setUp(self):
+        class Klass(object):
+            """Helper class where a Pin is always attached"""
+            pass
+
+        # define the Class and attach a Pin to it
+        self.Klass = Klass
+        Pin(service='metrics').onto(Klass)
+
+    def test_configuration_get_from(self):
+        # ensure a dictionary is returned
+        cfg = config.get_from(self.Klass)
+        ok_(isinstance(cfg, dict))
+
+    def test_configuration_get_from_twice(self):
+        # ensure the configuration is the same if `get_from` is used
+        # in the same instance
+        instance = self.Klass()
+        cfg1 = config.get_from(instance)
+        cfg2 = config.get_from(instance)
+        ok_(cfg1 is cfg2)
+
+    def test_configuration_set(self):
+        # ensure the configuration can be updated in the Pin
+        instance = self.Klass()
+        cfg = config.get_from(instance)
+        cfg['distributed_tracing'] = True
+        ok_(config.get_from(instance)['distributed_tracing'] is True)
+
+    def test_global_configuration_inheritance(self):
+        # ensure global configuration is inherited when it's set
+        cfg = config.get_from(self.Klass)
+        cfg['distributed_tracing'] = True
+        instance = self.Klass()
+        ok_(config.get_from(instance)['distributed_tracing'] is True)
+
+    def test_configuration_override_instance(self):
+        # ensure instance configuration doesn't override global settings
+        global_cfg = config.get_from(self.Klass)
+        global_cfg['distributed_tracing'] = True
+        instance = self.Klass()
+        cfg = config.get_from(instance)
+        cfg['distributed_tracing'] = False
+        ok_(config.get_from(self.Klass)['distributed_tracing'] is True)
+        ok_(config.get_from(instance)['distributed_tracing'] is False)
+
+    def test_service_name_for_pin(self):
+        # ensure for backward compatibility that changing the service
+        # name via the Pin object also updates integration config
+        Pin(service='intake').onto(self.Klass)
+        instance = self.Klass()
+        cfg = config.get_from(instance)
+        eq_(cfg['service_name'], 'intake')
+
+    def test_service_attribute_priority(self):
+        # ensure the `service` arg has highest priority over configuration
+        # for backward compatibility
+        global_config = {
+            'service_name': 'primary_service',
+        }
+        Pin(service='service', _config=global_config).onto(self.Klass)
+        instance = self.Klass()
+        cfg = config.get_from(instance)
+        eq_(cfg['service_name'], 'service')
+
+    def test_configuration_copy(self):
+        # ensure when a Pin is used, the given configuration is copied
+        global_config = {
+            'service_name': 'service',
+        }
+        Pin(service='service', _config=global_config).onto(self.Klass)
+        instance = self.Klass()
+        cfg = config.get_from(instance)
+        cfg['service_name'] = 'metrics'
+        eq_(global_config['service_name'], 'service')
+
+    def test_configuration_copy_upside_down(self):
+        # ensure when a Pin is created, it does not copy the given configuration
+        # until it's used for at least once
+        global_config = {
+            'service_name': 'service',
+        }
+        Pin(service='service', _config=global_config).onto(self.Klass)
+        # override the global config: users do that before using the integration
+        global_config['service_name'] = 'metrics'
+        # use the Pin via `get_from`
+        instance = self.Klass()
+        cfg = config.get_from(instance)
+        # it should have users updated value
+        eq_(cfg['service_name'], 'metrics')