Skip to content

fix(traces): keep backend as safety net for trace stats #1171

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.

Sign up for GitHub

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

Closed
duncanista wants to merge 1 commit into from
Closed

fix(traces): keep backend as safety net for trace stats #1171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions bottlecap/src/lifecycle/invocation/processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2255,8 +2255,9 @@ mod tests {
"client_computed_stats must be propagated to the aws.lambda span payload"
);

// Verify _dd.compute_stats is "0" in the built payload tags: client_computed_stats=true
// means the tracer has already computed stats, so neither extension nor backend should.
// Verify _dd.compute_stats is "1" in the built payload tags: the backend is always
// the safety net when compute_trace_stats_on_extension is false, regardless of
// client_computed_stats.
let send_data = payload.builder.build();
let libdd_trace_utils::tracer_payload::TracerPayloadCollection::V07(payloads) =
send_data.get_payloads()
Expand All @@ -2266,8 +2267,8 @@ mod tests {
for p in payloads {
assert_eq!(
p.tags.get(crate::tags::lambda::tags::COMPUTE_STATS_KEY),
Some(&"0".to_string()),
"_dd.compute_stats must be 0 when client_computed_stats is true"
Some(&"1".to_string()),
"_dd.compute_stats must be 1 (backend safety net) when compute_trace_stats_on_extension is false"
);
}
}
Expand Down
33 changes: 20 additions & 13 deletions bottlecap/src/traces/trace_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,14 +351,19 @@ impl TraceProcessor for ServerlessTraceProcessor {
for tracer_payload in collection.iter_mut() {
tracer_payload.tags.extend(tags.clone());
// Tell the backend whether to compute stats:
// - "1" (compute on backend) if neither the tracer nor the extension is computing them
// - "0" (skip on backend) if the extension or the tracer has already computed them
let compute_stats = if !config.compute_trace_stats_on_extension
&& !header_tags.client_computed_stats
{
"1"
} else {
// - "1" (compute on backend) if the extension is NOT computing stats
// - "0" (skip on backend) if the extension is computing stats
//
// We intentionally ignore client_computed_stats here to keep
// the backend as a safety net. If the tracer claims it computed
// stats but they don't actually reach the backend, setting this
// to "0" would cause stats to disappear entirely.
Comment thread
duncanista marked this conversation as resolved.
// This may lead to double-counting when the tracer also sends
// stats, but that is preferable to zero stats.
let compute_stats = if config.compute_trace_stats_on_extension {
"0"
} else {
"1"
};
Comment thread
duncanista marked this conversation as resolved.
tracer_payload
.tags
Expand Down Expand Up @@ -1400,7 +1405,7 @@ mod tests {
/// | Input: `compute_trace_stats_on_extension` | Input: `client_computed_stats` | Expected: `_dd.compute_stats` | Expected: Extension generates stats? |
/// |-------------------------------------------|--------------------------------|-------------------------------|--------------------------------------|
/// | `false` | `false` | `"1"` | No |
/// | `false` | `true` | `"0"` | No |
/// | `false` | `true` | `"1"` | No |
/// | `true` | `false` | `"0"` | Yes |
/// | `true` | `true` | `"0"` | No |
#[allow(clippy::unwrap_used)]
Expand All @@ -1414,11 +1419,12 @@ mod tests {
use libdd_trace_obfuscation::obfuscation_config::ObfuscationConfig;
use tokio::sync::mpsc;

// "_dd.compute_stats" is "1" only when neither side computes stats (backend must do it).
let expected_tag = if !compute_trace_stats_on_extension && !client_computed_stats {
"1"
} else {
// "_dd.compute_stats" is "1" when the extension is NOT computing stats (backend must do it).
// client_computed_stats does NOT affect this tag — the backend is the safety net.
let expected_tag = if compute_trace_stats_on_extension {
"0"
} else {
"1"
};
// The extension generates stats only when it is configured to do so and the tracer hasn't.
let expect_stats = compute_trace_stats_on_extension && !client_computed_stats;
Expand Down Expand Up @@ -1529,7 +1535,8 @@ mod tests {
#[tokio::test]
#[allow(clippy::unwrap_used)]
async fn test_compute_stats_tag_tracer_computes() {
// Tracer computed stats (Datadog-Client-Computed-Stats header set) → tag "0".
// Tracer computed stats (Datadog-Client-Computed-Stats header set) → tag "1".
// Backend still computes as a safety net; extension skips its own stats generation.
check_compute_stats_behavior(false, true).await;
}

Expand Down
Loading