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

Headers not always propagated to MDC (with sample) #2098

Closed
anderius opened this issue Jan 12, 2022 · 11 comments
Closed

Headers not always propagated to MDC (with sample) #2098

anderius opened this issue Jan 12, 2022 · 11 comments
Labels

Comments

@anderius
Copy link

anderius commented Jan 12, 2022

I want to read header values, and have them available on MDC. One single request works, but not when a test does many requests after each other.

Complete project that reproduces the error every time (on my computer, Java 17). Most often after only 6-7 iterations:

https://github.com/anderius/spring-sleuth-problem-demo

Excerpt from the sample:

@RestController
@SpringBootApplication
class DemoApplication {

    @Bean
    fun sleuthHttpServerRequestParser() = HttpRequestParser { request, context, _ ->
        request.header("customHeader")
            ?.let { BaggageField.create("customField").updateValue(context, it) }
    }

    @Bean
    fun spanHandler(): SpanHandler = SpanHandler.NOOP

    @GetMapping("/mdc")
    fun getMdc(): Map<String, String> = MDC.getCopyOfContextMap()
}

This issue might be related to #2064.

@marcingrzejszczak
Copy link
Contributor

Can you please rewrite this to java?

@anderius
Copy link
Author

Sure, tomorrow. :-) I use the Spring Start project to generate the project. Any preference for Maven/Gradle, or Java version?

@marcingrzejszczak
Copy link
Contributor

Maven will be great!

@anderius
Copy link
Author

@marcingrzejszczak Maven and Java is ready! :-)

Project is same place: https://github.com/anderius/spring-sleuth-problem-demo

The setup in java:

@RestController
@SpringBootApplication
public class DemoApplication {

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @Bean
    HttpRequestParser sleuthHttpServerRequestParser() {
        return (request, context, span) -> {
            String headerValue = request.header("customHeader");
            if (headerValue != null) {
                BaggageField.create("customField").updateValue(context, headerValue);
            }
        };
    }

    @Bean
    SpanHandler spanHandler() {
        return SpanHandler.NOOP;
    }

    @GetMapping("/mdc")
    Map<String, String> getMdc() {
        return MDC.getCopyOfContextMap();
    }
}

@marcingrzejszczak
Copy link
Contributor

It took me a while but I understand why it's not working. So the default sampling strategy is rate based. You're sending requests to fast and after ~8 request sampling is noop so no baggage is set. The tests pass once you set spring.sleuth.sampler.probability=1.0. Then you sample all of them.

@anderius
Copy link
Author

anderius commented Jan 17, 2022

@marcingrzejszczak Thank you for looking into this!

That means Spring Cloud Sleuth should not be used for any headers other than the ones used for sampling/tracing, unless we sample everything.

I had the idea the sampling was only how much was sent to zipkin (or similar), but it is also how much is instrumentated.

@marcingrzejszczak
Copy link
Contributor

That's how Brave works, I'd need to double check if OTel works in the same way.

@anderius
Copy link
Author

It would be nice to be able to instrument/populate every request onto MDC, and still only send samples.

I will also have to check if this sampling affects propagated headers, as I was expecting that to happen every time as well.

@marcingrzejszczak
Copy link
Contributor

Can you change your code and check if this helps. You can leave the sampling condition as it is but instead of

 @Bean
    SpanHandler spanHandler() {
        return SpanHandler.NOOP;
    }

just create a new instance of a SpanHandler that does nothing (but don't call the NOOP reference).

@anderius
Copy link
Author

Same result, I am afraid.

@marcingrzejszczak
Copy link
Contributor

Meh. So it has to be sampled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants