reorder config precedence to align with all other agents (#261)

refs elastic/apm-agent-nodejs#287
closes #255
closes #261

Signed-off-by: Benjamin Wohlwend <[email protected]>

Author: beniwohli

CHANGELOG.md

@@ -5,9 +5,11 @@
 [Check the diff](https://github.com/elastic/apm-agent-python/compare/v2.2.1...master)
 
  - **BREAKING**: dropped support for Python 3.3 (#242) 
+ - **BREAKING**: changed order of precedence when evaluating configuration (#255, #261)
  - **BREAKING**: changed default value of `span_frames_min_duration` setting 
    from `-1` (always collect) to `5` (only collect for spans longer than 5 ms) (#243)
  - added instrumentation for pymssql (#241)
+ - added instrumentation for pyodbc (#238) 
 
 ## v2.2.1
 

docs/configuration.asciidoc

@@ -4,6 +4,35 @@
 To adapt the Elastic APM agent to your needs, you can configure it using environment variables, or framework specific
 configuration.
 
+You can either configure the agent by setting environment variables:
+[source,bash]
+----
+ELASTIC_APM_SERVICE_NAME=foo python manage.py runserver
+----
+
+or with inline configuration: 
+
+[source,python]
+----
+apm_client = Client(service_name="foo")
+----
+
+or by using framework specific configuration e.g. in your Django `settings.py` file:
+
+[source,python]
+----
+ELASTIC_APM = {
+    "SERVICE_NAME": "foo",
+}
+----
+
+The precedence is as follows:
+
+ * environment variables
+ * inline configuration
+ * framework specific configuration
+ * default value
+
 [float]
 [[django-configuration]]
 === Django

elasticapm/base.py

@@ -99,7 +99,7 @@ class Client(object):
     """
     logger = logging.getLogger('elasticapm')
 
-    def __init__(self, config=None, **defaults):
+    def __init__(self, config=None, **inline):
         # configure loggers first
         cls = self.__class__
         self.logger = logging.getLogger('%s.%s' % (cls.__module__, cls.__name__))
@@ -113,7 +113,7 @@ def __init__(self, config=None, **defaults):
         self._transports = {}
         self._service_info = None
 
-        self.config = Config(config, default_dict=defaults)
+        self.config = Config(config, inline_dict=inline)
         if self.config.errors:
             for msg in self.config.errors.values():
                 self.error_logger.error(msg)

elasticapm/conf/__init__.py

@@ -97,28 +97,28 @@ def __set__(self, instance, value):
 class _ConfigBase(object):
     _NO_VALUE = object()  # sentinel object
 
-    def __init__(self, config_dict=None, env_dict=None, default_dict=None):
+    def __init__(self, config_dict=None, env_dict=None, inline_dict=None):
         self._values = {}
         self._errors = {}
         if config_dict is None:
             config_dict = {}
         if env_dict is None:
             env_dict = os.environ
-        if default_dict is None:
-            default_dict = {}
+        if inline_dict is None:
+            inline_dict = {}
         for field, config_value in self.__class__.__dict__.items():
             if not isinstance(config_value, _ConfigValue):
                 continue
             new_value = self._NO_VALUE
-            # check config dictionary first
-            if config_value.dict_key in config_dict:
-                new_value = config_dict[config_value.dict_key]
-            # then check environment
-            elif config_value.env_key and config_value.env_key in env_dict:
+            # first check environment
+            if config_value.env_key and config_value.env_key in env_dict:
                 new_value = env_dict[config_value.env_key]
-            # finally, if no value is set, check the client-provided defaults
-            elif field in default_dict:
-                new_value = default_dict[field]
+            # check the inline config
+            elif field in inline_dict:
+                new_value = inline_dict[field]
+            # finally, check config dictionary
+            elif config_value.dict_key in config_dict:
+                new_value = config_dict[config_value.dict_key]
             # only set if new_value changed. We'll fall back to the field default if not.
             if new_value is not self._NO_VALUE:
                 try:

elasticapm/contrib/django/client.py

@@ -60,13 +60,13 @@ def get_client(client=None):
 class DjangoClient(Client):
     logger = logging.getLogger('elasticapm.errors.client.django')
 
-    def __init__(self, config=None, **defaults):
+    def __init__(self, config=None, **inline):
         if config is None:
             config = getattr(django_settings, 'ELASTIC_APM', {})
-        if 'framework_name' not in defaults:
-            defaults['framework_name'] = 'django'
-            defaults['framework_version'] = django.get_version()
-        super(DjangoClient, self).__init__(config, **defaults)
+        if 'framework_name' not in inline:
+            inline['framework_name'] = 'django'
+            inline['framework_version'] = django.get_version()
+        super(DjangoClient, self).__init__(config, **inline)
 
     def get_user_info(self, request):
         user_info = {}

tests/config/tests.py

@@ -65,8 +65,8 @@ def test_config_environment():
         assert config.auto_log_stacks == False
 
 
-def test_config_defaults_dict():
-    config = Config(default_dict={
+def test_config_inline_dict():
+    config = Config(inline_dict={
         'service_name': 'foo',
         'secret_token': 'bar',
         'server_url': 'http://example.com:1234',
@@ -84,17 +84,22 @@ def test_config_defaults_dict():
 
 
 def test_config_precedence():
-    #  precendece order: config dict, environment, default dict
+    #  precedence order: environment, inline dict, config dict
     with mock.patch.dict('os.environ', {
         'ELASTIC_APM_SERVICE_NAME': 'bar',
-        'ELASTIC_APM_SECRET_TOKEN': 'secret'
     }):
         config = Config({
             'SERVICE_NAME': 'foo',
-        }, default_dict={'secret_token': 'notsecret'})
-
-    assert config.service_name == 'foo'
-    assert config.secret_token == 'secret'
+            'SECRET_TOKEN': 'secret',
+            'COLLECT_LOCAL_VARIABLES': 'all'
+        }, inline_dict={
+            'secret_token': 'notsecret',
+            'service_name': 'baz'
+        })
+
+    assert config.service_name == 'bar'
+    assert config.secret_token == 'notsecret'
+    assert config.collect_local_variables == 'all'
 
 
 def test_list_config_value():

tests/fixtures.py

@@ -134,9 +134,9 @@ def sending_elasticapm_client(request, validating_httpserver):
 
 
 class TempStoreClient(Client):
-    def __init__(self, **defaults):
+    def __init__(self, **inline):
         self.events = []
-        super(TempStoreClient, self).__init__(**defaults)
+        super(TempStoreClient, self).__init__(**inline)
 
     def send(self, url, **kwargs):
         self.events.append(kwargs)