From e31c1c71134fb477159b8a3bc489b49a04d2e503 Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 13 Feb 2020 10:56:46 -0500 Subject: [PATCH 1/3] utils: Update get_env to support any number of parts Changes get_env(integration, key) to get_env(*parts) to make it easier to get an env built from 1 part or more than 2 parts the major change here to pay attention to is the third positional argument used to be the default, not the default *must* be provided as a keyword argument --- ddtrace/context.py | 4 ++-- ddtrace/contrib/celery/patch.py | 4 ++-- ddtrace/contrib/dbapi/__init__.py | 2 +- ddtrace/contrib/falcon/patch.py | 2 +- ddtrace/contrib/jinja2/patch.py | 2 +- ddtrace/contrib/kombu/patch.py | 2 +- ddtrace/contrib/molten/patch.py | 4 ++-- ddtrace/contrib/pylons/patch.py | 2 +- ddtrace/contrib/pyramid/patch.py | 4 ++-- ddtrace/contrib/requests/patch.py | 6 +++--- ddtrace/settings/integration.py | 2 +- ddtrace/tracer.py | 5 +++-- ddtrace/utils/formats.py | 5 +++-- tests/test_utils.py | 7 ++++++- 14 files changed, 29 insertions(+), 22 deletions(-) diff --git a/ddtrace/context.py b/ddtrace/context.py index 7feff0c79f9..e2937b80eb6 100644 --- a/ddtrace/context.py +++ b/ddtrace/context.py @@ -25,8 +25,8 @@ class Context(object): This data structure is thread-safe. """ - _partial_flush_enabled = asbool(get_env('tracer', 'partial_flush_enabled', 'false')) - _partial_flush_min_spans = int(get_env('tracer', 'partial_flush_min_spans', 500)) + _partial_flush_enabled = asbool(get_env('tracer', 'partial_flush_enabled', default=False)) + _partial_flush_min_spans = int(get_env('tracer', 'partial_flush_min_spans', default=500)) def __init__(self, trace_id=None, span_id=None, sampling_priority=None, _dd_origin=None): """ diff --git a/ddtrace/contrib/celery/patch.py b/ddtrace/contrib/celery/patch.py index b6e9793840e..bb3d4320e1d 100644 --- a/ddtrace/contrib/celery/patch.py +++ b/ddtrace/contrib/celery/patch.py @@ -9,8 +9,8 @@ # Celery default settings config._add('celery', { - 'producer_service_name': get_env('celery', 'producer_service_name', PRODUCER_SERVICE), - 'worker_service_name': get_env('celery', 'worker_service_name', WORKER_SERVICE), + 'producer_service_name': get_env('celery', 'producer_service_name', default=PRODUCER_SERVICE), + 'worker_service_name': get_env('celery', 'worker_service_name', default=WORKER_SERVICE), }) diff --git a/ddtrace/contrib/dbapi/__init__.py b/ddtrace/contrib/dbapi/__init__.py index e8c577c7741..4c48ef66382 100644 --- a/ddtrace/contrib/dbapi/__init__.py +++ b/ddtrace/contrib/dbapi/__init__.py @@ -14,7 +14,7 @@ log = get_logger(__name__) config._add('dbapi2', dict( - trace_fetch_methods=asbool(get_env('dbapi2', 'trace_fetch_methods', 'false')), + trace_fetch_methods=asbool(get_env('dbapi2', 'trace_fetch_methods', default=False)), )) diff --git a/ddtrace/contrib/falcon/patch.py b/ddtrace/contrib/falcon/patch.py index 5eef31f6d6b..471c711d51d 100644 --- a/ddtrace/contrib/falcon/patch.py +++ b/ddtrace/contrib/falcon/patch.py @@ -23,7 +23,7 @@ def patch(): def traced_init(wrapped, instance, args, kwargs): mw = kwargs.pop('middleware', []) service = os.environ.get('DATADOG_SERVICE_NAME') or 'falcon' - distributed_tracing = asbool(get_env('falcon', 'distributed_tracing', True)) + distributed_tracing = asbool(get_env('falcon', 'distributed_tracing', default=True)) mw.insert(0, TraceMiddleware(tracer, service, distributed_tracing)) kwargs['middleware'] = mw diff --git a/ddtrace/contrib/jinja2/patch.py b/ddtrace/contrib/jinja2/patch.py index 988c33db9ea..65ac216a4f6 100644 --- a/ddtrace/contrib/jinja2/patch.py +++ b/ddtrace/contrib/jinja2/patch.py @@ -12,7 +12,7 @@ # default settings config._add('jinja2', { - 'service_name': get_env('jinja2', 'service_name', None), + 'service_name': get_env('jinja2', 'service_name'), }) diff --git a/ddtrace/contrib/kombu/patch.py b/ddtrace/contrib/kombu/patch.py index a82079f3e0c..828cc044b08 100644 --- a/ddtrace/contrib/kombu/patch.py +++ b/ddtrace/contrib/kombu/patch.py @@ -22,7 +22,7 @@ # kombu default settings config._add('kombu', { - 'service_name': get_env('kombu', 'service_name', DEFAULT_SERVICE) + 'service_name': get_env('kombu', 'service_name', default=DEFAULT_SERVICE) }) propagator = HTTPPropagator() diff --git a/ddtrace/contrib/molten/patch.py b/ddtrace/contrib/molten/patch.py index 967d06899f5..61e4de627f4 100644 --- a/ddtrace/contrib/molten/patch.py +++ b/ddtrace/contrib/molten/patch.py @@ -17,9 +17,9 @@ # Configure default configuration config._add('molten', dict( - service_name=get_env('molten', 'service_name', 'molten'), + service_name=get_env('molten', 'service_name', default='molten'), app='molten', - distributed_tracing=asbool(get_env('molten', 'distributed_tracing', True)), + distributed_tracing=asbool(get_env('molten', 'distributed_tracing', default=True)), )) diff --git a/ddtrace/contrib/pylons/patch.py b/ddtrace/contrib/pylons/patch.py index ad437d8c207..3ca18afedf2 100644 --- a/ddtrace/contrib/pylons/patch.py +++ b/ddtrace/contrib/pylons/patch.py @@ -32,7 +32,7 @@ def traced_init(wrapped, instance, args, kwargs): # set tracing options and create the TraceMiddleware service = os.environ.get('DATADOG_SERVICE_NAME', 'pylons') - distributed_tracing = asbool(get_env('pylons', 'distributed_tracing', True)) + distributed_tracing = asbool(get_env('pylons', 'distributed_tracing', default=True)) Pin(service=service, tracer=tracer).onto(instance) traced_app = PylonsTraceMiddleware(instance, tracer, service=service, distributed_tracing=distributed_tracing) diff --git a/ddtrace/contrib/pyramid/patch.py b/ddtrace/contrib/pyramid/patch.py index 8224d183def..c5d429eaa24 100644 --- a/ddtrace/contrib/pyramid/patch.py +++ b/ddtrace/contrib/pyramid/patch.py @@ -30,13 +30,13 @@ def patch(): def traced_init(wrapped, instance, args, kwargs): settings = kwargs.pop('settings', {}) service = os.environ.get('DATADOG_SERVICE_NAME') or 'pyramid' - distributed_tracing = asbool(get_env('pyramid', 'distributed_tracing', True)) + distributed_tracing = asbool(get_env('pyramid', 'distributed_tracing', default=True)) # DEV: integration-specific analytics flag can be not set but still enabled # globally for web frameworks analytics_enabled = get_env('pyramid', 'analytics_enabled') if analytics_enabled is not None: analytics_enabled = asbool(analytics_enabled) - analytics_sample_rate = get_env('pyramid', 'analytics_sample_rate', True) + analytics_sample_rate = get_env('pyramid', 'analytics_sample_rate', default=True) trace_settings = { SETTINGS_SERVICE: service, SETTINGS_DISTRIBUTED_TRACING: distributed_tracing, diff --git a/ddtrace/contrib/requests/patch.py b/ddtrace/contrib/requests/patch.py index 0072481514c..45939a9bac8 100644 --- a/ddtrace/contrib/requests/patch.py +++ b/ddtrace/contrib/requests/patch.py @@ -13,9 +13,9 @@ # requests default settings config._add('requests', { - 'service_name': get_env('requests', 'service_name', DEFAULT_SERVICE), - 'distributed_tracing': asbool(get_env('requests', 'distributed_tracing', True)), - 'split_by_domain': asbool(get_env('requests', 'split_by_domain', False)), + 'service_name': get_env('requests', 'service_name', default=DEFAULT_SERVICE), + 'distributed_tracing': asbool(get_env('requests', 'distributed_tracing', default=True)), + 'split_by_domain': asbool(get_env('requests', 'split_by_domain', default=False)), }) diff --git a/ddtrace/settings/integration.py b/ddtrace/settings/integration.py index 3d8b2288e8f..82cdd37d3e4 100644 --- a/ddtrace/settings/integration.py +++ b/ddtrace/settings/integration.py @@ -44,7 +44,7 @@ def __init__(self, global_config, name, *args, **kwargs): if analytics_enabled_env is not None: analytics_enabled_env = asbool(analytics_enabled_env) self.setdefault('analytics_enabled', analytics_enabled_env) - self.setdefault('analytics_sample_rate', float(get_env(name, 'analytics_sample_rate', 1.0))) + self.setdefault('analytics_sample_rate', float(get_env(name, 'analytics_sample_rate', default=1.0))) def __deepcopy__(self, memodict=None): new = IntegrationConfig(self.global_config, deepcopy(dict(self))) diff --git a/ddtrace/tracer.py b/ddtrace/tracer.py index 112eafe92d2..78606d89ceb 100644 --- a/ddtrace/tracer.py +++ b/ddtrace/tracer.py @@ -67,8 +67,9 @@ class Tracer(object): DEFAULT_HOSTNAME = environ.get('DD_AGENT_HOST', environ.get('DATADOG_TRACE_AGENT_HOSTNAME', 'localhost')) DEFAULT_PORT = int(environ.get('DD_TRACE_AGENT_PORT', 8126)) - DEFAULT_DOGSTATSD_PORT = int(get_env('dogstatsd', 'port', 8125)) - DEFAULT_DOGSTATSD_URL = get_env('dogstatsd', 'url', 'udp://{}:{}'.format(DEFAULT_HOSTNAME, DEFAULT_DOGSTATSD_PORT)) + DEFAULT_DOGSTATSD_PORT = int(get_env('dogstatsd', 'port', default=8125)) + DEFAULT_DOGSTATSD_URL = get_env('dogstatsd', 'url', + default='udp://{}:{}'.format(DEFAULT_HOSTNAME, DEFAULT_DOGSTATSD_PORT)) DEFAULT_AGENT_URL = environ.get('DD_TRACE_AGENT_URL', 'http://%s:%d' % (DEFAULT_HOSTNAME, DEFAULT_PORT)) def __init__(self, url=DEFAULT_AGENT_URL, dogstatsd_url=DEFAULT_DOGSTATSD_URL): diff --git a/ddtrace/utils/formats.py b/ddtrace/utils/formats.py index 7fe13dab553..57ea999b910 100644 --- a/ddtrace/utils/formats.py +++ b/ddtrace/utils/formats.py @@ -3,7 +3,7 @@ from .deprecation import deprecation -def get_env(integration, variable, default=None): +def get_env(*parts, default=None): """Retrieves environment variables value for the given integration. It must be used for consistency between integrations. The implementation is backward compatible with legacy nomenclature: @@ -15,7 +15,8 @@ def get_env(integration, variable, default=None): * return `default` otherwise """ - key = "{}_{}".format(integration, variable).upper() + key = "_".join(parts) + key = key.upper() legacy_env = "DATADOG_{}".format(key) env = "DD_{}".format(key) diff --git a/tests/test_utils.py b/tests/test_utils.py index 959c8acc7da..ccf44390b52 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -24,9 +24,14 @@ def test_get_env(self): # are not set value = get_env('django', 'distributed_tracing') self.assertIsNone(value) - value = get_env('django', 'distributed_tracing', False) + value = get_env('django', 'distributed_tracing', default=False) self.assertFalse(value) + def test_get_env_long(self): + os.environ['DD_SOME_VERY_LONG_TEST_KEY'] = '1' + value = get_env('some', 'very', 'long', 'test', 'key', default='2') + assert value == '1' + def test_get_env_found(self): # ensure `get_env` returns a value if the environment variable is set os.environ['DD_REQUESTS_DISTRIBUTED_TRACING'] = '1' From 3cfcf81342b2ed32d2d59e66feab201127701c9a Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 13 Feb 2020 11:17:34 -0500 Subject: [PATCH 2/3] keyword argument after *args is not supported in Python2 --- ddtrace/utils/formats.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ddtrace/utils/formats.py b/ddtrace/utils/formats.py index 57ea999b910..11d20a7d62c 100644 --- a/ddtrace/utils/formats.py +++ b/ddtrace/utils/formats.py @@ -3,7 +3,7 @@ from .deprecation import deprecation -def get_env(*parts, default=None): +def get_env(*parts, **kwargs): """Retrieves environment variables value for the given integration. It must be used for consistency between integrations. The implementation is backward compatible with legacy nomenclature: @@ -14,7 +14,15 @@ def get_env(*parts, default=None): arguments * return `default` otherwise + :param *parts: evironment variable parts that will be joined with ``_`` to generate the name + :type *parts: :obj:`str` + :param **kwargs: ``default`` is the only supported keyword argument which sets the default value + if no environment variable is found + :rtype: :obj:`str` | ``kwargs["default"]`` + :returns: The string environment variable value or the value of ``kwargs["default"]`` if not found """ + default = kwargs.get("default") + key = "_".join(parts) key = key.upper() legacy_env = "DATADOG_{}".format(key) From 5eb67b0a2ac34381510be589f9a4e2e74b09db0d Mon Sep 17 00:00:00 2001 From: brettlangdon Date: Thu, 13 Feb 2020 11:20:29 -0500 Subject: [PATCH 3/3] remove * --- ddtrace/utils/formats.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ddtrace/utils/formats.py b/ddtrace/utils/formats.py index 11d20a7d62c..66a089b8687 100644 --- a/ddtrace/utils/formats.py +++ b/ddtrace/utils/formats.py @@ -14,9 +14,9 @@ def get_env(*parts, **kwargs): arguments * return `default` otherwise - :param *parts: evironment variable parts that will be joined with ``_`` to generate the name - :type *parts: :obj:`str` - :param **kwargs: ``default`` is the only supported keyword argument which sets the default value + :param parts: evironment variable parts that will be joined with ``_`` to generate the name + :type parts: :obj:`str` + :param kwargs: ``default`` is the only supported keyword argument which sets the default value if no environment variable is found :rtype: :obj:`str` | ``kwargs["default"]`` :returns: The string environment variable value or the value of ``kwargs["default"]`` if not found