fix: remove url parts from http.url tag [backport #4906 to 0.61] (#4912)

Backport of #4906 

This fix removes unintended url parts in the `http.url` tag.

Co-authored-by: Tahir H. Butt <[email protected]>
Co-authored-by: Brett Langdon <[email protected]>

Co-authored-by: Zachary Groves <[email protected]>
Co-authored-by: Tahir H. Butt <[email protected]>

Author: 3 people

ddtrace/contrib/trace_utils.py

@@ -15,6 +15,7 @@
 from ddtrace import Pin
 from ddtrace import config
 from ddtrace.ext import http
+from ddtrace.internal.compat import parse
 from ddtrace.internal.logger import get_logger
 from ddtrace.internal.utils.cache import cached
 from ddtrace.internal.utils.http import normalize_header_name
@@ -132,6 +133,29 @@ def _store_response_headers(headers, span, integration_config):
     _store_headers(headers, span, integration_config, RESPONSE)
 
 
+def _sanitized_url(url):
+    # type: (str) -> str
+    """
+    Sanitize url by removing parts with potential auth info
+    """
+    if "@" in url:
+        parsed = parse.urlparse(url)
+        netloc = parsed.netloc
+        netloc = netloc[netloc.index("@") + 1 :]
+        return parse.urlunparse(
+            (
+                parsed.scheme,
+                netloc,
+                parsed.path,
+                "",
+                parsed.query,
+                "",
+            )
+        )
+
+    return url
+
+
 def with_traced_module(func):
     """Helper for providing tracing essentials (module and pin) for tracing
     wrappers.
@@ -246,6 +270,7 @@ def set_http_meta(
         span._set_str_tag(http.METHOD, method)
 
     if url is not None:
+        url = _sanitized_url(url)
         span._set_str_tag(http.URL, url if integration_config.trace_query_string else strip_query_string(url))
 
     if status_code is not None:

releasenotes/notes/fix-http-url-200fca099f41ce49.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    This fix removes unintended url parts in the ``http.url`` tag.

tests/contrib/aiohttp/test_aiohttp_client.py

@@ -16,7 +16,9 @@
 PORT = HTTPBIN_CONFIG["port"]
 SOCKET = "{}:{}".format(HOST, PORT)
 URL = "http://{}".format(SOCKET)
+URL_AUTH = "http://user:pass@{}".format(SOCKET)
 URL_200 = "{}/status/200".format(URL)
+URL_AUTH_200 = "{}/status/200".format(URL_AUTH)
 URL_500 = "{}/status/500".format(URL)
 
 
@@ -35,6 +37,14 @@ async def test_200_request(snapshot_context):
                 assert resp.status == 200
 
 
+@pytest.mark.asyncio
+async def test_auth_200_request(snapshot_context):
+    with snapshot_context():
+        async with aiohttp.ClientSession() as session:
+            async with session.get(URL_AUTH_200) as resp:
+                assert resp.status == 200
+
+
 @pytest.mark.asyncio
 async def test_200_request_post(snapshot_context):
     with snapshot_context():

tests/contrib/requests/test_requests.py

@@ -30,6 +30,7 @@
 SOCKET = "httpbin.org"
 URL_200 = "http://{}/status/200".format(SOCKET)
 URL_500 = "http://{}/status/500".format(SOCKET)
+URL_AUTH_200 = "http://user:pass@{}/status/200".format(SOCKET)
 
 
 class BaseRequestTestCase(object):
@@ -125,6 +126,13 @@ def test_200(self):
         assert s.span_type == "http"
         assert http.QUERY_STRING not in s.get_tags()
 
+    def test_auth_200(self):
+        self.session.get(URL_AUTH_200)
+        spans = self.pop_spans()
+        assert len(spans) == 1
+        s = spans[0]
+        assert s.get_tag(http.URL) == URL_200
+
     def test_200_send(self):
         # when calling send directly
         req = requests.Request(url=URL_200, method="GET")

tests/snapshots/tests.contrib.aiohttp.test_aiohttp_client.test_auth_200_request.json

@@ -0,0 +1,36 @@
+[[
+  {
+    "name": "aiohttp.request",
+    "service": null,
+    "resource": "aiohttp.request",
+    "trace_id": 0,
+    "span_id": 1,
+    "parent_id": 0,
+    "type": "http",
+    "meta": {
+      "http.method": "GET",
+      "http.status_code": "200",
+      "http.status_msg": "OK",
+      "http.url": "http://localhost:8001/status/200",
+      "runtime-id": "7a569f0d94574038b2b11ee3579d0936"
+    },
+    "metrics": {
+      "_dd.agent_psr": 1.0,
+      "_dd.top_level": 1,
+      "_dd.tracer_kr": 1.0,
+      "_sampling_priority_v1": 1,
+      "system.pid": 57194
+    },
+    "duration": 8933000,
+    "start": 1673646330398679000
+  },
+     {
+       "name": "TCPConnector.connect",
+       "service": null,
+       "resource": "TCPConnector.connect",
+       "trace_id": 0,
+       "span_id": 2,
+       "parent_id": 1,
+       "duration": 1426000,
+       "start": 1673646330399204000
+     }]]

tests/tracer/test_trace_utils.py

@@ -305,6 +305,30 @@ def test_ext_service(int_config, pin, config_val, default, expected):
         (None, None, None, None, None, None),
         ("GET", "http://localhost/", 200, "OK", None, {"my-header": "value1"}),
         ("GET", "http://localhost/", 200, "OK", "search?q=test+query", {"my-header": "value1"}),
+        (
+            "GET",
+            "http://user:pass@localhost/",
+            0,
+            None,
+            None,
+            None,
+        ),
+        (
+            "GET",
+            "http://user@localhost/",
+            0,
+            None,
+            None,
+            None,
+        ),
+        (
+            "GET",
+            "http://user:pass@localhost/api?q=test",
+            0,
+            None,
+            None,
+            None,
+        ),
     ],
 )
 def test_set_http_meta(span, int_config, method, url, status_code, status_msg, query, request_headers):
@@ -326,7 +350,13 @@ def test_set_http_meta(span, int_config, method, url, status_code, status_msg, q
         assert http.METHOD not in span.get_tags()
 
     if url is not None:
-        assert span.get_tag(http.URL) == stringify(url)
+        if url.startswith("http://user"):
+            # Remove any userinfo that may be in the original url
+            expected_url = url[: url.index(":")] + "://" + url[url.index("@") + 1 :]
+        else:
+            expected_url = url
+
+        assert span.get_tag(http.URL) == stringify(expected_url)
     else:
         assert http.URL not in span.get_tags()