chore: fix/update tracer flares to the format expected (#13447)

Fixes:

- Default value for some remote config version fields (required by
system tests).
- Add uuid.
- Use a strict ordering of the payload.
- Check the contents of the config, not the name as per the RFC.
- Don't send the API KEY since it will be forwarded by the agent.
- Add more tests to check these changes.


Signed-off-by: Juanjo Alvarez <[email protected]>##
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
- [ ] 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)

---------

Signed-off-by: Juanjo Alvarez <[email protected]>
Co-authored-by: brettlangdon <[email protected]>
Co-authored-by: Gabriele N. Tornetta <[email protected]>
Co-authored-by: Rachel Yang <[email protected]>
Co-authored-by: Quinna Halim <[email protected]>
Co-authored-by: Christophe Papazian <[email protected]>
Co-authored-by: Federico Mon <[email protected]>
Co-authored-by: ncybul <[email protected]>

Author: 8 people

ddtrace/internal/flare/flare.py

@@ -1,4 +1,3 @@
-import binascii
 import dataclasses
 import io
 import json
@@ -7,9 +6,8 @@
 import os
 import pathlib
 import shutil
-from typing import Dict
+import time
 from typing import Optional
-from typing import Tuple
 import zipfile
 
 from ddtrace._logger import _add_file_handler
@@ -32,6 +30,7 @@ class FlareSendRequest:
     case_id: str
     hostname: str
     email: str
+    uuid: str  # UUID from AGENT_TASK config for race condition prevention
     source: str = "tracer_python"
 
 
