Merge branch 'main' into cbeauchesne/crash-investigation

Author: juanjux

.gitlab/benchmarks.yml

@@ -33,6 +33,7 @@ variables:
     UPSTREAM_COMMIT_SHA: $CI_COMMIT_SHA # The commit revision the project is built for.
     KUBERNETES_SERVICE_ACCOUNT_OVERWRITE: dd-trace-py
     FF_USE_LEGACY_KUBERNETES_EXECUTION_STRATEGY: "true"
+    CARGO_NET_GIT_FETCH_WITH_CLI: "true" # use system git binary to pull git dependencies
 
 benchmarks-pr-comment:
   stage: benchmarks-pr-comment

ddtrace/_trace/_span_link.py

@@ -109,3 +109,10 @@ def to_dict(self):
             d["flags"] = self.flags
 
         return d
+
+    def __str__(self) -> str:
+        attrs_str = ",".join([f"{k}:{v}" for k, v in self.attributes.items()])
+        return (
+            f"trace_id={self.trace_id} span_id={self.span_id} attributes={attrs_str} "
+            f"tracestate={self.tracestate} flags={self.flags} dropped_attributes={self._dropped_attributes}"
+        )

ddtrace/_trace/span.py

@@ -72,6 +72,10 @@ def __dict__(self):
             d["attributes"] = self.attributes
         return d
 
+    def __str__(self):
+        attrs_str = ",".join([f"{k}:{v}" for k, v in self.attributes.items()])
+        return f"name={self.name} time={self.time_unix_nano} attributes={attrs_str}"
+
 
 log = get_logger(__name__)
 
@@ -569,6 +573,8 @@ def _pprint(self) -> str:
             ("error", self.error),
             ("tags", dict(sorted(self._meta.items()))),
             ("metrics", dict(sorted(self._metrics.items()))),
