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

WIP: propagation scopes #18

Closed
wants to merge 1 commit into from
Closed

WIP: propagation scopes #18

wants to merge 1 commit into from

Conversation

tpolecat
Copy link
Member

@tpolecat tpolecat commented Aug 30, 2019

This updates put to take a Propagation value that explains how the fields should be propagated to child spans. Resolves #17

Value Meaning
Propagation.None fields will not be propagated to child spans
Propagation.Local fields will be propagated to child spans, but will not cross network boundaries
Propagation.Extended fields will be propagated to child spans, and across network boundaries

Partially implemented for Honeycomb. Parameter currently ignored for other back ends.

/** Fields will not be propagated to child spans. */
case object None extends Propagation

/** Fields will be propagated to child spans, but will not cross network boundaries. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a case for propagating Local fields back out to parent spans as well as into child spans? For example, I have a situation where I'm inserting an entity into the db, allocating a primary key at that point, and I'm adding the generated key to the span that wraps the db call. But it would be valuable if that key also appeared on the parent spans so that I can easily cross-reference them.

I can see, though, that there would be cases to address such as what to do if the parent span already has a value for a Local field set by a child span.

case object Local extends Propagation

/** Fields will be propagated to child spans, and across network boundaries. */
case object Extended extends Propagation
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any thoughts as to how these Extended params would be made available to things like the http4s middleware so that they can be propagated across the network boundary? I imagine one way is that they could be merged onto the Kernel that the middleware pulls off the trace.

Also, any thoughts about the mechanism the http4s middleware might use to do the propagation? Each field could become a header in the request (perhaps prefixing with X-Natchez-Field- to avoid fields being set with names that clash with http headers?) or a single X-Natches-Fields header that encodes the name/value pairs into a single string?

One specific thing I'd really like to see, though, is a way to add a field that ends up being set as the X-Correlation-Id header on http calls. Maybe that ends up as logic in the middleware to extract a value with a known name from the kernel.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, they become part of the kernel. Prefixing with X-Natchez-Field is a good idea. We could set the trace ID as the correlation ID although this would need to be optional because it's an existing header people may already be using. I don't really see the point of using both tracing and a correlation ID but I guess people do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the code I'm working on I'm setting the trace id to be the correlation id on outbound requests, and using an incoming correlation id to set the trace id if it's present, unless there's already an X-Natchez-Trace-ID header, in which case the correlation id gets lost. Ideally it should never be the case that I get an incoming call with both X-Correlation-ID and X-Natchez-Trace-ID if I've managed the code well, but it makes me nervous that it's possible :-)

@mpilquist mpilquist closed this Nov 5, 2023
@bpholt bpholt deleted the propagation branch October 4, 2024 20:00
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.

Propagate span values to children spans
3 participants