-
Notifications
You must be signed in to change notification settings - Fork 15
instrumentation telemetry: validate session id headers #6510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8d9511f
f387188
4974b0f
116bfa7
ba3eca6
1b42dfd
4f96b14
bc3d7a5
f3c55ea
8143d57
43bd633
a0b2978
f307e4b
cd21b0c
b590862
d51f23d
d0b1214
74a841e
548a29c
9281860
eab3a09
3e45265
5bf3eb7
75eb41d
d3cb17e
441d753
f6ea416
3470951
50bdced
7e72a42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -580,6 +580,83 @@ def test_app_product_change(self): | |
| if app_product_change_event_found is False: | ||
| raise Exception("app-product-change is not emitted when product change is enabled") | ||
|
|
||
| def setup_session_id_headers_across_forks(self): | ||
| """Trigger spawn_child endpoint to create a fork tree for session ID header validation.""" | ||
| weblog.get("/spawn_child", params={"sleep": 2, "crash": False, "fork": True}) | ||
|
|
||
| def setup_session_id_headers_across_spawned(self): | ||
| """Trigger spawn_child endpoint with exec (fork=false) for session ID header validation.""" | ||
| weblog.get("/spawn_child", params={"sleep": 2, "crash": False, "fork": False}) | ||
|
|
||
| def _validate_session_id_headers_across_processes(self) -> None: | ||
|
mabdinur marked this conversation as resolved.
|
||
| """Validate DD-Session-ID, DD-Root-Session-ID, DD-Parent-Session-ID in telemetry. | ||
|
|
||
| Stable Service Instance Identifier RFC: each app instance has one root runtime_id. | ||
| DD-Session-ID (instance id) must equal runtime_id. When only DD-Session-ID is sent | ||
| (no DD-Root-Session-ID), the process is treated as the root. This test confirms | ||
| at least two different runtimes are captured (parent and child from spawn_child). | ||
| """ | ||
| # Use lifecycle events only; metrics and log events from lib-datadog can contain | ||
| # runtime/session_ids that do not map to tracer-generated telemetry. | ||
| telemetry_data = list(interfaces.library.get_lifecycle_events()) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This validator reads all lifecycle telemetry ( Useful? React with 👍 / 👎. |
||
| if not telemetry_data: | ||
| raise ValueError("No telemetry data to validate on") | ||
|
|
||
| assert len(telemetry_data) > 1, ( | ||
| f"Expected multiple telemetry events to verify consistency, got {len(telemetry_data)}" | ||
| ) | ||
|
|
||
| runtime_ids = set[str]() | ||
| parent_runtime_ids = set[str]() | ||
| root_runtime_ids = set[str]() | ||
|
|
||
| for data in telemetry_data: | ||
| # Headers are not case sensitive | ||
| curr_sid = get_header(data, "request", "dd-session-id") | ||
| curr_rid = get_header(data, "request", "dd-root-session-id") | ||
| curr_pid = get_header(data, "request", "dd-parent-session-id") | ||
| curr_id = data["request"]["content"].get("runtime_id") | ||
|
|
||
| # Instance id (DD-Session-ID) must be present in all lifecycle events and equal to runtime_id | ||
| assert curr_sid is not None, f"DD-Session-ID is required in telemetry data: {data}" | ||
| assert curr_sid == curr_id, f"DD-Session-ID must match runtime_id: {curr_sid} != {curr_id}" | ||
|
|
||
| runtime_ids.add(curr_id) | ||
| if curr_pid is not None: | ||
| parent_runtime_ids.add(curr_pid) | ||
| if curr_rid is not None: | ||
| root_runtime_ids.add(curr_rid) | ||
| else: | ||
| # If dd-root-session-id is not set, dd-session-id is treated as root | ||
| root_runtime_ids.add(curr_id) | ||
|
|
||
| # One root per app instance: all processes share the same root session ID | ||
| assert len(root_runtime_ids) == 1, f"Expected 1 root runtime_id, got {root_runtime_ids}" | ||
|
mabdinur marked this conversation as resolved.
|
||
|
|
||
| if len(runtime_ids) > 1: | ||
| # Multiple runtimes (per-process tracers): root must be consistent | ||
| # across all payloads from all processes | ||
| if parent_runtime_ids: | ||
| # DD-Parent-Session-ID is optional but must reference a known runtime if present | ||
| missing_parent_runtime_ids = parent_runtime_ids.difference(runtime_ids) | ||
| assert not missing_parent_runtime_ids, ( | ||
| f"Parent runtime_id with no telemetry data: {missing_parent_runtime_ids}" | ||
| ) | ||
| else: | ||
| # Single runtime (e.g. nginx workers sharing one tracer): session ID | ||
| # must be consistent across all events | ||
| sole_rid = next(iter(runtime_ids)) | ||
| sole_root = next(iter(root_runtime_ids)) | ||
| assert sole_rid == sole_root, f"Single runtime_id {sole_rid} does not match root {sole_root}" | ||
|
|
||
| def test_session_id_headers_across_forks(self): | ||
| """Test session ID headers in telemetry (fork=true). Stable Service Instance Identifier RFC.""" | ||
| self._validate_session_id_headers_across_processes() | ||
|
|
||
| def test_session_id_headers_across_spawned(self): | ||
| """Test session ID headers in telemetry (fork=false, exec). Stable Service Instance Identifier RFC.""" | ||
| self._validate_session_id_headers_across_processes() | ||
|
|
||
|
mabdinur marked this conversation as resolved.
|
||
|
|
||
| @features.telemetry_app_started_event | ||
| @scenarios.telemetry_enhanced_config_reporting | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| #include <signal.h> | ||
| #include <stdio.h> | ||
| #include <sys/stat.h> | ||
| #include <sys/wait.h> | ||
| #include <unistd.h> | ||
|
|
||
| #define PORT 7778 | ||
|
|
@@ -310,6 +311,85 @@ static enum MHD_Result answer_to_connection(void *cls, struct MHD_Connection *co | |
| return ret; | ||
| } | ||
|
|
||
| if (strcmp(url, "/spawn_child") == 0) { | ||
| const char *sleep_str = MHD_lookup_connection_value(connection, MHD_GET_ARGUMENT_KIND, "sleep"); | ||
| const char *crash_str = MHD_lookup_connection_value(connection, MHD_GET_ARGUMENT_KIND, "crash"); | ||
| const char *fork_str = MHD_lookup_connection_value(connection, MHD_GET_ARGUMENT_KIND, "fork"); | ||
|
|
||
| if (!sleep_str || !crash_str || !fork_str) { | ||
| const char *msg = "sleep, crash, and fork parameters required"; | ||
| struct MHD_Response *response = MHD_create_response_from_buffer( | ||
| strlen(msg), (void *)msg, MHD_RESPMEM_PERSISTENT); | ||
| int ret = MHD_queue_response(connection, 400, response); | ||
| MHD_destroy_response(response); | ||
| return ret; | ||
| } | ||
|
|
||
| int sleep_secs = atoi(sleep_str); | ||
| bool do_crash = strcmp(crash_str, "true") == 0; | ||
| bool use_fork = strcmp(fork_str, "true") == 0; | ||
|
Comment on lines
+329
to
+330
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The cpp_nginx handler compares Useful? React with 👍 / 👎. |
||
|
|
||
| if (use_fork) { | ||
| pid_t pid = fork(); | ||
| if (pid < 0) { | ||
| const char *msg = "fork failed"; | ||
| struct MHD_Response *response = MHD_create_response_from_buffer( | ||
| strlen(msg), (void *)msg, MHD_RESPMEM_PERSISTENT); | ||
| int ret = MHD_queue_response(connection, 500, response); | ||
| MHD_destroy_response(response); | ||
| return ret; | ||
| } | ||
| if (pid == 0) { | ||
| sleep(sleep_secs); | ||
| if (do_crash) { | ||
| raise(SIGSEGV); | ||
| } | ||
| _exit(0); | ||
| } | ||
| int wstatus; | ||
| waitpid(pid, &wstatus, 0); | ||
| char buf[128]; | ||
| snprintf(buf, sizeof(buf), "Child process %d exited with status %d", pid, WEXITSTATUS(wstatus)); | ||
| struct MHD_Response *response = MHD_create_response_from_buffer( | ||
| strlen(buf), buf, MHD_RESPMEM_MUST_COPY); | ||
| int ret = MHD_queue_response(connection, 200, response); | ||
| MHD_destroy_response(response); | ||
| return ret; | ||
| } | ||
|
|
||
| /* exec path: fork + exec a child process */ | ||
| { | ||
| pid_t pid = fork(); | ||
| if (pid < 0) { | ||
| const char *msg = "fork failed"; | ||
| struct MHD_Response *response = MHD_create_response_from_buffer( | ||
| strlen(msg), (void *)msg, MHD_RESPMEM_PERSISTENT); | ||
| int ret = MHD_queue_response(connection, 500, response); | ||
| MHD_destroy_response(response); | ||
| return ret; | ||
| } | ||
| if (pid == 0) { | ||
| if (do_crash) { | ||
| execlp("sh", "sh", "-c", | ||
| sleep_str[0] ? "sleep $0 && kill -SEGV $$" : "kill -SEGV $$", | ||
| sleep_str, (char *)NULL); | ||
| } else { | ||
| execlp("sleep", "sleep", sleep_str, (char *)NULL); | ||
| } | ||
| _exit(1); | ||
| } | ||
| int wstatus; | ||
| waitpid(pid, &wstatus, 0); | ||
| char buf[128]; | ||
| snprintf(buf, sizeof(buf), "Child process %d exited with status %d", pid, WEXITSTATUS(wstatus)); | ||
| struct MHD_Response *response = MHD_create_response_from_buffer( | ||
| strlen(buf), buf, MHD_RESPMEM_MUST_COPY); | ||
| int ret = MHD_queue_response(connection, 200, response); | ||
| MHD_destroy_response(response); | ||
| return ret; | ||
| } | ||
| } | ||
|
|
||
| if (strcmp(url, "/content") != 0 || !status_str || !value) | ||
| return MHD_NO; // Only respond to the correct URL and if all parameters are present | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.