Spring-cloud-sleuth: [enhancement] Extract baggage entries from HTTP headers and manage baggage entries as SLF4J MDC entries

[fabriziocucci]

Scenario

Suppose we have a legacy service-x (out of our jurisdiction) which need to call a service-a (in our jurisdiction) which, in turn, will need to call other downstream services (also in our jurisdiction). Every request that service-x sends to service-a contains a custom HTTP header (e.g. X-Tenant-Id).

We would like such header to be:

1. extracted and propagated from service-a to the downstream services;
2. included in the log traces, together with the default distributed tracing info (e.g. trace id, span id, etc).

Current Approach

For the goal 1, I've implemented a custom GenericFilterBean in the service-a to extract the header and add its value to the span context as a baggage key-value pair. This is sufficient to propagate the tenant identifier to the downstream services because the span context travels together with the trace and is attached to every span.

For the goal 2, I've implemented a custom SpanLogger which adds to and removes from the MDC the tenant identifier (as it's currently done for trace id, span id, etc). And I've also updated the logging pattern accordingly.

First things first, is this the correct approach (in the context of the 1.3.3.RELEASE version)?

Enhancement Requests

1. Add a property to automatically extract HTTP headers and put them in the baggage, e.g.

spring.sleuth.http.baggage.fromHeaders:
  - X-Tenant-Id

With the above configuration, if the application receives the HTTP header X-Tenant-Id, it will add the same to the baggage of the span context.

1. Add a property to automatically manage key-value pairs in the baggage as MDC key-value pairs, e.g.

spring.sleuth.log.slf4j.mdc.fromBaggage:
  - X-Tenant-Id

