Not seeing traceids in the http response headers

[bijukunjummen]

Hi, I can see from issue #327 that the HTTP response headers related to a trace is now removed from the response headers(the documentation still says that it should exist). I thought that this was very useful way to grab a traceid for user initiated request and then use it to go to zipkin and trace out the request. Is there a easier way to grab a traceid for a request initiated by a user. Sorry to open it as an issue, but felt that this is a useful feature to have

[marcingrzejszczak]

Yes that was deliberate due to security reasons. I'll update the docs to reflect that change. Of course you can create your own Filter that can add to the response the current span's data.

[bijukunjummen]

Thanks @marcingrzejszczak, makes sense. Do you want me to keep this open to track the documentation update?

[marcingrzejszczak]

Sure! Actually you can file a PR with an update yourself :D

[bijukunjummen]

Sure :-), will send a PR.

[bijukunjummen]

PR on its way - #429

[codefromthecrypt]

here's a related issue openzipkin/openzipkin.github.io#48

[marcingrzejszczak]

Done in 8437bef

[sanjeevchopra]

@marcingrzejszczak - can you please explain why is trace/span info in response headers considered sensitive information ?

[marcingrzejszczak]

It's not that these particular values are sensitive. Initially AFAIR we were passing back all header values including the trace and span. And that wasn't too safe from the security perspective. As for trace and span - there's no need to pass it since if you're doing an RPC then it's redundant information cause you've already set these values in the request. Does it make any sense?

[sanjeevchopra]

Just the trace-id will be useful in the response as a reference. In case of errors, consumers of a service can report the trace-id back to service owners/developers for troubleshooting. I realize this could be implemented by the service, though it would be useful as a general capability.

Consider this use-case:

• Say I offer a service, A to my users. This service(A) uses sleuth. The request to service A does not have a trace, so sleuth will initiate a trace. 'A' then calls some downstream services and eventually the request fails. It would be useful for the trace-id to go out as a response header to the original request.

(Cc: @sabertiger @anubhavi25)

[devinsba]

The existence of the headers is a security concern not the values in them. If this were to be a feature it must absolutely be intentionally opt-in, which the commit listed above 8437bef covers.

From my point of view, leaking those headers out makes your service a target if its publicly available. If an attacker can figure out you are using sleuth for instance they can try to affect your application in ways that are specific to sleuth

[sanjeevchopra]

Agreed. The risk would depend on the type of service and authentication/authorization it employs. Another mitigation could be that the developer provides a header name to populate with trace-id, so its not sleuth specific.
I'd be happy to submit a PR with a opt-in, if the project will consider it?

[marcingrzejszczak]

The opt-in is already there. https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java#L386-L400 you can provide your own response tags or set some values on the response itself

[sanjeevchopra]

@marcingrzejszczak Please elaborate on how I can use addResponseTags to add traceId in the response header?

[marcingrzejszczak]

You have access to the response object, you can retrieve the current span and add a header to the response

[sanjeevchopra]

So you are proposing to extend TraceFilter and override addResponseTags ? I agree.

The additional question is if this custom filter should be included in sleuth? A user would have to set a property for traceId to be included as a response header.

[marcingrzejszczak]

So you are proposing to extend TraceFilter and override addResponseTags ? I agree.

Yup - that's what I propose.

The additional question is if this custom filter should be included in sleuth? A user would have to set a property for traceId to be included as a response header.

We already had it included and for the reasons we've mentioned before we decided to remove it.

[sanjeevchopra]

sorry @marcingrzejszczak , this discussion came full-circle :)

If you see @devinsba 's comment above - we were discussing if there is value in doing a easier opt-in? For users, who don't have a security concern in including only the traceId in response headers, they could simply update a property, instead of having to create a custom TraceFilter.

[codefromthecrypt]

The most sensible route is allowing a user to choose a header that contains the hex-encoded trace ID.

ex. when "sleuth.whatever.trace-response-header=Customer-TraceId" is set, autoconfiguration fires such that "Customer-TraceId: ab13bfe3213b.." is set.

By not documenting this as use for X-B3-TraceId, rather a custom header used by customers who want to expose trace ID in the response, we..

• don't add more confusion around X-B3-TraceId being a response header as it never has been defined like that
• don't interfere when/if other propagation formats are in use (ex the customer-header is stable even if propagation headers change)

Bear in mind that this is a middleware concern. Tracing as exists today does not need to affect the data in the server response. IOTW, this broadens Sleuth's scope to a place where it currently isn't. It seems the intent here is to add a middleware component to sleuth as a convenience as opposed to having an externally hosted component that looks up the current trace ID and plops it as a response header (as I would suggest users of other libraries like brave to do on their own)

