enhancement(app): add support for always-on compressed storage of debug logs - #2116
enhancement(app): add support for always-on compressed storage of debug logs#2116tobz wants to merge 1 commit into
Conversation
dd-octo-sts
Bot
added
area/core
This comment has been minimized.
This comment has been minimized.
tobz
changed the title
54ecf80 to
a087a94
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Binary Size Analysis (Agent Data Plane)Baseline: ee113cc · Comparison: 59970d9 · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ❌ Failed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
a087a94 to
45d2157
Compare
… compressed debug logs
45d2157 to
59970d9
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59970d9e81
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| tracing_subscriber::registry() | ||
| .with(output_layer.with_filter(filter_layer)) | ||
| .with(ring_buffer.with_filter(LevelFilter::DEBUG)) |
There was a problem hiding this comment.
This moves the ring buffer directly into the global subscriber, while CompressedRingBuffer exposes no read/export API and the processor thread exclusively owns ProcessorState. Since this is the only production construction and no diagnostic collector receives a handle, flare collection cannot retrieve any of the stored events; the feature continuously captures logs that are permanently inaccessible.
Useful? React with 👍 / 👎.
| static VALUE_BUF: std::cell::RefCell<String> = const { std::cell::RefCell::new(String::new()) }; | ||
| } | ||
|
|
||
| match self.events_tx.send_ref() { |
There was a problem hiding this comment.
When the 1,024-slot channel fills during a debug-log burst or while the processor is performing level-19 zstd compression, mpsc::blocking::Sender::send_ref waits for a slot instead of dropping the event. Because on_event invokes this synchronously, arbitrary application threads emitting logs can stall behind the compression worker, contrary to the layer's stated nonblocking behavior; use the channel's try-send operation and handle a full result as a dropped event.
Useful? React with 👍 / 👎.
| self.col_timestamps.len() | ||
| + self.col_msg_variables.len() | ||
| + self.col_field_key_indices.len() | ||
| + self.col_field_values.len() | ||
| + self.col_callsite_indices.len() |
There was a problem hiding this comment.
For workloads with static or varied message templates, this estimate omits col_msg_template_indices, col_field_counts, the string and callsite tables, the persistent cluster cache, and all vector capacities; it also counts each usize callsite entry as one byte. ensure_size_limits relies exclusively on this value, so the configured 2 MiB ceiling can be exceeded by many megabytes (or by an arbitrarily large static message) even while the reported crb_bytes_live remains under budget.
Useful? React with 👍 / 👎.
| // Quick check: split by whitespace and count. | ||
| let tokens: Vec<&str> = message.split_ascii_whitespace().collect(); | ||
| if tokens.len() == *expected_token_count { | ||
| for &pos in wildcard_positions.iter() { |
There was a problem hiding this comment.
After a callsite converges, any later message with the same token count takes this fast path without checking that its non-wildcard tokens still match the cached skeleton. For example, if a dynamic-message callsite converges on Request 1 completed and later emits Request 2 failed, up to 999 events are encoded using the old Request … completed skeleton, silently changing the recovered logs; compare the static positions as well as the token count before returning the cached template.
Useful? React with 👍 / 👎.
![datadog-datadog-us1-prod[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new logging layer can block event emitters when its queue fills. It also has no flare read path, does not enforce its memory limit, and can write stale message text.
🤖 Datadog Autotest · Commit 59970d9 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| static VALUE_BUF: std::cell::RefCell<String> = const { std::cell::RefCell::new(String::new()) }; | ||
| } | ||
|
|
||
| match self.events_tx.send_ref() { |
There was a problem hiding this comment.
A log burst can stop request or ingestion work while compression falls behind.
Assertion details
- Input: More than 1024 matching tracing events arrive before the background compressor can consume them.
- Expected:
The logging path must use a non-blocking send. It must drop an event when the queue is full. - Actual:
The blocking send waits for free queue space. The event-emitting thread stalls when the 1024-slot queue is full.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| }; | ||
|
|
||
| let processor_state = ProcessorState::new(config); | ||
| std::thread::spawn(move || run_processor(events_rx, processor_state)); |
There was a problem hiding this comment.
Users cannot get these logs in a flare, so the main feature has no usable output.
Assertion details
- Input: Any caller creates the compressed ring buffer and later requests diagnostic output.
- Expected:
The layer must expose a read handle. The flare collector must use that handle to read the stored logs. - Actual:
The code moves all compressed segments into the background thread. It returns only the write-side sender. No diagnostic collector or read handle can access the stored logs.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| // callsite index column compresses dramatically -- the serialized RLE is typically a small | ||
| // fraction of the element count. We estimate ~1 byte per element as a rough upper bound, | ||
| // since most runs are longer than 1. | ||
| self.col_timestamps.len() |
There was a problem hiding this comment.
The process can use much more heap than the stated fixed memory limit.
Assertion details
- Input: Events contain many unique callsites or large unique message templates.
- Expected:
The configured limit must cover all retained ring-buffer allocations. - Actual:
The size calculation counts only five column lengths. It omits retained strings, patterns, template indexes, field counts, callsite data, hash maps, and allocation capacity.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| if !needs_revalidation { | ||
| // Quick check: split by whitespace and count. | ||
| let tokens: Vec<&str> = message.split_ascii_whitespace().collect(); | ||
| if tokens.len() == *expected_token_count { |
There was a problem hiding this comment.
The diagnostic log can contain incorrect message text during an incident.
Assertion details
- Input: A converged callsite emits a new message shape that has the same number of tokens as the cached shape.
- Expected:
The fast path must also compare fixed token positions before it uses the cached template. - Actual:
The fast path checks only the token count. It reuses the old fixed words for up to 999 events when a message changes to a different shape with the same token count.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
1 participant
Summary
This PR adds support for always-on compressed storage of debug logs to allow for "looking back" at previous debug logs without having to have explicitly set the application's log level to DEBUG or higher.
In many debugging scenarios, it can often be necessary to change configuration settings to collect more verbose output to assist in said debugging. However, this can often be suboptimal because it means the adverse conditions must be triggered again in order to be captured... when if only you had diagnostics/debug output from the initial adverse condition, you could avoid having to spend the time to recreate the issue.
This PR introduces a new logging mechanism that aims to keep as many of the most recent debug logs as possible in a fixed amount of space, in a rotating fashion, such that when we want to debug an adverse condition that recently occurred, we don't have to enable debug logs and then hope the issue happens again or hope that we can reproduce it.. the logs are just already there, and automatically present in flare output. It employs a number of techniques that allow it to store a large number of debug logs, in a nearly lossless form, in a small amount of space.
As part of this PR, we've done the following:
CompressedRingBufferlayer implementation fortracingsaluki-appto collect debug logs by default (configurable), up to 1MiB (that is, it uses a maximum of 1MiB of actual heap to store the values in memory)Architecturally, this implementation works by:
Change Type
How did you test this PR?
New and existing tests.
References
DADP-2