With the above configuration, MDC will be updated properly by adding and removing the key-value pair related to the X-Tenant-Id when necessary (as it's currently done for trace id, span id, etc).

Sample Code

• https://github.com/fabriziocucci/spike-spring-cloud-sleuth

Notes

• I'd be happy to contribute the enhancements if you feel like they are general enough and they make sense in the context of Sleuth.

[marcingrzejszczak]

First things first, is this the correct approach (in the context of the 1.3.3.RELEASE version)?

Yeah, that's correct.

Add a property to automatically extract HTTP headers and put them in the baggage, e.g.

If you check out 2.0.0 (https://cloud.spring.io/spring-cloud-static/Finchley.RC2/single/spring-cloud.html#_prefixed_fields) the automated passing of baggage is done via spring.sleuth.baggage-keys or spring.sleuth.propagation-keys.

Add a property to automatically manage key-value pairs in the baggage as MDC key-value pairs, e.g.

I'm unsure about this. You can extend the MDC in so many ways, that I'd prefer the user to just provide their own implementation. WDYT @adriancole ?

[fabriziocucci]

If you check out 2.0.0 (https://cloud.spring.io/spring-cloud-static/Finchley.RC2/single/spring-cloud.html#_prefixed_fields) the automated passing of baggage is done via spring.sleuth.baggage-keys or spring.sleuth.propagation-keys.

Thanks a lot for the pointer, I missed that because I may be stuck on the 1.x for a while...

I'm unsure about this. You can extend the MDC in so many ways, that I'd prefer the user to just provide their own implementation.

What I had in mind is nothing fancier than what is currently done here, i.e. injecting the array of headers coming from the properties in the Slf4jCurrentTraceContext, putting and removing such headers in the MDC like it's currently done for the other info (trace id, span id, etc).

[marcingrzejszczak]

Thanks a lot for the pointer, I missed that because I may be stuck on the 1.x for a while...

👍

What I had in mind is nothing fancier than what is currently done here, i.e. injecting the array of headers coming from the properties in the Slf4jCurrentTraceContext, putting and removing such headers in the MDC like it's currently done for the other info (trace id, span id, etc).

We'll give it a thought. If there are more people who want sth similar we'll try to figure things out. Currently it's basically enough to copy the class and register your own bean that does exactly what you need.

[fabriziocucci]

Currently it's basically enough to copy the class and register your own bean that does exactly what you need.

You mean copying the whole Slf4jCurrentTraceContext, adding a couple MDC.put(...) and MDC.remove(...) and registering the bean so that the default implementation doesn't kick in?

Yes, that is totally doable but _maybe_ the configuration option would be a bit nicer? :sweat_smile:

Thanks for adding this to the backlog and, again, I'd be happy to contribute with a PR.

[codefromthecrypt]

I don't think this issue should be closed at the moment, but in the middle
of something big plus jetlag so hard to dig into it. please nag me if I
don't respond by Monday ok?

On Thu, 7 Jun 2018, 06:36 Fabrizio Cucci, [e-mail] wrote:

Currently it's basically enough to copy the class and register your own
bean that does exactly what you need.

You mean copying the whole Slf4jCurrentTraceContext
https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jCurrentTraceContext.java,
adding a couple MDC.put(...) and MDC.remove(...) and registering the bean
so that the default implementation doesn't kick in?

Yes, that is totally doable but maybe the configuration option would be
a bit nicer? 😅

Thanks for adding this to the backlog and, again, I'd be happy to
contribute with a PR.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/spring-cloud/spring-cloud-sleuth/issues/1003#issuecomment-395422603,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD612wtAevkILZ_uPTSwH1GXIhYxBKyks5t6SxVgaJpZM4UeXFr
.

[dhoffi]

this seems to be related/same as my https://github.com/spring-cloud/spring-cloud-sleuth/issues/927 isn't it?

[marcingrzejszczak]

Yeah, it looks kind of similar.

[fabriziocucci]

please nag me if I don't respond by Monday ok?

@adriancole any post-jetlag thoughts on this? :)

[codefromthecrypt]

sorry I read through your issue and the part about managing with MDC is
similar to https://github.com/openzipkin/brave/pull/577

[matthewjmason]

I think this would be a really useful feature. Where I work, we currently have a homegrown solution for traceability that relies on custom MDC fields. There is interest in leveraging sleuth as a replacement for some of the functionality but the MDC fields will have to be supported in any long term solution due to existing contracts.

[codefromthecrypt]

do you mind elaborating exactly what you would be doing with this feature?
for example do you have properties that are just getting to MDC and not
intended to leave the process? Which properties if any are based on data in
the span etc? If you have a custom workaround now, could you post a
gist.github.com or sample project of it?

On Fri, 22 Jun 2018, 20:17 Matthew Mason, [e-mail] wrote:

I think this would be a really useful feature. Where I work, we currently
have a homegrown solution for traceability that relies on custom MDC
fields. There is interest in leveraging sleuth as a replacement for some of
the functionality but the MDC fields will have to be supported in any long
term solution due to existing contracts.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/spring-cloud/spring-cloud-sleuth/issues/1003#issuecomment-399423440,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD611S9ykGAGvyex6Bg15pum5KD49J5ks5t_OBygaJpZM4UeXFr
.

[codefromthecrypt]

very likely is a dupe but we do need folks to aay what they need.

as collecting feedback in different issues is troublesome. please reply with your use case on #927 or https://github.com/openzipkin/brave/pull/577

[mattmadhavan]

Hi Marcin,
Your quote - "We'll give it a thought. If there are more people who want sth similar we'll try to figure things out. Currently it's basically enough to copy the class and register your own bean that does exactly what you need."

Can you please let me know the steps to accomplish the same? I have a very similar requirements . I have have a need to add/remove configurable number of fields to be copied/removed from MDC.

Any help will be appreciated. Thx in advance!

[marcingrzejszczak]

It's done and will be available with 2.1.0.M1 - https://github.com/spring-cloud/spring-cloud-sleuth/issues/1071

[dhoffi]

is there some documentation on how to achieve it with 2.1.0.M1+
or will all baggage-keys automagically be put in MDC context??

addendum:
oh, I found it:
application.properties.log.slf4.whitelisted-mdc-keys

[marcingrzejszczak]

https://cloud.spring.io/spring-cloud-sleuth/single/spring-cloud-sleuth.html#prefixed-fields

[mattmadhavan]

Hi Marcin
I am now using SpringBoot 2.1.0.RELEASE and Finchley.RELEASE version for Spring Cloud .
Have the following props:
spring.sleuth.propagation-keys=vin, customerId
spring.sleuth.log.slf4j.whitelisted-mdc-keys=vin, customerId

I am able to do successfully add remove a baggage prop;
ExtraFieldPropagation.set("customerId", "matt-m");
String retValue = ExtraFieldPropagation.get("customerId");

How ever the MDC prop - spring.sleuth.log.slf4j.whitelisted-mdc-keys does not work. I checked the ThreadLocal immediately after I set the Prop and its not there. Any ideas Please? Is there a sample I can see?

Thanks in advance!
Matt

[mattmadhavan]

Dude please! I am now debugging thro' your code to see whats not working? Show some clarity please! The time I spent on this I could have written a Framework my self. Either help me (its your s/w and you represent Spring) or challenge me! I will learn the whole Brave and other spec and make you proud

[codefromthecrypt]

gitter is a better place for troubleshooting than issues. issues spam all
watchers. understand you are frustrated but it doesn't serve you either to
emote at an overworked maintainer.

additional MDC entries become visible the next time the span is placed in
scope

https://cloud.spring.io/spring-cloud-sleuth/single/spring-cloud-sleuth.html#_setting_a_span_in_scope_manually

On Sat, 10 Nov 2018, 12:06 mattmadhavan, [e-mail] wrote:

Dude please! I am now debugging thro' your code to see whats not working?
Show some clarity please! The time I spent on this I could have written a
Framework my self. Either help me (its your s/w and you represent Spring)
or challenge me! I will learn the whole Brave and other spec and make you
proud

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/spring-cloud/spring-cloud-sleuth/issues/1003#issuecomment-437556515,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD61wY9DyNEx464blK47ociV_VVblXwks5utlDegaJpZM4UeXFr
.

[marcingrzejszczak]

You're using boot 2.1 with Finchley which is wrong. You have to use boot 2.1 with Greenwich release train

[mattmadhavan]

Hi Marcin,
I think I might have found the issue. When you are copying the MDC variables you are checking for only checking for baggageNames and not propagation keys!

private Stream<String> whitelistedBaggageKeys(TraceContext context) {
        return this.sleuthProperties.getBaggageKeys().stream().filter(
                (s) -> this.sleuthSlf4jProperties.getWhitelistedMdcKeys().contains(s)
                        && context != null
                        && StringUtils.hasText(ExtraFieldPropagation.get(context, s)));
    }

I am not sure if this is as intended.

One more potential issue!
When you add /modify a propogation fieldas follows
public static void set(TraceContext context, String name, String value) { PropagationFields.put(context, lowercase(name), value, Extra.class); }

• You are not updating the baggage value - The above issue with remains
• Also don't you think you need to update the MDC also if they are mdc whitelisted?

Please advise!

Thanks
Matt

[codefromthecrypt]

the propagation keys used for the trace context are not stable. ex if using gcp they will be redundant etc with google headers, similar when folks use X-Ray interop. plus MDC isn't free.. surprisingly more expensive from overhead than you'd guess.

Probably best to not add a feature that encourages locking into certain header names. make sense?

[mattmadhavan]

Hi Adrian,
I am not even sure this the page to discuss these things? Is there any forum I can ask questions/share views?

The goal of this thread is to copy the baggage/propagation values to the MDC. The code as I showed above depends on the baggage property! But rest of the code/doc emphasize on the propogations properties.

The document is not clear at all - differentiating between baggage/propagation. Only values defined under baggage variables get copied to MDC!

When I add a new variable to Span - only propogation prop is affected (Extra variables) and thus mdc white list is ignored and its not copied to to MDC even if it is listed in mdc variable.

I am not able to build the Sleuth source code. I can contribute - please let me know

[mattmadhavan]

Also the document is not clear defining baggage variables for JMS calls! Do I need to use baggage or propagation. I am kinda' familiar with the Sleuth Code now - and comfortable with Brave too. I need these features - I can help. Please let me know. I need them

[codefromthecrypt]

@matthewjmason yeah commenting on closed issues are generally not a good way to try to change design etc :P JMS has constraints which begs the need for a different sort of carrier for properties, one that doesn't imply camel-like name dodging. The community has discussed options for JMS in the past. As you'll notice the amount of your questions is indeed wandering off into several related dimensions of functionality and practice. I would recommend using chat for starters. I might not be able to answer all of your questions in one go, but can try to help answer some of them. https://gitter.im/spring-cloud/spring-cloud-sleuth