+            ("links", ", ".join([str(link) for _, link in self._links.items()])),
+            ("events", ", ".join([str(e) for e in self._events])),
         ]
         return " ".join(
             # use a large column width to keep pprint output on one line

ddtrace/contrib/trace_utils.py

@@ -144,7 +144,7 @@ def _store_headers(headers, span, integration_config, request_or_response):
         return
 
     for header_name, header_value in headers.items():
-        """config._header_tag_name gets an element of the dictionary in config.http._header_tags
+        """config._header_tag_name gets an element of the dictionary in config.trace_http_header_tags
         which gets the value from DD_TRACE_HEADER_TAGS environment variable."""
         tag_name = integration_config._header_tag_name(header_name)
         if tag_name is None:

ddtrace/internal/utils/formats.py

@@ -70,6 +70,8 @@ def parse_tags_str(tags_str):
     The expected string is of the form::
         "key1:value1,key2:value2"
         "key1:value1 key2:value2"
+        "key1,key2"
+        "key1 key2"
 
     :param tags_str: A string of the above form to parse tags from.
     :return: A dict containing the tags that were parsed.
@@ -86,10 +88,14 @@ def parse_tags(tags):
 
         for tag in tags:
             key, sep, value = tag.partition(":")
-            if not sep or not key or "," in key:
+            if not key.strip() or "," in key or (sep and not value):
                 invalids.append(tag)
-            else:
+            elif sep:
+                # parse key:val,key2:value2
                 parsed_tags.append((key, value))
+            else:
+                # parse key,key2
+                parsed_tags.append((key, ""))
 
         return parsed_tags, invalids
 

ddtrace/settings/crashtracker.py

@@ -5,12 +5,19 @@
 from ddtrace.internal.utils.formats import parse_tags_str
 
 
+resolver_default = "full"
+
+
 def _derive_stacktrace_resolver(config: "CrashtrackerConfig") -> t.Optional[str]:
     resolver = str(config._stacktrace_resolver or "")
     resolver = resolver.lower()
+    if resolver == "none":
+        return None
     if resolver in ("fast", "full", "safe"):
         return resolver
-    return None
+
+    # Invalid values should degrade to the default
+    return resolver_default
 
 
 def _check_for_crashtracker_available() -> bool:
@@ -75,10 +82,10 @@ class CrashtrackerConfig(En):
     _stacktrace_resolver = En.v(
         t.Optional[str],
         "stacktrace_resolver",
-        default=None,
+        default=resolver_default,
         help_type="String",
         help="How to collect native stack traces during a crash, if at all.  Accepted values are 'none', 'fast',"
-        " 'safe', and 'full'.  The default value is 'none' (no stack traces).",
+        " 'safe', and 'full'.  The default value is '" + resolver_default + "'.",
     )
     stacktrace_resolver = En.d(t.Optional[str], _derive_stacktrace_resolver)
 

docs/configuration.rst

@@ -31,6 +31,8 @@ The following environment variables for the tracer are supported:
    DD_TAGS:
      description: |
          Set global tags to be attached to every span. Value must be either comma or space separated. e.g. ``key1:value1,key2:value2`` or ``key1:value key2:value2``.
+
+         If a tag value is not supplied the value will be an empty string. e.g. ``key1,key2`` or ``key1 key2``.
      version_added:
        v0.38.0: Comma separated support added
        v0.48.0: Space separated support added
@@ -290,9 +292,11 @@ The following environment variables for the tracer are supported:
 
    DD_TRACE_HEADER_TAGS:
      description: |
-         A map of case-insensitive header keys to tag names. Automatically applies matching header values as tags on root spans.
+         A map of case-insensitive http headers to tag names. Automatically applies matching header values as tags on request and response spans. For example if
+         ``DD_TRACE_HEADER_TAGS=User-Agent:http.useragent,content-type:http.content_type``. The value of the header will be stored in tags with the name ``http.useragent`` and ``http.content_type``.
 
-         For example, ``User-Agent:http.useragent,content-type:http.content_type``.
+         If a tag name is not supplied the header name will be used. For example if
+         ``DD_TRACE_HEADER_TAGS=User-Agent,content-type``. The value of http header will be stored in tags with the names ``http.<response/request>.headers.user-agent`` and ``http.<response/request>.headers.content-type``.
 
    DD_TRACE_API_VERSION:
      default: |

releasenotes/notes/list_dd_header_tags-f8a9ce1daa2a1cf6.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    tracing: Updates ``DD_HEADER_TAGS`` and ``DD_TAGS`` to support the following formats:
+    ``key1,key2,key3``,  ``key1:val,key2:val,key3:val3``, ``key1:val key2:val key3:val3``, and ``key1 key2 key3``.
+    Key value pairs that do not match an expected format will be logged and ignored by the tracer.

src/core/Cargo.toml

@@ -21,3 +21,9 @@ pyo3-build-config = "0.21.2"
 name = "_core"
 path = "lib.rs"
 crate-type = ["cdylib"]
+
+
+[net]
+# Use git binary from the system instead of the built-in git client
+# "Setting this to true can be helpful if you have special authentication requirements that Cargo does not support."
+git-fetch-with-cli = true

tests/internal/crashtracker/test_crashtracker.py

@@ -306,7 +306,9 @@ def test_crashtracker_preload_default(ddtrace_run_python_code_in_subprocess):
     conn = utils.listen_get_conn(sock)
     assert conn
     data = utils.conn_to_bytes(conn)
+    conn.close()
     assert data
+    assert b"string_at" in data
 
 
 @pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
@@ -359,7 +361,35 @@ def test_crashtracker_auto_default(run_python_code_in_subprocess):
     conn = utils.listen_get_conn(sock)
     assert conn
     data = utils.conn_to_bytes(conn)
+    conn.close()
+    assert data
+    assert b"string_at" in data
+
+
+@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
+def test_crashtracker_auto_nostack(run_python_code_in_subprocess):
+    # Setup the listening socket before we open ddtrace
+    port, sock = utils.crashtracker_receiver_bind()
+    assert sock
+
+    # Call the program
+    env = os.environ.copy()
+    env["DD_TRACE_AGENT_URL"] = "http://localhost:%d" % port
+    env["DD_CRASHTRACKER_STACKTRACE_RESOLVER"] = "none"
+    stdout, stderr, exitcode, _ = run_python_code_in_subprocess(auto_code, env=env)
+
+    # Check for expected exit condition
+    assert not stdout
+    assert not stderr
+    assert exitcode == -11
+
+    # Wait for the connection
+    conn = utils.listen_get_conn(sock)
+    assert conn
+    data = utils.conn_to_bytes(conn)
+    conn.close()
     assert data
+    assert b"string_at" not in data
 
 
 @pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")

tests/internal/test_settings.py

@@ -103,6 +103,17 @@ def _deleted_rc_config():
             "expected": {"trace_http_header_tags": {"header": "value"}},
             "expected_source": {"trace_http_header_tags": "code"},
         },
+        {
+            "env": {"DD_TRACE_HEADER_TAGS": "X-Header-Tag-1,X-Header-Tag-2,X-Header-Tag-3:specific_tag3"},
+            "expected": {
+                "trace_http_header_tags": {
+                    "X-Header-Tag-1": "",
+                    "X-Header-Tag-2": "",
+                    "X-Header-Tag-3": "specific_tag3",
+                }
+            },
+            "expected_source": {"trace_http_header_tags": "env_var"},
+        },
         {
             "env": {"DD_TRACE_HEADER_TAGS": "X-Header-Tag-1:header_tag_1,X-Header-Tag-2:header_tag_2"},
             "rc": {

tests/telemetry/test_writer.py

@@ -133,7 +133,7 @@ def test_app_started_event(telemetry_writer, test_agent_session, mock_time):
                     {"name": "crashtracking_available", "origin": "unknown", "value": sys.platform == "linux"},
                     {"name": "crashtracking_debug_url", "origin": "unknown", "value": "None"},
                     {"name": "crashtracking_enabled", "origin": "unknown", "value": sys.platform == "linux"},
-                    {"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "None"},
+                    {"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "full"},
                     {"name": "crashtracking_started", "origin": "unknown", "value": False},
                     {"name": "crashtracking_stderr_filename", "origin": "unknown", "value": "None"},
                     {"name": "crashtracking_stdout_filename", "origin": "unknown", "value": "None"},
@@ -323,7 +323,7 @@ def test_app_started_event_configuration_override(
             {"name": "crashtracking_available", "origin": "unknown", "value": sys.platform == "linux"},
             {"name": "crashtracking_debug_url", "origin": "unknown", "value": "None"},
             {"name": "crashtracking_enabled", "origin": "unknown", "value": sys.platform == "linux"},
-            {"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "None"},
+            {"name": "crashtracking_stacktrace_resolver", "origin": "unknown", "value": "full"},
             {"name": "crashtracking_started", "origin": "unknown", "value": sys.platform == "linux"},
             {"name": "crashtracking_stderr_filename", "origin": "unknown", "value": "None"},
             {"name": "crashtracking_stdout_filename", "origin": "unknown", "value": "None"},

tests/tracer/test_span.py

@@ -695,6 +695,9 @@ def test_span_pprint():
     root = Span("test.span", service="s", resource="r", span_type=SpanTypes.WEB)
     root.set_tag("t", "v")
     root.set_metric("m", 1.0)
+    root._add_event("message", {"importance": 10}, 16789898242)
+    root.set_link(trace_id=99, span_id=10, attributes={"link.name": "s1_to_s2", "link.kind": "scheduled_by"})
+
     root.finish()
     actual = root._pprint()
     assert "name='test.span'" in actual
@@ -704,6 +707,11 @@ def test_span_pprint():
     assert "error=0" in actual
     assert "tags={'t': 'v'}" in actual
     assert "metrics={'m': 1.0}" in actual
+    assert "events='name=message time=16789898242 attributes=importance:10'" in actual
+    assert (
+        "links='trace_id=99 span_id=10 attributes=link.name:s1_to_s2,link.kind:scheduled_by "
+        "tracestate=None flags=None dropped_attributes=0'" in actual
+    )
     assert re.search("id=[0-9]+", actual) is not None
     assert re.search("trace_id=[0-9]+", actual) is not None
     assert "parent_id=None" in actual

tests/tracer/test_trace_utils.py

@@ -306,6 +306,47 @@ def test_ext_service(int_config, pin, config_val, default, expected):
     assert trace_utils.ext_service(pin, int_config.myint, default) == expected
 
 
+@pytest.mark.subprocess(
+    parametrize={
+        "DD_TRACE_HEADER_TAGS": ["header1 header2 header3:third-header", "header1,header2,header3:third-header"]
+    }
+)
+def test_set_http_meta_with_http_header_tags_config():
+    from ddtrace import config
+    from ddtrace._trace.span import Span
+    from ddtrace.contrib.trace_utils import set_http_meta
+
+    assert config.trace_http_header_tags == {
+        "header1": "",
+        "header2": "",
+        "header3": "third-header",
+    }, config.trace_http_header_tags
+    integration_config = config.new_integration
+    assert integration_config.is_header_tracing_configured
+
+    # test request headers
+    request_span = Span(name="new_integration.request")
+    set_http_meta(
+        request_span,
+        integration_config,
+        request_headers={"header1": "value1", "header2": "value2", "header3": "value3"},
+    )
+    assert request_span.get_tag("http.request.headers.header1") == "value1"
+    assert request_span.get_tag("http.request.headers.header2") == "value2"
+    assert request_span.get_tag("third-header") == "value3"
+
+    # test response headers
+    response_span = Span(name="new_integration.response")
+    set_http_meta(
+        response_span,
+        integration_config,
+        response_headers={"header1": "value1", "header2": "value2", "header3": "value3"},
+    )
+    assert response_span.get_tag("http.response.headers.header1") == "value1"
+    assert response_span.get_tag("http.response.headers.header2") == "value2"
+    assert response_span.get_tag("third-header") == "value3"
+
+
 @pytest.mark.parametrize("appsec_enabled", [False, True])
 @pytest.mark.parametrize("span_type", [SpanTypes.WEB, SpanTypes.HTTP, None])
 @pytest.mark.parametrize(

tests/tracer/test_tracer.py

@@ -979,11 +979,11 @@ def test_dd_tags(self):
         assert self.tracer._tags.get("key1") == "value1"
         assert self.tracer._tags.get("key2") == "value2"
 
-    @run_in_subprocess(env_overrides=dict(DD_TAGS="key1:value1,key2:value2,key3"))
+    @run_in_subprocess(env_overrides=dict(DD_TAGS="key1:value1,key2:value2, key3"))
     def test_dd_tags_invalid(self):
         assert self.tracer._tags.get("key1")
         assert self.tracer._tags.get("key2")
-        assert self.tracer._tags.get("key3") is None
+        assert not self.tracer._tags.get("key3")
 
     @run_in_subprocess(env_overrides=dict(DD_TAGS="service:mysvc,env:myenv,version:myvers"))
     def test_tags_from_DD_TAGS(self):

tests/tracer/test_utils.py

@@ -56,33 +56,46 @@ def test_asbool(self):
         ("key:val,key2:val2,key3:1234.23", dict(key="val", key2="val2", key3="1234.23"), None),
         ("key:val key2:val2 key3:1234.23", dict(key="val", key2="val2", key3="1234.23"), None),
         ("key: val", dict(key=" val"), None),
-        ("key key: val", {"key key": " val"}, None),
+        (
+            "key key: val",
+            {"key": "", "val": ""},
+            [mock.call(_LOG_ERROR_MALFORMED_TAG, "key:", "key key: val")],
+        ),
         ("key: val,key2:val2", dict(key=" val", key2="val2"), None),
         (" key: val,key2:val2", {"key": " val", "key2": "val2"}, None),
-        ("key key2:val1", {"key key2": "val1"}, None),
+        ("key key2:val1", {"key": "", "key2": "val1"}, None),
         ("key:val key2:val:2", {"key": "val", "key2": "val:2"}, None),
         (
             "key:val,key2:val2 key3:1234.23",
             dict(),
             [mock.call(_LOG_ERROR_FAIL_SEPARATOR, "key:val,key2:val2 key3:1234.23")],
         ),
-        ("key:val key2:val2 key3: ", dict(key="val", key2="val2", key3=""), None),
+        (
+            "key:val key2:val2 key3: ",
+            {"key": "val", "key2": "val2"},
+            [mock.call(_LOG_ERROR_MALFORMED_TAG, "key3:", "key:val key2:val2 key3:")],
+        ),
         (
             "key:val key2:val 2",
-            dict(key="val", key2="val"),
-            [mock.call(_LOG_ERROR_MALFORMED_TAG, "2", "key:val key2:val 2")],
+            {"2": "", "key": "val", "key2": "val"},
+            None,
         ),
         (
             "key: val key2:val2 key3:val3",
-            {"key": "", "key2": "val2", "key3": "val3"},
-            [mock.call(_LOG_ERROR_MALFORMED_TAG, "val", "key: val key2:val2 key3:val3")],
+            {"key2": "val2", "key3": "val3", "val": ""},
+            [mock.call(_LOG_ERROR_MALFORMED_TAG, "key:", "key: val key2:val2 key3:val3")],
+        ),
+        (
+            "key:,key3:val1,",
+            {"key3": "val1"},
+            [mock.call(_LOG_ERROR_MALFORMED_TAG, "key:", "key:,key3:val1")],
         ),
-        ("key:,key3:val1,", dict(key3="val1", key=""), None),
         (",", dict(), [mock.call(_LOG_ERROR_FAIL_SEPARATOR, "")]),
         (":,:", dict(), [mock.call(_LOG_ERROR_FAIL_SEPARATOR, ":,:")]),
-        ("key,key2:val1", {"key2": "val1"}, [mock.call(_LOG_ERROR_MALFORMED_TAG, "key", "key,key2:val1")]),
+        ("key,key2:val1", {"key": "", "key2": "val1"}, None),
         ("key2:val1:", {"key2": "val1:"}, None),
-        ("key,key2,key3", dict(), [mock.call(_LOG_ERROR_FAIL_SEPARATOR, "key,key2,key3")]),
+        ("key,key2,key3", {"key": "", "key2": "", "key3": ""}, None),
+        ("key key2 key3", {"key": "", "key2": "", "key3": ""}, None),
         ("foo:bar,foo:baz", dict(foo="baz"), None),
         ("hash:asd url:https://github.com/foo/bar", dict(hash="asd", url="https://github.com/foo/bar"), None),
     ],