feat(tracing): support http client error status configuration

ddtrace/contrib/internal/botocore/patch.py

@@ -27,7 +27,7 @@
 from ddtrace.internal.schema import schematize_cloud_messaging_operation
 from ddtrace.internal.schema import schematize_service_name
 from ddtrace.internal.settings import env
-from ddtrace.internal.settings._config import Config
+from ddtrace.internal.settings.http import _HTTPServerConfig
 from ddtrace.internal.utils import get_argument_value
 from ddtrace.internal.utils.formats import asbool
 from ddtrace.internal.utils.formats import deep_getattr
@@ -100,14 +100,14 @@ def _load_dynamodb_primary_key_names_for_tables() -> dict[str, set[str]]:
 config._add(
     "botocore",
     {
-        "_default_service": env.get("DD_BOTOCORE_SERVICE", default="aws"),
-        "distributed_tracing": asbool(env.get("DD_BOTOCORE_DISTRIBUTED_TRACING", default=True)),
-        "invoke_with_legacy_context": asbool(env.get("DD_BOTOCORE_INVOKE_WITH_LEGACY_CONTEXT", default=False)),
-        "operations": collections.defaultdict(Config._HTTPServerConfig),
-        "tag_no_params": asbool(env.get("DD_AWS_TAG_NO_PARAMS", default=False)),
-        "instrument_internals": asbool(env.get("DD_BOTOCORE_INSTRUMENT_INTERNALS", default=False)),
-        "propagation_enabled": asbool(env.get("DD_BOTOCORE_PROPAGATION_ENABLED", default=False)),
-        "empty_poll_enabled": asbool(env.get("DD_BOTOCORE_EMPTY_POLL_ENABLED", default=True)),
+        "_default_service": os.getenv("DD_BOTOCORE_SERVICE", default="aws"),
+        "distributed_tracing": asbool(os.getenv("DD_BOTOCORE_DISTRIBUTED_TRACING", default=True)),
+        "invoke_with_legacy_context": asbool(os.getenv("DD_BOTOCORE_INVOKE_WITH_LEGACY_CONTEXT", default=False)),
+        "operations": collections.defaultdict(_HTTPServerConfig),
+        "tag_no_params": asbool(os.getenv("DD_AWS_TAG_NO_PARAMS", default=False)),
+        "instrument_internals": asbool(os.getenv("DD_BOTOCORE_INSTRUMENT_INTERNALS", default=False)),
+        "propagation_enabled": asbool(os.getenv("DD_BOTOCORE_PROPAGATION_ENABLED", default=False)),
+        "empty_poll_enabled": asbool(os.getenv("DD_BOTOCORE_EMPTY_POLL_ENABLED", default=True)),
         "dynamodb_primary_key_names_for_tables": _load_dynamodb_primary_key_names_for_tables(),
         "add_span_pointers": asbool(env.get("DD_BOTOCORE_ADD_SPAN_POINTERS", default=True)),
         "payload_tagging_request": env.get("DD_TRACE_CLOUD_REQUEST_PAYLOAD_TAGGING", default=None),

ddtrace/contrib/internal/trace_utils.py

@@ -21,12 +21,15 @@
 from ddtrace._trace.pin import Pin
 from ddtrace._trace.span import Span
 from ddtrace.constants import _ORIGIN_KEY
+from ddtrace.constants import SPAN_KIND
 from ddtrace.contrib.internal.trace_utils_base import USER_AGENT_PATTERNS  # noqa:F401
 from ddtrace.contrib.internal.trace_utils_base import _get_header_value_case_insensitive
 from ddtrace.contrib.internal.trace_utils_base import _get_request_header_user_agent
 from ddtrace.contrib.internal.trace_utils_base import _normalize_tag_name
 from ddtrace.contrib.internal.trace_utils_base import _set_url_tag
 from ddtrace.contrib.internal.trace_utils_base import set_user  # noqa:F401
+from ddtrace.ext import SpanKind
+from ddtrace.ext import SpanTypes
 from ddtrace.ext import http
 from ddtrace.ext import net
 from ddtrace.internal import core
@@ -450,7 +453,23 @@ def set_http_meta(
             log.debug("failed to convert http status code %r to int", status_code)
         else:
             span._set_attribute(http.STATUS_CODE, str(status_code))
-            if config._http_server.is_error_code(int_status_code):
+            span_kind = span.get_tag(SPAN_KIND)
+            if span_kind == SpanKind.CLIENT:
+                http_config = config._http_client
+            elif span_kind == SpanKind.SERVER:
+                http_config = config._http_server
+            else:
+                log.warning(
+                    "span.kind not set on span %r, please set span.kind to '%s' or '%s'",
+                    span.name,
+                    SpanKind.CLIENT,
+                    SpanKind.SERVER,
+                )
+                if span.span_type == SpanTypes.HTTP:
+                    http_config = config._http_client
+                else:
+                    http_config = config._http_server
+            if http_config is not None and http_config.is_error_code(int_status_code):
                 span.error = 1
 
     if status_msg is not None:

ddtrace/internal/settings/_config.py

@@ -42,6 +42,8 @@
 from ._inferred_base_service import detect_service
 from .endpoint_config import fetch_config_from_endpoint
 from .http import HttpConfig
+from .http import _HTTPClientConfig
+from .http import _HTTPServerConfig
 from .integration import IntegrationConfig
 
 
@@ -274,23 +276,6 @@ def _deepmerge(source, destination):
     return destination
 
 
-def get_error_ranges(error_range_str: str) -> list[tuple[int, int]]:
-    error_ranges = []
-    error_range_str = error_range_str.strip()
-    error_ranges_str = error_range_str.split(",")
-    for error_range in error_ranges_str:
-        values = error_range.split("-")
-        try:
-            # Note: mypy does not like variable type changing
-            values = [int(v) for v in values]  # type: ignore[misc]
-        except ValueError:
-            log.exception("Error status codes was not a number %s", values)
-            continue
-        error_range = (min(values), max(values))  # type: ignore[assignment]
-        error_ranges.append(error_range)
-    return error_ranges  # type: ignore[return-value]
-
-
 _ConfigSource = Literal["default", "env_var", "code", "remote_config"]
 _JSONType = Union[None, int, float, str, bool, list["_JSONType"], dict[str, "_JSONType"]]
 
@@ -399,33 +384,6 @@ class Config(object):
     available and can be updated by users.
     """
 
-    class _HTTPServerConfig(object):
-        _error_statuses: str = _get_config("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500-599")
-        _error_ranges: list[tuple[int, int]] = get_error_ranges(_error_statuses)
-
-        @property
-        def error_statuses(self) -> str:
-            return self._error_statuses
-
-        @error_statuses.setter
-        def error_statuses(self, value: str) -> None:
-            self._error_statuses = value
-            self._error_ranges = get_error_ranges(value)
-            # Mypy can't catch cached method's invalidate()
-            self.is_error_code.cache_clear()  # type: ignore[attr-defined]
-
-        @property
-        def error_ranges(self) -> list[tuple[int, int]]:
-            return self._error_ranges
-
-        @cachedmethod()
-        def is_error_code(self, status_code: int) -> bool:
-            """Returns a boolean representing whether or not a status code is an error code."""
-            for error_range in self.error_ranges:
-                if error_range[0] <= status_code <= error_range[1]:
-                    return True
-            return False
-
     def __init__(self) -> None:
         # Must validate Otel configurations before creating the config object.
         validate_otel_envs()
@@ -508,7 +466,8 @@ def __init__(self) -> None:
 
         self._extra_services: set[str] = set()
         self.version = _get_config("DD_VERSION", self.tags.get("version"))
-        self._http_server = self._HTTPServerConfig()
+        self._http_server = _HTTPServerConfig()
+        self._http_client = _HTTPClientConfig()
 
         self._extra_services_sent: set[str] = set()
         self._extra_services_queue = None

ddtrace/internal/settings/http.py

@@ -3,13 +3,89 @@
 from typing import Union  # noqa:F401
 
 from ddtrace.internal.logger import get_logger
+from ddtrace.internal.telemetry import get_config as _get_config
 from ddtrace.internal.utils.cache import cachedmethod
+from ddtrace.internal.utils.deprecations import DDTraceDeprecationWarning
 from ddtrace.internal.utils.http import normalize_header_name
+from ddtrace.vendor.debtcollector import deprecate
 
 
 log = get_logger(__name__)
 
 
+def get_error_ranges(error_range_str: str) -> list[tuple[int, int]]:
+    error_ranges = []
+    error_range_str = error_range_str.strip()
+    error_ranges_str = error_range_str.split(",")
+    for error_range in error_ranges_str:
+        values = error_range.split("-")
+        try:
+            # Note: mypy does not like variable type changing
+            values = [int(v) for v in values]  # type: ignore[misc]
+        except ValueError:
+            log.exception("Error status codes was not a number %s", values)
+            continue
+        error_range = (min(values), max(values))  # type: ignore[assignment]
+        error_ranges.append(error_range)
+    return error_ranges  # type: ignore[return-value]
+
+
+class _HTTPBaseConfig(object):
+    _error_statuses: str
+    _error_ranges: list[tuple[int, int]]
+
+    @property
+    def error_statuses(self) -> str:
+        return self._error_statuses
+
+    @error_statuses.setter
+    def error_statuses(self, value: str) -> None:
+        self._error_statuses = value
+        self._error_ranges = get_error_ranges(value)
+        # Mypy can't catch cached method's invalidate()
+        self.is_error_code.cache_clear()  # type: ignore[attr-defined]
+
+    @property
+    def error_ranges(self) -> list[tuple[int, int]]:
+        return self._error_ranges
+
+    @cachedmethod()
+    def is_error_code(self, status_code: int) -> bool:
+        """Returns a boolean representing whether or not a status code is an error code."""
+        for error_range in self.error_ranges:
+            if error_range[0] <= status_code <= error_range[1]:
+                return True
+        return False
+
+
+class _HTTPServerConfig(_HTTPBaseConfig):
+    _error_statuses: str = _get_config("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500-599")
+    _error_ranges: list[tuple[int, int]] = get_error_ranges(_error_statuses)
+
+
+class _HTTPClientConfig(_HTTPBaseConfig):
+    def __init__(self) -> None:
+        _unset = object()
+        client_configured = _get_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES", _unset, report_telemetry=False)
+        if client_configured is not _unset:
+            self._error_statuses = _get_config("DD_TRACE_HTTP_CLIENT_ERROR_STATUSES", "500-599")
+        else:
+            server_configured = _get_config("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", _unset, report_telemetry=False)
+            if server_configured is not _unset:
+                deprecate(
+                    "DD_TRACE_HTTP_SERVER_ERROR_STATUSES is being applied to HTTP client spans.",
+                    message=(
+                        "Set DD_TRACE_HTTP_CLIENT_ERROR_STATUSES explicitly to configure client span error statuses. "
+                        "In a future version, client spans will default to '500-599' independently of "
+                        "DD_TRACE_HTTP_SERVER_ERROR_STATUSES."
+                    ),
+                    removal_version="5.0.0",
+                    category=DDTraceDeprecationWarning,
+                )
+            self._error_statuses = _get_config("DD_TRACE_HTTP_SERVER_ERROR_STATUSES", "500-599")
+        self._error_ranges = get_error_ranges(self._error_statuses)
+
+
 class HttpConfig(object):
     """
     Configuration object that expose an API to set and retrieve both global and integration specific settings

docs/configuration.rst

@@ -216,12 +216,21 @@ Traces
      default: True
      description: Send query strings in http.url tag in http client integrations.
 
+   DD_TRACE_HTTP_CLIENT_ERROR_STATUSES:
+     type: String
+     default: "500-599"
+
+     description: |
+        Comma-separated list of HTTP status codes that should be considered errors for HTTP client spans.
+        Multiple comma separated error ranges can be set (ex:  ``200,400-404,500-599``).
+        The status codes are used to set the ``error`` field on the span.
+
    DD_TRACE_HTTP_SERVER_ERROR_STATUSES:
      type: String
      default: "500-599"
 
      description: |
-        Comma-separated list of HTTP status codes that should be considered errors when returned by an HTTP request.
+        Comma-separated list of HTTP status codes that should be considered errors for HTTP server spans.
         Multiple comma separated error ranges can be set (ex:  ``200,400-404,500-599``).
         The status codes are used to set the ``error`` field on the span.
 

releasenotes/notes/add-http-client-error-codes-8a5f6510061df45f.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    tracing: Adds ``DD_TRACE_HTTP_CLIENT_ERROR_STATUSES`` environment variable to configure list of HTTP status codes that should be considered errors when instrumenting HTTP clients.

releasenotes/notes/support_http_client_error_status_env-acdf049c64330a20.yaml

@@ -1,4 +1,4 @@
 ---
 features:
   - |
-    tracing: Adds ``DD_TRACE_HTTP_CLIENT_ERROR_STATUSES`` environment variable to configure the list of HTTP status codes that should be considered errors when instrumenting HTTP severs.
+    tracing: Adds ``DD_TRACE_HTTP_SERVER_ERROR_STATUSES`` environment variable to configure the list of HTTP status codes that should be considered errors when instrumenting HTTP servers.

tests/contrib/httplib/test_httplib.py

@@ -303,6 +303,37 @@ def test_httplib_request_non_200_request(self):
         assert_span_http_status_code(span, 404)
         self.assertEqual(span.get_tag("http.url"), URL_404)
 
+    def test_httplib_request_custom_client_error_statuses(self):
+        """
+        When making a GET request via httplib.HTTPConnection.request
+            when DD_TRACE_HTTP_CLIENT_ERROR_STATUSES includes 4xx
+                we mark the span as an error for a 404 response
+                we capture the correct span tags
+        """
+        original = config._http_client.error_statuses
+        try:
+            config._http_client.error_statuses = "400-499"
+            conn = self.get_http_connection(SOCKET)
+            with contextlib.closing(conn):
+                conn.request("GET", "/status/404")
+                conn.getresponse()
+            spans = self.pop_spans()
+            self.assertEqual(len(spans), 1)
+            span = spans[0]
+            self.assert_is_not_measured(span)
+            self.assertEqual(span.span_type, "http")
+            self.assertEqual(span.service, "tests.contrib.httplib")
+            self.assertEqual(span.name, self.SPAN_NAME)
+            self.assertEqual(span.error, 1)
+            self.assertEqual(span.get_tag("http.method"), "GET")
+            self.assertEqual(span.get_tag("component"), "httplib")
+            self.assertEqual(span.get_tag("span.kind"), "client")
+            self.assertEqual(span.get_tag("out.host"), "localhost")
+            assert_span_http_status_code(span, 404)
+            self.assertEqual(span.get_tag("http.url"), URL_404)
+        finally:
+            config._http_client.error_statuses = original
+
     def test_httplib_request_get_request_disabled(self):
         """
         When making a GET request via httplib.HTTPConnection.request

tests/contrib/httpx/test_httpx.py

@@ -414,6 +414,29 @@ async def test_get_500(snapshot_context):
             assert resp.status_code == 500
 
 
+@pytest.mark.asyncio
+async def test_custom_client_error_statuses(snapshot_context):
+    """
+    When DD_TRACE_HTTP_CLIENT_ERROR_STATUSES includes 4xx
+        we mark the span as an error for a 404 response
+    """
+    original = config._http_client.error_statuses
+    try:
+        config._http_client.error_statuses = "400-499"
+        url = get_url("/status/404")
+
+        with snapshot_context(wait_for_num_traces=1):
+            resp = httpx.get(url, headers=DEFAULT_HEADERS)
+            assert resp.status_code == 404
+
+        with snapshot_context(wait_for_num_traces=1):
+            async with httpx.AsyncClient() as client:
+                resp = await client.get(url, headers=DEFAULT_HEADERS)
+                assert resp.status_code == 404
+    finally:
+        config._http_client.error_statuses = original
+
+
 @pytest.mark.asyncio
 async def test_split_by_domain(snapshot_context):
     """

tests/snapshots/tests.contrib.httpx.test_httpx.test_custom_client_error_statuses.json

@@ -0,0 +1,34 @@
+[[
+  {
+    "name": "http.request",
+    "service": "tests.contrib.httpx",
+    "resource": "http.request",
+    "trace_id": 0,
+    "span_id": 1,
+    "parent_id": 0,
+    "type": "http",
+    "error": 1,
+    "meta": {
+      "_dd.p.dm": "-0",
+      "_dd.p.tid": "69c6d8aa00000000",
+      "_dd.tags.process": "entrypoint.name:pytest,entrypoint.type:script,entrypoint.workdir:project,svc.auto:tests.contrib.httpx",
+      "component": "httpx",
+      "http.method": "GET",
+      "http.status_code": "404",
+      "http.url": "http://localhost:8001/status/404",
+      "http.useragent": "python-httpx/x.xx.x",
+      "language": "python",
+      "out.host": "localhost",
+      "runtime-id": "90b215813a13422b9ee8d6073de1d5d8",
+      "span.kind": "client"
+    },
+    "metrics": {
+      "_dd.measured": 1.0,
+      "_dd.top_level": 1.0,
+      "_dd.tracer_kr": 1.0,
+      "_sampling_priority_v1": 1.0,
+      "process_id": 21766.0
+    },
+    "duration": 7996583,
+    "start": 1774639274668166804
+  }]]