Skip to content
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

Implement trace propagation #49

Closed
wants to merge 8 commits into from

Conversation

danieljharvey
Copy link
Contributor

@danieljharvey danieljharvey commented Sep 27, 2023

This PR implements trace propagation, adds a MakeSpan trait that grab any traceparent and tracestate headers, and injecting them into span context. The result is that requests from v3-engine show up in NDC agents as follows:

Screenshot 2023-09-29 at 12 52 35

@@ -331,10 +333,7 @@ where
});

router
.layer(
TraceLayer::new_for_http()
.make_span_with(DefaultMakeSpan::default().level(Level::INFO)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this make_span_with means we default to the default TraceLayer implementation, which makes all error responses into error traces rather than always INFO, solving #43

Info: https://docs.rs/tower-http/latest/tower_http/trace/struct.TraceLayer.html#method.new_for_http

pub struct PropagateParentTraces;

impl<B> MakeSpan<B> for PropagateParentTraces {
fn make_span(&mut self, req: &Request<B>) -> Span {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This grabs the W3 trace header (ie, traceheader and tracestate) and adds them to the context of any spans we create, meaning we can follow between services.

@danieljharvey danieljharvey changed the title Add axum-tracing-opentelemetry to add parent traces etc Fix trace propagation Sep 29, 2023
@danieljharvey danieljharvey changed the title Fix trace propagation Implement trace propagation Sep 29, 2023
@danieljharvey danieljharvey marked this pull request as ready for review September 29, 2023 15:00
@@ -0,0 +1,23 @@
# re-build on code changes, and run the reference agent each time a build is
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A few quality-of-life just commands. Happy to move these into a separate PR if needed.

@danieljharvey
Copy link
Contributor Author

Just realised there is already an implementation in ndc-multitenant, will copy that instead for the sake of consistency: #49

@danieljharvey
Copy link
Contributor Author

Closing in favour of #51

@daniel-chambers daniel-chambers deleted the djh/attempt-trace-propagation branch March 22, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant