chore(profiling): add interface for crashtracker tags (#9892)

Adds an interface and a test for crashtracker tags

1. can set with crashtracer.add_tag(key, val)
2. can set with DD_CRASHTRACKER_TAGS in the normal way

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- 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)

ddtrace/internal/core/crashtracking.py

@@ -19,6 +19,11 @@ def _update_runtime_id(runtime_id: str) -> None:
     crashtracker.set_runtime_id(runtime_id)
 
 
+def add_tag(key: str, value: str) -> None:
+    if is_available:
+        crashtracker.set_tag(key, value)
+
+
 def start() -> bool:
     if not is_available:
         return False
@@ -44,6 +49,10 @@ def start() -> bool:
     if crashtracker_config.stderr_filename:
         crashtracker.set_stderr_filename(crashtracker_config.stderr_filename)
 
+    # Add user tags
+    for key, value in crashtracker_config.tags.items():
+        add_tag(key, value)
+
     # Only start if it is enabled
     if crashtracker_config.enabled:
         return crashtracker.start()

ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyi

@@ -17,5 +17,6 @@ def set_resolve_frames_full() -> None: ...
 def set_profiling_state_sampling(on: bool) -> None: ...
 def set_profiling_state_unwinding(on: bool) -> None: ...
 def set_profiling_state_serializing(on: bool) -> None: ...
+def set_tag(key: StringType, value: StringType) -> None: ...
 def start() -> bool: ...
 def is_started() -> bool: ...

ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx

@@ -33,6 +33,7 @@ cdef extern from "crashtracker_interface.hpp":
     void crashtracker_set_resolve_frames_full()
     void crashtracker_set_resolve_frames_safe()
     bint crashtracker_set_receiver_binary_path(string_view path)
+    void crashtracker_set_tag(string_view key, string_view value)
     void crashtracker_profiling_state_sampling_start()
     void crashtracker_profiling_state_sampling_stop()
     void crashtracker_profiling_state_unwinding_start()
@@ -134,6 +135,15 @@ def set_profiling_state_serializing(on: bool) -> None:
         crashtracker_profiling_state_serializing_stop()
 
 
+def set_tag(key: StringType, value: StringType) -> None:
+    key_bytes = ensure_binary_or_empty(key)
+    value_bytes = ensure_binary_or_empty(value)
+    crashtracker_set_tag(
+        string_view(<const char*>key_bytes, len(key_bytes)),
+        string_view(<const char*>value_bytes, len(value_bytes))
+    )
+
+
 def start() -> bool:
     # The file is "crashtracker_exe" in the same directory as the libdd_wrapper.so
     exe_dir = os.path.dirname(__file__)

ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker.hpp

@@ -7,6 +7,7 @@
 #include <optional>
 #include <string>
 #include <string_view>
+#include <unordered_map>
 
 namespace Datadog {
 
@@ -46,6 +47,8 @@ class Crashtracker
     std::string library_version;
     std::string url;
 
+    std::unordered_map<std::string, std::string> user_tags;
+
     static constexpr std::string_view family{ g_language_name };
     static constexpr std::string_view library_name{ g_library_name };
 
@@ -65,6 +68,7 @@ class Crashtracker
     void set_runtime_version(std::string_view _runtime_version);
     void set_library_version(std::string_view _library_version);
     void set_url(std::string_view _url);
+    void set_tag(std::string_view _key, std::string_view _value);
 
     void set_create_alt_stack(bool _create_alt_stack);
     void set_stderr_filename(std::string_view _stderr_filename);

ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp

@@ -24,6 +24,7 @@ extern "C"
     void crashtracker_set_resolve_frames_full();
     void crashtracker_set_resolve_frames_safe();
     bool crashtracker_set_receiver_binary_path(std::string_view path);
+    void crashtracker_set_tag(std::string_view key, std::string_view value);
     void crashtracker_profiling_state_sampling_start();
     void crashtracker_profiling_state_sampling_stop();
     void crashtracker_profiling_state_unwinding_start();

ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker.cpp

@@ -86,6 +86,16 @@ Datadog::Crashtracker::set_library_version(std::string_view _library_version)
     library_version = std::string(_library_version);
 }
 
+void
+Datadog::Crashtracker::set_tag(std::string_view key, std::string_view value)
+{
+    // Maybe this should be called "add tag," but the interface to the uploader is already called "set_tag"
+    // and we have another refactor incoming anyway, so let's just kick the can for now
+    if (!key.empty() && !value.empty()) {
+        user_tags[std::string(key)] = std::string(value);
+    }
+}
+
 bool
 Datadog::Crashtracker::set_receiver_binary_path(std::string_view _path)
 {
@@ -147,10 +157,19 @@ Datadog::Crashtracker::get_tags()
         { ExportTagKey::library_version, library_version },
     };
 
+    // Add system tags
     std::string errmsg; // Populated, but unused
