Skip to content

[APMSVLS-197] Add check to extract trace context present within event.request.headers #1011

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
Feb 17, 2026
Merged

[APMSVLS-197] Add check to extract trace context present within event.request.headers #1011

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e677d7e
[APMSVLS-197] feat: Add fallback to extract trace context from event.…
Dogbu-cyber Jan 28, 2026
50ae32f
[APMSVLS-197] test: Add unit tests for event.request.headers trace co…
Dogbu-cyber Jan 29, 2026
87af0a4
[APMSVLS-197] refactor: Prioritize event.request.headers extraction f…
Dogbu-cyber Jan 29, 2026
3ef4752
[APMSVLS-197] refactor: Clean up trace context extraction code and te…
Dogbu-cyber Feb 5, 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
76 changes: 76 additions & 0 deletions bottlecap/src/lifecycle/invocation/processor.rs
View file
Open in desktop
Copy link
Contributor

@duncanista duncanista Jan 28, 2026

Choose a reason for hiding this comment

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

Could you add some unit testing and show an example trace in the PR description? Just to see what were we missing

Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,15 @@ impl Processor {
return Some(sc);
}

if let Some(sc) = payload_value
.get("request")
.and_then(|req| req.get("headers"))
.and_then(|headers| propagator.extract(headers))
{
debug!("Extracted trace context from event.request.headers");
return Some(sc);
}

if let Some(payload_headers) = payload_value.get("headers") {
if let Some(sc) = propagator.extract(payload_headers) {
debug!("Extracted trace context from event headers");
Expand Down Expand Up @@ -1851,4 +1860,71 @@ mod tests {
"Should be snapstart span (id=3), not cold start span (id=2)"
);
}

#[test]
fn test_extract_span_context_priority_order() {
let config = Arc::new(config::Config {
trace_propagation_style_extract: vec![
config::trace_propagation_style::TracePropagationStyle::Datadog,
config::trace_propagation_style::TracePropagationStyle::TraceContext,
],
..config::Config::default()
});
let propagator = Arc::new(DatadogCompositePropagator::new(Arc::clone(&config)));

let mut headers = HashMap::new();
headers.insert(DATADOG_TRACE_ID_KEY.to_string(), "111".to_string());
headers.insert(DATADOG_PARENT_ID_KEY.to_string(), "222".to_string());

let payload = json!({
"headers": {
"x-datadog-trace-id": "333",
"x-datadog-parent-id": "444"
},
"request": {
"headers": {
"x-datadog-trace-id": "555",
"x-datadog-parent-id": "666"
}
}
});

let result = Processor::extract_span_context(&headers, &payload, propagator);

assert!(result.is_some());
let context = result.unwrap();
assert_eq!(
context.trace_id, 555,
"Should prioritize event.request.headers as service-specific extraction"
Comment on lines +1897 to +1898
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The test assertion comment claims event.request.headers should be prioritized, but according to the implementation, event.request.headers is checked after trigger-specific headers and before event.headers. With the test data showing headers containing trace_id=111, the actual priority would favor the trigger-specific headers first. The test should either verify that trace_id=111 is extracted (if headers are checked first), or the test data should be adjusted to only include event.request.headers to properly validate this extraction path.

Copilot uses AI. Check for mistakes.
);
Comment on lines +1896 to +1899
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The test name test_extract_span_context_priority_order and assertion message claim that event.request.headers should be prioritized, but the implementation shows it's actually checked after event.headers (line 1075). The trace_id assertion expects 555 (from event.request.headers) but based on the extraction order, it should extract 333 from event.headers first. Either the test assertion is incorrect, or the implementation order needs adjustment.

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

@Dogbu-cyber Dogbu-cyber Feb 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This is expected behavior?
Extraction from managed services (including Appsync) is higher priority than trace context within event.headers.

Copy link
Contributor

@duncanista duncanista Feb 5, 2026

Choose a reason for hiding this comment

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

Maybe it just bugged due to the changes

}

#[test]
fn test_extract_span_context_no_request_headers() {
let config = Arc::new(config::Config {
trace_propagation_style_extract: vec![
config::trace_propagation_style::TracePropagationStyle::Datadog,
config::trace_propagation_style::TracePropagationStyle::TraceContext,
],
..config::Config::default()
});
let propagator = Arc::new(DatadogCompositePropagator::new(Arc::clone(&config)));
let headers = HashMap::new();

let payload = json!({
"argumentsMap": {
"id": "123"
},
"request": {
"body": "some body"
}
});

let result = Processor::extract_span_context(&headers, &payload, propagator);

assert!(
result.is_none(),
"Should return None when no trace context found"
);
}
}
Loading