Only call _store_request_headers/_store_response_headers if header tracing is enabled (#2550)

* Only call _store_request_headers/_store_response_headers if header tracing is enabled.
This was found during the investigation of #2549,
we only need to convert the headers to a `dict`
if there are traced headers configured

* properties are nice

* Add additional tests

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Kyle Verhoog <[email protected]>

Author: 3 people

ddtrace/contrib/trace_utils.py

@@ -250,10 +250,10 @@ def set_http_meta(
     if query is not None and integration_config.trace_query_string:
         span._set_str_tag(http.QUERY_STRING, query)
 
-    if request_headers is not None:
+    if request_headers is not None and integration_config.is_header_tracing_configured:
         _store_request_headers(dict(request_headers), span, integration_config)
 
-    if response_headers is not None:
+    if response_headers is not None and integration_config.is_header_tracing_configured:
         _store_response_headers(dict(response_headers), span, integration_config)
 
     if retries_remain is not None:

ddtrace/settings/integration.py

@@ -100,6 +100,16 @@ def trace_query_string(self):
             return self.http.trace_query_string
         return self.global_config.http.trace_query_string
 
+    @property
+    def is_header_tracing_configured(self):
+        # type: (...) -> bool
+        """Returns whether header tracing is enabled for this integration.
+
+        Will return true if traced headers are configured for this integration
+        or if they are configured globally.
+        """
+        return self.http.is_header_tracing_configured or self.global_config.http.is_header_tracing_configured
+
     def header_is_traced(self, header_name):
         """
         Returns whether or not the current header should be traced.

tests/tracer/test_settings.py

@@ -1,3 +1,5 @@
+import pytest
+
 from ddtrace.settings import Config
 from ddtrace.settings import HttpConfig
 from ddtrace.settings import IntegrationConfig
@@ -319,3 +321,31 @@ def test_service_name_env_var(self):
     def test_service_name_env_var_legacy(self):
         ic = IntegrationConfig(self.config, "foo")
         assert ic.service == "foo-svc"
+
+
+@pytest.mark.parametrize(
+    "global_headers,int_headers,expected",
+    (
+        (None, None, (False, False, False)),
+        ([], None, (False, False, False)),
+        (["Header"], None, (True, False, True)),
+        (None, ["Header"], (False, True, True)),
+        (None, [], (False, False, False)),
+        (["Header"], ["Header"], (True, True, True)),
+        ([], [], (False, False, False)),
+    ),
+)
+def test_config_is_header_tracing_configured(global_headers, int_headers, expected):
+    config = Config()
+    integration_config = config.myint
+
+    if global_headers is not None:
+        config.trace_headers(global_headers)
+    if int_headers is not None:
+        integration_config.http.trace_headers(int_headers)
+
+    assert (
+        config.http.is_header_tracing_configured,
+        integration_config.http.is_header_tracing_configured,
+        integration_config.is_header_tracing_configured,
+    ) == expected

tests/tracer/test_trace_utils.py

@@ -376,6 +376,21 @@ def test_set_http_meta_custom_errors(mock_log, span, int_config, error_codes, st
         mock_log.exception.assert_not_called()
 
 
+@mock.patch("ddtrace.contrib.trace_utils._store_headers")
+def test_set_http_meta_no_headers(mock_store_headers, span, int_config):
+    assert int_config.myint.is_header_tracing_configured is False
+    trace_utils.set_http_meta(
+        span,
+        int_config.myint,
+        request_headers={"HTTP_REQUEST_HEADER", "value"},
+        response_headers={"HTTP_RESPONSE_HEADER": "value"},
+    )
+    assert list(span.meta.keys()) == [
+        "runtime-id",
+    ]
+    mock_store_headers.assert_not_called()
+
+
 @mock.patch("ddtrace.contrib.trace_utils.log")
 @pytest.mark.parametrize(
     "val, bad",