-    for (const auto& [tag, data] : tag_data) {
-        if (!data.empty()) {
-            add_tag(tags, tag, data, errmsg); // We don't have a good way of handling errors here
+    for (const auto& [key, value] : tag_data) {
+        // NB - keys here are members of an enum; `add_tag()` specialization below will stringify them
+        if (!value.empty()) {
+            add_tag(tags, key, value, errmsg); // We don't have a good way of handling errors here
+        }
+    }
+
+    // Add user tags
+    for (const auto& [key, value] : user_tags) {
+        if (!key.empty() && !value.empty()) {
+            add_tag(tags, key, value, errmsg);
         }
     }
 

ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp

@@ -112,6 +112,12 @@ crashtracker_set_receiver_binary_path(std::string_view path) // cppcheck-suppres
     return crashtracker.set_receiver_binary_path(path);
 }
 
+void
+crashtracker_set_tag(std::string_view key, std::string_view value) // cppcheck-suppress unusedFunction
+{
+    crashtracker.set_tag(key, value);
+}
+
 // Store the old segfault handler (uses sigaction prototype)
 void (*old_sigsegv_handler)(int, siginfo_t*, void*) = nullptr;
 void (*old_sigbus_handler)(int, siginfo_t*, void*) = nullptr;

ddtrace/settings/crashtracker.py

@@ -2,6 +2,8 @@
 
 from envier import En
 
+from ddtrace.internal.utils.formats import parse_tags_str
+
 
 def _derive_stacktrace_resolver(config: "CrashtrackerConfig") -> t.Optional[str]:
     resolver = str(config._stacktrace_resolver or "")
@@ -41,8 +43,8 @@ class CrashtrackerConfig(En):
         "debug_url",
         default=None,
         help_type="String",
-        help="Overrides the URL parameter set by the ddtrace library.  This is for testing and debugging purposes"
-        " and is not generally useful for end-users.",
+        help="Overrides the URL parameter set by the ddtrace library. "
+        "This is generally useful only for dd-trace-py development.",
     )
 
     stdout_filename = En.v(
@@ -66,7 +68,8 @@ class CrashtrackerConfig(En):
         "alt_stack",
         default=False,
         help_type="Boolean",
-        help="Whether to use an alternate stack for the crashtracker.  This is used for internal development.",
+        help="Whether to use an alternate stack for the crashtracker."
+        "This is generally useful only for dd-trace-py development.",
     )
 
     _stacktrace_resolver = En.v(
@@ -79,5 +82,15 @@ class CrashtrackerConfig(En):
     )
     stacktrace_resolver = En.d(t.Optional[str], _derive_stacktrace_resolver)
 
+    tags = En.v(
+        dict,
+        "tags",
+        parser=parse_tags_str,
+        default={},
+        help_type="Mapping",
+        help="Additional crashtracking tags. Must be a list in the ``key1:value,key2:value2`` format. "
+        "This is generally useful only for dd-trace-py development.",
+    )
+
 
 config = CrashtrackerConfig()

tests/internal/crashtracker/test_crashtracker.py

@@ -382,3 +382,131 @@ def test_crashtracker_auto_disabled(run_python_code_in_subprocess):
     # Wait for the connection, which should fail
     conn = utils.listen_get_conn(sock)
     assert not conn
+
+
+@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
+def test_crashtracker_user_tags_envvar(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
+
+    # Injecting tags, but since the way we validate them is with a raw-data string search, we make things unique
+    tag_prefix = "cryptocrystalline"
+    tags = {
+        tag_prefix + "_tag1": "quartz_flint",
+        tag_prefix + "_tag2": "quartz_chert",
+    }
+    env["DD_CRASHTRACKER_TAGS"] = ",".join(["%s:%s" % (k, v) for k, v in tags.items()])
+    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)
+    assert data
+
+    # Now check for the tags
+    for k, v in tags.items():
+        assert k.encode() in data
+        assert v.encode() in data
+
+
+@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
+@pytest.mark.subprocess()
+def test_crashtracker_user_tags_profiling():
+    # Tests tag ingestion in the backend API (which is currently out of profiling)
+    import ctypes
+    import os
+
+    import ddtrace.internal.datadog.profiling.crashtracker as crashtracker
+    import tests.internal.crashtracker.utils as utils
+
+    # Define some tags
+    tag_prefix = "manganese_oxides"
+    tags = {
+        tag_prefix + "_tag1": "pyrolusite",
+        tag_prefix + "_tag2": "birnessite",
+    }
+
+    port, sock = utils.crashtracker_receiver_bind()
+    assert port
+    assert sock
+
+    pid = os.fork()
+    if pid == 0:
+        # Set the tags before starting
+        for k, v in tags.items():
+            crashtracker.set_tag(k, v)
+        assert utils.start_crashtracker(port)
+        stdout_msg, stderr_msg = utils.read_files(["stdout.log", "stderr.log"])
+        assert not stdout_msg
+        assert not stderr_msg
+
+        ctypes.string_at(0)
+        exit(-1)
+
+    conn = utils.listen_get_conn(sock)
+    assert conn
+    data = utils.conn_to_bytes(conn)
+    conn.close()
+    assert b"string_at" in data
+
+    # Now check for the tags
+    for k, v in tags.items():
+        assert k.encode() in data
+        assert v.encode() in data
+
+
+@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="Linux only")
+@pytest.mark.subprocess()
+def test_crashtracker_user_tags_core():
+    # Tests tag ingestion in the core API
+    import ctypes
+    import os
+
+    from ddtrace.internal.core import crashtracking
+    import tests.internal.crashtracker.utils as utils
+
+    # Define some tags
+    tag_prefix = "manganese_oxides"
+    tags = {
+        tag_prefix + "_tag1": "pyrolusite",
+        tag_prefix + "_tag2": "birnessite",
+    }
+
+    port, sock = utils.crashtracker_receiver_bind()
+    assert port
+    assert sock
+
+    pid = os.fork()
+    if pid == 0:
+        # Set the tags before starting
+        for k, v in tags.items():
+            crashtracking.add_tag(k, v)
+        assert utils.start_crashtracker(port)
+        stdout_msg, stderr_msg = utils.read_files(["stdout.log", "stderr.log"])
+        assert not stdout_msg
+        assert not stderr_msg
+
+        ctypes.string_at(0)
+        exit(-1)
+
+    conn = utils.listen_get_conn(sock)
+    assert conn
+    data = utils.conn_to_bytes(conn)
+    conn.close()
+    assert b"string_at" in data
+
+    # Now check for the tags
+    for k, v in tags.items():
+        assert k.encode() in data
+        assert v.encode() in data