[sanjeevchopra]

+1 on "sleuth.whatever.trace-response-header=Customer-TraceId" . I dont know enough about sleuth to say, if this belongs here. Based on your response, it seems like it doesnt.

Thanks all, for the discussion.

[codefromthecrypt]

@sanjeevchopra my style is usually to emphasize when scope grows as that impacts all current and future http integration. ex other projects never touch the http server response, so can't break it. It is easy to miss this detail.

I do think this could be done as an outside project first, but that might not be feasible in practice. OTOH, there have been many asking for this, and it might be worth the scope creep to do it. We just need to consider carefully first is all. Thanks for your comments!

[marvinrichter]

BTW the solution @marcingrzejszczak suggested would be much cleaner if methods like tracer() were protected, too. Or the current span were given the method addResponseTag() as a parameter. :)

[marvinrichter]

I just tried it and this solution doesn't work. The reason is that in addResponseTag() the response is already committed. Does anyone have another solution?

[marcingrzejszczak]

Can you present exactly what you tried to do?

[marvinrichter]

Sure. Here is my custom filter implementation and the configuration.

public class CustomTraceFilter extends TraceFilter {

    private static final String TRACE_HEADER_NAME = "X-B3-TraceId";
    private final BeanFactory beanFactory;
    private Tracer tracer;

    public CustomTraceFilter(BeanFactory beanFactory) {
        super(beanFactory);
        this.beanFactory = beanFactory;
    }

    public CustomTraceFilter(BeanFactory beanFactory, Pattern skipPattern) {
        super(beanFactory, skipPattern);
        this.beanFactory = beanFactory;
    }

    @Override
    protected void addResponseTags(HttpServletResponse response, Throwable e) {
        super.addResponseTags(response, e);
        Span currentSpan = tracer().getCurrentSpan();
        response.addHeader(TRACE_HEADER_NAME, currentSpan.traceIdString());
    }

    private Tracer tracer() {
        if (this.tracer == null) {
            this.tracer = this.beanFactory.getBean(Tracer.class);
        }
        return this.tracer;
    }
}

@Configuration
public class TraceFilterConfig {

    @Bean
    public TraceFilter traceFilter(BeanFactory beanFactory) {
        return new CustomTraceFilter(beanFactory);
    }
}

[72MiguelGomes]

@marvinrichter Thanks for share your custom filter. I tried use it but addResponseTags is now private and can't be override.
This is the implementation that I used, if someone has the same problem:

public class CustomTraceFilter extends TraceFilter {

    private static final String TRACE_HEADER_NAME = "X-B3-TraceId";
    private final BeanFactory beanFactory;
    private Tracer tracer;

    public CustomTraceFilter(BeanFactory beanFactory) {
        super(beanFactory);
        this.beanFactory = beanFactory;
    }

    @Override
    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
        super.doFilter(servletRequest, servletResponse, new CustomFilterChain(filterChain));
    }

    private Tracer tracer() {
        if (this.tracer == null) {
            this.tracer = this.beanFactory.getBean(Tracer.class);
        }
        return this.tracer;
    }

    private class CustomFilterChain implements FilterChain {

        private final FilterChain filterChain;

        private CustomFilterChain(FilterChain filterChain) {
            this.filterChain = filterChain;
        }

        @Override
        public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse) throws IOException, ServletException {
            HttpServletResponse response = (HttpServletResponse)servletResponse;

            response.addHeader(TRACE_HEADER_NAME, tracer().currentSpan().context().traceIdString());

            this.filterChain.doFilter(servletRequest, servletResponse);
        }
    }
}

@Configuration
public class TraceFilterConfig {

    @Bean
    public TraceFilter traceFilter(BeanFactory beanFactory) {
        return new CustomTraceFilter(beanFactory);
    }

}

[marcingrzejszczak]

With Brave what you can do is register your own version of ServerParser. Example here

https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpServerParser.java

and its registration

https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java#L51-L59

That way you don't have to do any filters.

[codefromthecrypt]

clever idea. I hadn't thought of that, but yeah when the response type is ServletResponse, you certainly could add a header like like this when parsing. Note that this would only be called when the trace is sampled (at least until we do firehose mode), which is probably your intent anyway.

[venk410]

Hi,
I am trying to use the spring-cloud-sleuth in our built in micro services project, I have added the dependency in the pom and It is generating the traceid and spanId but when It hits the different service, It is not maintaining the same TraceId's. Anyone have idea?

[rastogirishabh]

@venk410 - I guess I'm late at the party but

1. You should have sleuth dependencies in both the project.
2. If you're hitting service2 from service1 using RestTemplate then RestTemplate object should be managed by Spring and you shouldn't create it via new keyword.