Merge pull request #434 from palazzem/requests-split

[requests] use the domain name as a service name

Author: Emanuele Palazzetti

ddtrace/compat.py

@@ -26,9 +26,9 @@
     from io import StringIO
 
 try:
-    import urlparse
+    import urlparse as parse
 except ImportError:
-    from urllib import parse as urlparse
+    from urllib import parse
 
 try:
     from asyncio import iscoroutinefunction
@@ -88,5 +88,5 @@ def to_unicode(s):
     'stringify',
     'StringIO',
     'urlencode',
-    'urlparse',
+    'parse',
 ]

ddtrace/contrib/requests/connection.py

@@ -6,25 +6,32 @@
 from .constants import DEFAULT_SERVICE
 
 from ...ext import http
+from ...compat import parse
 from ...propagation.http import HTTPPropagator
 
 
 log = logging.getLogger(__name__)
 
 
-def _extract_service_name(session, span):
+def _extract_service_name(session, span, netloc=None):
     """Extracts the right service name based on the following logic:
     - `requests` is the default service name
     - users can change it via `session.service_name = 'clients'`
     - if the Span doesn't have a parent, use the set service name
       or fallback to the default
     - if the Span has a parent, use the set service name or the
       parent service value if the set service name is the default
+    - if `split_by_domain` is used, always override users settings
+      and use the network location as a service name
 
     The priority can be represented as:
     Updated service name > parent service name > default to `requests`.
     """
-    service_name = config.get_from(session)['service_name']
+    cfg = config.get_from(session)
+    if cfg['split_by_domain'] and netloc:
+        return netloc
+
+    service_name = cfg['service_name']
     if (service_name == DEFAULT_SERVICE and
             span._parent is not None and
             span._parent.service is not None):
@@ -47,10 +54,11 @@ def _wrap_request(func, instance, args, kwargs):
     method = kwargs.get('method') or args[0]
     url = kwargs.get('url') or args[1]
     headers = kwargs.get('headers', {})
+    parsed_uri = parse.urlparse(url)
 
     with tracer.trace("requests.request", span_type=http.TYPE) as span:
         # update the span service name before doing any action
-        span.service = _extract_service_name(instance, span)
+        span.service = _extract_service_name(instance, span, netloc=parsed_uri.netloc)
 
         # propagate distributed tracing headers
         if config.get_from(instance).get('distributed_tracing'):

ddtrace/contrib/requests/patch.py

@@ -16,6 +16,7 @@
 config._add('requests',{
     'service_name': get_env('requests', 'service_name', DEFAULT_SERVICE),
     'distributed_tracing': asbool(get_env('requests', 'distributed_tracing', False)),
+    'split_by_domain': asbool(get_env('requests', 'split_by_domain', False)),
 })
 
 

tests/contrib/requests/test_requests.py

@@ -2,7 +2,8 @@
 import requests
 
 from requests import Session
-from nose.tools import eq_
+from requests.exceptions import MissingSchema
+from nose.tools import eq_, assert_raises
 
 from ddtrace import config
 from ddtrace.ext import http, errors
@@ -225,3 +226,45 @@ def test_user_service_name_precedence(self):
 
         eq_(s.name, 'requests.request')
         eq_(s.service, 'clients')
+
+    def test_split_by_domain(self):
+        # ensure a service name is generated by the domain name
+        # of the ongoing call
+        cfg = config.get_from(self.session)
+        cfg['split_by_domain'] = True
+        out = self.session.get(URL_200)
+        eq_(out.status_code, 200)
+
+        spans = self.tracer.writer.pop()
+        eq_(len(spans), 1)
+        s = spans[0]
+
+        eq_(s.service, 'httpbin.org')
+
+    def test_split_by_domain_precedence(self):
+        # ensure the split by domain has precedence all the time
+        cfg = config.get_from(self.session)
+        cfg['split_by_domain'] = True
+        cfg['service_name'] = 'intake'
+        out = self.session.get(URL_200)
+        eq_(out.status_code, 200)
+
+        spans = self.tracer.writer.pop()
+        eq_(len(spans), 1)
+        s = spans[0]
+
+        eq_(s.service, 'httpbin.org')
+
+    def test_split_by_domain_wrong(self):
+        # ensure the split by domain doesn't crash in case of a wrong URL;
+        # in that case, the default service name must be used
+        cfg = config.get_from(self.session)
+        cfg['split_by_domain'] = True
+        with assert_raises(MissingSchema):
+            self.session.get('http:/some>thing')
+
+        spans = self.tracer.writer.pop()
+        eq_(len(spans), 1)
+        s = spans[0]
+
+        eq_(s.service, 'requests')