Skip to content

Commit 1c5a3ba

Browse files
authored
[core] Update config to allow configuration before patching (#650)
* Add ddtrace.utils.merge.deepmerge helper function * [core] Update config to allow configuration before patching Previously if we tried to set a config setting before the contrib module was imported we would receive an error that the integration key did not exist. With this new approach we are allowing any integration setting to be configured by the user, whenever they want and then when we call `config._add()` in the contrib module, we will merge the settings with any existing settings, keeping those that already exist. We have also added a Config.__repr__ * [tests] Fix spelling mistakes in global config test docstrings
1 parent 87b9ff7 commit 1c5a3ba

File tree

3 files changed

+89
-8
lines changed

3 files changed

+89
-8
lines changed

ddtrace/settings.py

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from copy import deepcopy
44

55
from .pin import Pin
6+
from .utils.merge import deepmerge
67

78

89
log = logging.getLogger(__name__)
@@ -26,12 +27,9 @@ def __init__(self):
2627
self._config = {}
2728

2829
def __getattr__(self, name):
29-
try:
30-
return self._config[name]
31-
except KeyError as e:
32-
raise ConfigException(
33-
'Integration "{}" is not registered in this configuration'.format(e.message)
34-
)
30+
if name not in self._config:
31+
self._config[name] = dict()
32+
return self._config[name]
3533

3634
def get_from(self, obj):
3735
"""Retrieves the configuration for the given object.
@@ -46,14 +44,36 @@ def get_from(self, obj):
4644

4745
return pin._config
4846

49-
def _add(self, integration, settings):
47+
def _add(self, integration, settings, merge=True):
5048
"""Internal API that registers an integration with given default
5149
settings.
5250
5351
:param str integration: The integration name (i.e. `requests`)
5452
:param dict settings: A dictionary that contains integration settings;
5553
to preserve immutability of these values, the dictionary is copied
5654
since it contains integration defaults.
55+
:param bool merge: Whether to merge any existing settings with those provided,
56+
or if we should overwrite the settings with those provided;
57+
Note: when merging existing settings take precedence.
5758
"""
59+
# DEV: Use `getattr()` to call our `__getattr__` helper
60+
existing = getattr(self, integration)
61+
settings = deepcopy(settings)
5862

59-
self._config[integration] = deepcopy(settings)
63+
if merge:
64+
# DEV: This may appear backwards keeping `existing` as the "source" and `settings` as
65+
# the "destination", but we do not want to let `_add(..., merge=True)` overwrite any
66+
# existing settings
67+
#
68+
# >>> config.requests['split_by_domain'] = True
69+
# >>> config._add('requests', dict(split_by_domain=False))
70+
# >>> config.requests['split_by_domain']
71+
# True
72+
self._config[integration] = deepmerge(existing, settings)
73+
else:
74+
self._config[integration] = settings
75+
76+
def __repr__(self):
77+
cls = self.__class__
78+
integrations = ', '.join(self._config.keys())
79+
return '{}.{}({})'.format(cls.__module__, cls.__name__, integrations)

ddtrace/utils/merge.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Borrowed from: https://stackoverflow.com/questions/20656135/python-deep-merge-dictionary-data#20666342
2+
def deepmerge(source, destination):
3+
"""
4+
Merge the first provided ``dict`` into the second.
5+
6+
:param dict source: The ``dict`` to merge into ``destination``
7+
:param dict destination: The ``dict`` that should get updated
8+
:rtype: dict
9+
:returns: ``destination`` modified
10+
"""
11+
for key, value in source.items():
12+
if isinstance(value, dict):
13+
# get node or create one
14+
node = destination.setdefault(key, {})
15+
deepmerge(value, node)
16+
else:
17+
destination[key] = value
18+
19+
return destination

tests/test_global_config.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,45 @@ def test_missing_integration(self):
4848
def test_global_configuration(self):
4949
# ensure a global configuration is available in the `ddtrace` module
5050
ok_(isinstance(global_config, Config))
51+
52+
def test_settings_merge(self):
53+
"""
54+
When calling `config._add()`
55+
when existing settings exist
56+
we do not overwrite the existing settings
57+
"""
58+
self.config.requests['split_by_domain'] = True
59+
self.config._add('requests', dict(split_by_domain=False))
60+
eq_(self.config.requests['split_by_domain'], True)
61+
62+
def test_settings_overwrite(self):
63+
"""
64+
When calling `config._add(..., merge=False)`
65+
when existing settings exist
66+
we overwrite the existing settings
67+
"""
68+
self.config.requests['split_by_domain'] = True
69+
self.config._add('requests', dict(split_by_domain=False), merge=False)
70+
eq_(self.config.requests['split_by_domain'], False)
71+
72+
def test_settings_merge_deep(self):
73+
"""
74+
When calling `config._add()`
75+
when existing "deep" settings exist
76+
we do not overwrite the existing settings
77+
"""
78+
self.config.requests['a'] = dict(
79+
b=dict(
80+
c=True,
81+
),
82+
)
83+
self.config._add('requests', dict(
84+
a=dict(
85+
b=dict(
86+
c=False,
87+
d=True,
88+
),
89+
),
90+
))
91+
eq_(self.config.requests['a']['b']['c'], True)
92+
eq_(self.config.requests['a']['b']['d'], True)

0 commit comments

Comments
 (0)