fix(httplib): avoid tracing requests made by trace writers (#6635)

## Description
Avoids tracing http requests made by trace writers to datadog internal
services (i.e. prevents the ci visibility writer from sending traced
requests to internal services).

## Motivation

- Prevents tracing internals from being traced.
- Resolves flakiness in the httplib test suite with the agentless ci
visibility writer

## Checklist
- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Co-authored-by: Munir Abdinur <[email protected]>

ddtrace/contrib/httplib/patch.py

@@ -11,6 +11,7 @@
 from .. import trace_utils
 from ...constants import ANALYTICS_SAMPLE_RATE_KEY
 from ...constants import SPAN_KIND
+from ...internal.constants import _HTTPLIB_NO_TRACE_REQUEST
 from ...ext import SpanKind
 from ...ext import SpanTypes
 from ...internal.compat import PY2
@@ -183,9 +184,15 @@ def _wrap_putheader(func, instance, args, kwargs):
 
 def should_skip_request(pin, request):
     """Helper to determine if the provided request should be traced"""
+    if getattr(request, _HTTPLIB_NO_TRACE_REQUEST, False):
+        return True
+
     if not pin or not pin.enabled():
         return True
 
+    # httplib is used to send apm events (profiling,di, tracing, etc.) to the datadog agent
+    # Tracing these requests introduces a significant noise and instability in ddtrace tests.
+    # TO DO: Avoid tracing requests to APM internal services (ie: extend this functionality to agentless products).
     agent_url = pin.tracer.agent_trace_url
     if agent_url:
         parsed = parse.urlparse(agent_url)

ddtrace/internal/constants.py

@@ -66,3 +66,5 @@
 FLASK_ENDPOINT = "flask.endpoint"
 FLASK_VIEW_ARGS = "flask.view_args"
 FLASK_URL_RULE = "flask.url_rule"
+
+_HTTPLIB_NO_TRACE_REQUEST = "_dd_no_trace"

ddtrace/internal/writer/writer.py

@@ -33,6 +33,7 @@
 from .._encoding import BufferFull
 from .._encoding import BufferItemTooLarge
 from ..agent import get_connection
+from ..constants import _HTTPLIB_NO_TRACE_REQUEST
 from ..encoding import JSONEncoderV2
 from ..logger import get_logger
 from ..runtime import container
@@ -281,14 +282,15 @@ def _reset_connection(self):
                 self._conn.close()
                 self._conn = None
 
-    def _put(self, data, headers, client):
-        # type: (bytes, Dict[str, str], WriterClientBase) -> Response
+    def _put(self, data, headers, client, no_trace):
+        # type: (bytes, Dict[str, str], WriterClientBase, bool) -> Response
         sw = StopWatch()
         sw.start()
         with self._conn_lck:
             if self._conn is None:
                 log.debug("creating new intake connection to %s with timeout %d", self.intake_url, self._timeout)
                 self._conn = get_connection(self._intake_url(client), self._timeout)
+                setattr(self._conn, _HTTPLIB_NO_TRACE_REQUEST, no_trace)
             try:
                 log.debug("Sending request: %s %s %s", self.HTTP_METHOD, client.ENDPOINT, headers)
                 self._conn.request(
@@ -329,7 +331,7 @@ def _send_payload(self, payload, count, client):
 
         self._metrics_dist("http.requests")
 
-        response = self._put(payload, headers, client)
+        response = self._put(payload, headers, client, no_trace=True)
 
         if response.status >= 400:
             self._metrics_dist("http.errors", tags=("type:%s" % response.status,))

tests/ci_visibility/test_ci_visibility.py

@@ -438,7 +438,7 @@ def test_civisibilitywriter_coverage_agentless_url():
         assert cov_client._intake_url == "https://citestcov-intake.datadoghq.com"
 
         with mock.patch("ddtrace.internal.writer.writer.get_connection") as _get_connection:
-            dummy_writer._put("", {}, cov_client)
+            dummy_writer._put("", {}, cov_client, no_trace=True)
             _get_connection.assert_called_once_with("https://citestcov-intake.datadoghq.com", 2.0)
 
 
@@ -456,7 +456,7 @@ def test_civisibilitywriter_coverage_agentless_with_intake_url_param():
         assert cov_client._intake_url == "https://citestcov-intake.datadoghq.com"
 
         with mock.patch("ddtrace.internal.writer.writer.get_connection") as _get_connection:
-            dummy_writer._put("", {}, cov_client)
+            dummy_writer._put("", {}, cov_client, no_trace=True)
             _get_connection.assert_called_once_with("https://citestcov-intake.datadoghq.com", 2.0)
 
 
@@ -474,7 +474,7 @@ def test_civisibilitywriter_coverage_evp_proxy_url():
         assert cov_client.ENDPOINT == "/evp_proxy/v2/api/v2/citestcov"
 
         with mock.patch("ddtrace.internal.writer.writer.get_connection") as _get_connection:
-            dummy_writer._put("", {}, cov_client)
+            dummy_writer._put("", {}, cov_client, no_trace=True)
             _get_connection.assert_called_once_with("http://localhost:8126", 2.0)
 
 

tests/contrib/httplib/test_httplib.py

@@ -12,6 +12,7 @@
 from ddtrace.internal.compat import PY2
 from ddtrace.internal.compat import httplib
 from ddtrace.internal.compat import parse
+from ddtrace.internal.constants import _HTTPLIB_NO_TRACE_REQUEST
 from ddtrace.internal.schema import DEFAULT_SPAN_SERVICE_NAME
 from ddtrace.pin import Pin
 from ddtrace.vendor import wrapt
@@ -134,6 +135,24 @@ def test_should_skip_request(self):
         request = self.get_http_connection(parsed.hostname, parsed.port)
         pin = Pin.get_from(request)
         self.assertTrue(should_skip_request(pin, request))
+
+    def test_httplib_request_get_request_no_ddtrace(self):
+        """
+        When making a GET request via httplib.HTTPConnection.request
+            while setting _dd_no_trace attr to True
+                we do not capture any spans
+        """
+        self.tracer.enabled = True
+        conn = self.get_http_connection(SOCKET)
+        setattr(conn, _HTTPLIB_NO_TRACE_REQUEST, True)
+        with contextlib.closing(conn):
+            conn.request("GET", "/status/200")
+            resp = conn.getresponse()
+            self.assertEqual(self.to_str(resp.read()), "")
+            self.assertEqual(resp.status, 200)
+
+        spans = self.pop_spans()
+        self.assertEqual(len(spans), 0)
 
     def test_httplib_request_get_request(self, query_string=""):
         """

tests/utils.py

@@ -1192,7 +1192,7 @@ def flush_test_tracer_spans(writer):
         if encoded_traces is None:
             return
         headers = writer._get_finalized_headers(n_traces, client)
-        response = writer._put(encoded_traces, add_dd_env_variables_to_headers(headers), client)
+        response = writer._put(encoded_traces, add_dd_env_variables_to_headers(headers), client, no_trace=True)
     except Exception:
         return