Skip to content

feat(libdd-trace-utils): check for empty value in header datadog-client-computed-stats #1900

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

Merged
gh-worker-dd-mergequeue-cf854d merged 4 commits into from
Apr 22, 2026
Merged

feat(libdd-trace-utils): check for empty value in header datadog-client-computed-stats #1900

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
17c3f1d
check for empty value in header datadog-client-computed-stats
duncanpharvey Apr 20, 2026
dd9f5ac
remove is_header_empty
duncanpharvey Apr 22, 2026
9da57c8
update struct doc for TracerHeaderTags
duncanpharvey Apr 22, 2026
b2bcec9
Merge branch 'main' into duncan-harvey/datadog-client-computed-stats
duncanpharvey Apr 22, 2026
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
34 changes: 28 additions & 6 deletions libdd-trace-utils/src/tracer_header_tags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ pub struct TracerHeaderTags<'a> {
pub lang_vendor: &'a str,
pub tracer_version: &'a str,
pub container_id: &'a str,
// specifies that the client has marked top-level spans, when set. Any non-empty value will
// mean 'yes'.
// specifies that the client has marked top-level spans, when set. If the header is present
// this value will resolve to 'true'
pub client_computed_top_level: bool,
// specifies whether the client has computed stats so that the agent doesn't have to. Any
// non-empty value will mean 'yes'.
// specifies whether the client has computed stats so that the agent doesn't have to. If the
// header is present and is non-empty this value will resolve to 'true'
pub client_computed_stats: bool,
// number of trace chunks dropped in the tracer
pub dropped_p0_traces: usize,
Expand Down Expand Up @@ -133,8 +133,8 @@ impl<'a> From<&'a HeaderMap<HeaderValue>> for TracerHeaderTags<'a> {
if headers.get("datadog-client-computed-top-level").is_some() {
tags.client_computed_top_level = true;
Comment on lines 133 to 134
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datadog-client-computed-top-level is still treated as enabled solely by header presence (is_some()), so an explicitly empty header value (""), if sent, will set client_computed_top_level=true. This conflicts with the struct doc comment (“Any non-empty value will mean 'yes'”) and now differs from the datadog-client-computed-stats handling in the next lines. Consider parsing the value and requiring it to be non-empty (similar to client_computed_stats) to make the semantics consistent.

Suggested change
if headers.get("datadog-client-computed-top-level").is_some() {
tags.client_computed_top_level = true;
if let Some(v) = headers.get("datadog-client-computed-top-level") {
tags.client_computed_top_level = !is_header_empty(v.to_str().unwrap_or_default());

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@duncanpharvey duncanpharvey Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The datadog-client-computed-top-level header is outside the scope of this change. Will leave this header handling as is until the need arises to change it.

Copy link
Copy Markdown
Contributor

@yannham yannham Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we still at least fix the struct doc (potentially in a separate PR? If the Codex assessment is true, contradicting doc and implementation can be confusing.

Copy link
Copy Markdown
Contributor Author

@duncanpharvey duncanpharvey Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the TracerHeaderTags struct doc to accurately reflect the behavior of these fields:

libdatadog/libdd-trace-utils/src/tracer_header_tags.rs

Lines 30 to 35 in 9da57c8

// specifies that the client has marked top-level spans, when set. If the header is present
// this value will resolve to 'true'
pub client_computed_top_level: bool,
// specifies whether the client has computed stats so that the agent doesn't have to. If the
// header is present and is non-empty this value will resolve to 'true'
pub client_computed_stats: bool,

}
if headers.get("datadog-client-computed-stats").is_some() {
tags.client_computed_stats = true;
if let Some(v) = headers.get("datadog-client-computed-stats") {
tags.client_computed_stats = !v.to_str().unwrap_or_default().is_empty();
}
if let Some(count) = headers.get("datadog-client-dropped-p0-traces") {
tags.dropped_p0_traces = count
Expand Down Expand Up @@ -262,4 +262,26 @@ mod tests {
assert_eq!(tags.dropped_p0_traces, 12);
assert_eq!(tags.dropped_p0_spans, 0);
}

#[test]
fn test_header_map_to_tags_computed_stats_empty_string() {
let val = "";
let mut header_map = HeaderMap::new();
header_map.insert("datadog-client-computed-stats", val.parse().unwrap());
let tags: TracerHeaderTags = (&header_map).into();
assert!(
!tags.client_computed_stats,
"expected client_computed_stats=false for datadog-client-computed-stats header value {val:?}"
);
}

#[test]
fn test_header_map_to_tags_computed_stats_not_set() {
let header_map = HeaderMap::new();
let tags: TracerHeaderTags = (&header_map).into();
assert!(
!tags.client_computed_stats,
"expected client_computed_stats=false when datadog-client-computed-stats header is not set"
);
}
}
Loading