@@ -55,6 +54,8 @@ def __init__(
         self.url: str = trace_agent_url
         self._api_key: Optional[str] = api_key
         self.ddconfig = ddconfig
+        # Use a fixed boundary for consistency
+        self._BOUNDARY = "83CAD6AA-8A24-462C-8B3D-FF9CC683B51B"
 
     def prepare(self, log_level: str):
         """
@@ -64,32 +65,15 @@ def prepare(self, log_level: str):
         try:
             self.flare_dir.mkdir(exist_ok=True)
         except Exception as e:
-            log.error("Failed to create %s directory: %s", self.flare_dir, e)
+            log.error("Flare prepare: failed to create %s directory: %s", self.flare_dir, e)
             return
 
         flare_log_level_int = logging.getLevelName(log_level)
         if type(flare_log_level_int) != int:
-            raise TypeError("Invalid log level provided: %s", log_level)
+            raise TypeError("Flare prepare: Invalid log level provided: %s", log_level)
 
-        ddlogger = get_logger("ddtrace")
-        pid = os.getpid()
-        flare_file_path = self.flare_dir / f"tracer_python_{pid}.log"
-        self.original_log_level = ddlogger.level
-
-        # Set the logger level to the more verbose between original and flare
-        # We do this valid_original_level check because if the log level is NOTSET, the value is 0
-        # which is the minimum value. In this case, we just want to use the flare level, but still
-        # retain the original state as NOTSET/0
-        valid_original_level = (
-            logging.CRITICAL if self.original_log_level == logging.NOTSET else self.original_log_level
-        )
-        logger_level = min(valid_original_level, flare_log_level_int)
-        ddlogger.setLevel(logger_level)
-        self.file_handler = _add_file_handler(
-            ddlogger, flare_file_path.__str__(), flare_log_level_int, TRACER_FLARE_FILE_HANDLER_NAME
-        )
-
-        # Create and add config file
+        # Setup logging and create config file
+        pid = self._setup_flare_logging(flare_log_level_int)
         self._generate_config_file(pid)
 
     def send(self, flare_send_req: FlareSendRequest):
@@ -98,36 +82,16 @@ def send(self, flare_send_req: FlareSendRequest):
         before sending the flare.
         """
         self.revert_configs()
-        # We only want the flare to be sent once, even if there are
-        # multiple tracer instances
-        lock_path = self.flare_dir / TRACER_FLARE_LOCK
-        if not os.path.exists(lock_path):
-            try:
-                open(lock_path, "w").close()
-            except Exception as e:
-                log.error("Failed to create %s file", lock_path)
-                raise e
-            try:
-                client = get_connection(self.url, timeout=self.timeout)
-                headers, body = self._generate_payload(flare_send_req.__dict__)
-                client.request("POST", TRACER_FLARE_ENDPOINT, body, headers)
-                response = client.getresponse()
-                if response.status == 200:
-                    log.info("Successfully sent the flare to Zendesk ticket %s", flare_send_req.case_id)
-                else:
-                    msg = "Tracer flare upload responded with status code %s:(%s) %s" % (
-                        response.status,
-                        response.reason,
-                        response.read().decode(),
-                    )
-                    raise TracerFlareSendError(msg)
-            except Exception as e:
-                log.error("Failed to send tracer flare to Zendesk ticket %s: %s", flare_send_req.case_id, e)
-                raise e
-            finally:
-                client.close()
-                # Clean up files regardless of success/failure
-                self.clean_up_files()
+
+        # Ensure the flare directory exists (it might have been deleted by clean_up_files)
+        self.flare_dir.mkdir(exist_ok=True)
+
+        try:
+            if not self._validate_case_id(flare_send_req.case_id):
+                return
+            self._send_flare_request(flare_send_req)
+        finally:
+            self.clean_up_files()
 
     def _generate_config_file(self, pid: int):
         config_file = self.flare_dir / f"tracer_config_{pid}.json"
@@ -161,41 +125,134 @@ def revert_configs(self):
             log.debug("Could not find %s to remove", TRACER_FLARE_FILE_HANDLER_NAME)
         ddlogger.setLevel(self.original_log_level)
 
-    def _generate_payload(self, params: Dict[str, str]) -> Tuple[dict, bytes]:
+    def _validate_case_id(self, case_id: str) -> bool:
+        """
+        Validate case_id (must be a digit and cannot be 0 according to spec).
+        Returns True if valid, False otherwise. Cleans up files if invalid.
+        """
+        if case_id in ("0", 0):
+            log.warning("Case ID cannot be 0, skipping flare send")
+            return False
+
+        if not case_id.isdigit():
+            log.warning("Case ID string must contain a digit, skipping flare send")
+            return False
+
+        return True
+
+    def _setup_flare_logging(self, flare_log_level_int: int) -> int:
+        """
+        Setup flare logging configuration.
+        Returns the process ID.
+        """
+        ddlogger = get_logger("ddtrace")
+        pid = os.getpid()
+        flare_file_path = self.flare_dir / f"tracer_python_{pid}.log"
+        self.original_log_level = ddlogger.level
+
+        # Set the logger level to the more verbose between original and flare
+        # We do this valid_original_level check because if the log level is NOTSET, the value is 0
+        # which is the minimum value. In this case, we just want to use the flare level, but still
+        # retain the original state as NOTSET/0
+        valid_original_level = (
+            logging.CRITICAL if self.original_log_level == logging.NOTSET else self.original_log_level
+        )
+        logger_level = min(valid_original_level, flare_log_level_int)
+        ddlogger.setLevel(logger_level)
+        self.file_handler = _add_file_handler(
+            ddlogger, flare_file_path.__str__(), flare_log_level_int, TRACER_FLARE_FILE_HANDLER_NAME
+        )
+        return pid
+
+    def _create_zip_content(self) -> bytes:
+        """
+        Create ZIP file content containing all flare files.
+        Returns the ZIP file content as bytes.
+        """
         zip_stream = io.BytesIO()
         with zipfile.ZipFile(zip_stream, mode="w", compression=zipfile.ZIP_DEFLATED) as zipf:
             for flare_file_name in self.flare_dir.iterdir():
                 zipf.write(flare_file_name, arcname=flare_file_name.name)
         zip_stream.seek(0)
+        return zip_stream.getvalue()
 
-        newline = b"\r\n"
+    def _write_body_field(self, body: io.BytesIO, name: str, value: str):
+        """Write a form field to the multipart body."""
+        body.write(f"--{self._BOUNDARY}\r\n".encode())
+        body.write(f'Content-Disposition: form-data; name="{name}"\r\n\r\n'.encode())
+        body.write(f"{value}\r\n".encode())
 
-        boundary = binascii.hexlify(os.urandom(16))
+    def _generate_payload(self, flare_send_req):
+        """
+        Generate the multipart form-data payload for the flare request.
+        """
+
+        # Create the multipart form data in the same order:
+        # source, case_id, hostname, email, uuid, flare_file
         body = io.BytesIO()
-        for key, value in params.items():
-            encoded_key = key.encode()
-            encoded_value = value.encode()
-            body.write(b"--" + boundary + newline)
-            body.write(b'Content-Disposition: form-data; name="%s"%s%s' % (encoded_key, newline, newline))
-            body.write(b"%s%s" % (encoded_value, newline))
-
-        body.write(b"--" + boundary + newline)
-        body.write((b'Content-Disposition: form-data; name="flare_file"; filename="flare.zip"%s' % newline))
-        body.write(b"Content-Type: application/octet-stream%s%s" % (newline, newline))
-        body.write(zip_stream.getvalue() + newline)
-        body.write(b"--" + boundary + b"--")
+        self._write_body_field(body, "source", "tracer_python")
+        self._write_body_field(body, "case_id", flare_send_req.case_id)
+        self._write_body_field(body, "hostname", flare_send_req.hostname)
+        self._write_body_field(body, "email", flare_send_req.email)
+        self._write_body_field(body, "uuid", flare_send_req.uuid)
+
+        # flare_file field with descriptive filename
+        timestamp = int(time.time() * 1000)
+        filename = f"tracer-python-{flare_send_req.case_id}-{timestamp}-debug.zip"
+        body.write(f"--{self._BOUNDARY}\r\n".encode())
+        body.write(f'Content-Disposition: form-data; name="flare_file"; filename="{filename}"\r\n'.encode())
+        body.write(b"Content-Type: application/octet-stream\r\n\r\n")
+
+        # Create the zip file content separately
+        body.write(self._create_zip_content() + b"\r\n")
+
+        # Ending boundary
+        body.write(f"--{self._BOUNDARY}--\r\n".encode())
+
+        # Set headers
         headers = {
-            "Content-Type": b"multipart/form-data; boundary=%s" % boundary,
-            "Content-Length": body.getbuffer().nbytes,
+            "Content-Type": f"multipart/form-data; boundary={self._BOUNDARY}",
+            "Content-Length": str(body.tell()),
         }
-        if self._api_key:
-            headers["DD-API-KEY"] = self._api_key
+
+        # Note: don't send DD-API-KEY or Host Header - the agent should add it when forwarding to backend
         return headers, body.getvalue()
 
     def _get_valid_logger_level(self, flare_log_level: int) -> int:
         valid_original_level = 100 if self.original_log_level == 0 else self.original_log_level
         return min(valid_original_level, flare_log_level)
 
+    def _send_flare_request(self, flare_send_req: FlareSendRequest):
+        """
+        Send the flare request to the agent.
+        """
+        # We only want the flare to be sent once, even if there are
+        # multiple tracer instances
+        lock_path = self.flare_dir / TRACER_FLARE_LOCK
+        if not os.path.exists(lock_path):
+            open(lock_path, "w").close()
+            client = None
+            try:
+                client = get_connection(self.url, timeout=self.timeout)
+                headers, body = self._generate_payload(flare_send_req)
+                client.request("POST", TRACER_FLARE_ENDPOINT, body, headers)
+                response = client.getresponse()
+                if response.status == 200:
+                    log.info("Successfully sent the flare to Zendesk ticket %s", flare_send_req.case_id)
+                else:
+                    msg = "Tracer flare upload responded with status code %s:(%s) %s" % (
+                        response.status,
+                        response.reason,
+                        response.read().decode(),
+                    )
+                    raise TracerFlareSendError(msg)
+            except Exception as e:
+                log.error("Failed to send tracer flare to Zendesk ticket %s: %s", flare_send_req.case_id, e)
+                raise e
+            finally:
+                if client is not None:
+                    client.close()
+
     def clean_up_files(self):
         try:
             shutil.rmtree(self.flare_dir)

ddtrace/internal/flare/handler.py

@@ -59,18 +59,22 @@ def _prepare_tracer_flare(flare: Flare, configs: List[Any]) -> bool:
     """
     for c in configs:
         # AGENT_CONFIG is currently being used for multiple purposes
-        # We only want to prepare for a tracer flare if the config name
-        # starts with 'flare-log-level'
+        # We only want to prepare for a tracer flare if the config content
+        # has a log_level.
         if not isinstance(c, dict):
             log.debug("Config item is not type dict, received type %s instead. Skipping...", str(type(c)))
             continue
-        if not c.get("name", "").startswith("flare-log-level"):
+
+        config_content = c.get("config", {})
+        log_level = config_content.get("log_level", "")
+        if log_level == "":
             log.debug(
-                "Config item name does not start with flare-log-level, received %s instead. Skipping...", c.get("name")
+                "Config item does not contain log_level, received %s instead. Skipping...",
+                config_content.get("log_level"),
             )
             continue
 
-        flare_log_level = c.get("config", {}).get("log_level").upper()
+        flare_log_level = log_level.lower()
         flare.prepare(flare_log_level)
         return True
     return False
@@ -95,8 +99,13 @@ def _generate_tracer_flare(flare: Flare, configs: List[Any]) -> bool:
             )
             continue
         args = c.get("args", {})
+        uuid = c.get("uuid")
+        if not uuid:
+            log.warning("AGENT_TASK config missing UUID, skipping tracer flare")
+            continue
+
         flare_request = FlareSendRequest(
-            case_id=args.get("case_id"), hostname=args.get("hostname"), email=args.get("user_handle")
+            case_id=args.get("case_id"), hostname=args.get("hostname"), email=args.get("user_handle"), uuid=uuid
         )
 
         flare.revert_configs()

ddtrace/internal/remoteconfig/client.py

@@ -101,7 +101,7 @@ class Root:
     expires: datetime
     keys: Mapping[str, Key]
     roles: Mapping[str, Role]
-    version: int
+    version: int = 0
 
     def __post_init__(self):
         if self._type != "root":
@@ -143,7 +143,7 @@ class Targets:
     expires: datetime
     spec_version: str
     targets: Mapping[str, TargetDesc]
-    version: int
+    version: int = 0
 
     def __post_init__(self):
         if self._type != "targets":
@@ -161,6 +161,7 @@ def __post_init__(self):
 class SignedTargets:
     signatures: List[Signature]
     signed: Targets
+    version: int = 0
 
     def __post_init__(self):
         for i in range(len(self.signatures)):