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

Return traceId in header response. #11931

Open
dcdh opened this issue Sep 7, 2020 · 20 comments
Open

Return traceId in header response. #11931

dcdh opened this issue Sep 7, 2020 · 20 comments

Comments

@dcdh
Copy link
Contributor

dcdh commented Sep 7, 2020

Description
To debug more easily an Ajax request I propose to return the traceId in response header.

image

image

image

Implementation ideas

I've follow this code https://github.com/quarkusio/quarkus/blob/master/extensions/jaeger/runtime/src/main/java/io/quarkus/jaeger/runtime/MDCScope.java

package com.damdamdeo.todo.publicfrontend.interfaces;

import io.jaegertracing.internal.JaegerSpanContext;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;

import javax.inject.Inject;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerResponseContext;
import javax.ws.rs.container.ContainerResponseFilter;
import javax.ws.rs.ext.Provider;
import java.io.IOException;

@Provider
public class TraceIdResponseInjector implements ContainerResponseFilter {

    @Inject
    Tracer tracer;

    private static final String TRACE_ID = "traceId";

    @Override
    public void filter(final ContainerRequestContext requestContext, final ContainerResponseContext responseContext) throws IOException {
        final SpanContext spanContext = tracer.scopeManager().active().span().context();
        if (spanContext instanceof JaegerSpanContext) {
            responseContext.getHeaders().add(TRACE_ID, ((JaegerSpanContext) spanContext).getTraceId());
        }
    }

}

I guess it should be implemented in quarkus-smallrye-opentracing.

Regards,

Damien

@quarkusbot
Copy link

/cc @kenfinnigan, @Ladicek

dcdh added a commit to dcdh/eventstore-quarkus-sample-app that referenced this issue Sep 7, 2020
@Ladicek
Copy link
Contributor

Ladicek commented Sep 7, 2020

I think this makes sense, though there probably should be a configuration switch to turn it off. (I think it's pretty safe to have it on by default, even in production, but some users in highly regulated environments would probably like to turn it off.)

CC @pavolloffay

@kenfinnigan
Copy link
Member

I'd prefer it the other way around, off by default except when running with quarkus:dev which enables it.

@dcdh can you elaborate on the use case? Is it purely for testing in development, or some other reason?

@dcdh
Copy link
Contributor Author

dcdh commented Sep 7, 2020

@kenfinnigan I am working for a client for an internal platform compound of several micro-services. We use the Spring Boot + Spring Cloud Sleuth ... stack. Spring Cloud Sleuth add in our response header some data coming from opentracing like x-b3-traceid.
It was added to help support team to easier trace the flow of interaction between our multiple micro-services from the frontend. So when we figure out an issue from the front, we take the control of the user's computer and try again displaying the Ajax call and retrieve the x-b3-traceid. Likes this it help use to retrieve directly traces in Zipkin and logs in Kibana.
Also it is very useful when you want to debug or do a demo to display all the calls from action as you can easily trace them from front using your browser inspector and zipkin.

@Ladicek
Copy link
Contributor

Ladicek commented Sep 8, 2020

Tracing is, by definition, meant for production. I just wonder what would happen in case the tracer decides not to sample the trace. I don't know, but I hope that there's a way to detect that and either not include the header in the response at all, or somehow indicate that the trace is not sampled.

@pavolloffay
Copy link
Contributor

This seems like a good feature.

@kenfinnigan
Copy link
Member

@dcdh, @Ladicek raised a good point around sampling.

When you're using Spring Cloud Sleuth, do you sample all the requests? What happens if you're not sampling them all?

@pavolloffay
Copy link
Contributor

The IDs should be generated when though the trace is not being sampled. We could propagate the sampling bit also to inform the caller.

@dcdh
Copy link
Contributor Author

dcdh commented Sep 17, 2020

Ok, so I was speaking about b3 propagation. Info about specification can be found here https://github.com/openzipkin/b3-propagation
I've tried to implement it using Spring cloud sleuth without success. I need to check how it was done in my job client.

@Ladicek
Copy link
Contributor

Ladicek commented Sep 22, 2020

The IDs should be generated when though the trace is not being sampled. We could propagate the sampling bit also to inform the caller.

Yea, sorry for not being clear. I understand that each trace has an ID, even if the tracer decides to not sample it. I was just wondering what should we do in such case, or if there's even something we can do.

@Legion2
Copy link

Legion2 commented Nov 4, 2021

For the W3C Trace Context there was a spec added for response headers w3c/trace-context#365, so it should also be added to the W3C propagation.

@brunobat brunobat added the good first issue Good for newcomers label Jul 26, 2022
@melloware
Copy link
Contributor

Yes similar to Spring Boot it should respect incoming HTTP headers for Trace/Span and always put them on the outgoing response. This way you can trace microservice to microservice calls across your organization. This is similar to what Spring Boot Slueth does.

@mfpc
Copy link
Contributor

mfpc commented Jan 31, 2023

Could anyone confirm if this is correct, Should I create a PR ?

smallrye/smallrye-opentracing@main...mfpc:smallrye-opentracing:traceID-header-response

@Legion2
Copy link

Legion2 commented Mar 19, 2023

According to this w3c draft the response header should be called traceresponse and not only contain the trace-id.

@juniormichieletto
Copy link

juniormichieletto commented May 21, 2023

Good point @dcdh in this proposal!
Actually we are doing this manually in some projects 😅 and it was useful in some moments to use 😃
Also, could be nice the same in "opentelemetry" extension (if is not done at this moment) since the "openttracing" extension is marked to be discontinued.

Ps. I also undestand with you proposal doesn't should not affect any client propagation headers that should keep working as before, respecting the propagation strategy like x-b3, w3 or other.
Am I right?

https://quarkus.io/guides/opentracing
image

@mcruzdev
Copy link
Contributor

mcruzdev commented Jun 11, 2023

Hi @brunobat how are you?
Can I get this one... or is there some definitions to get before?

@brunobat
Copy link
Contributor

I'd say, given the discussion, there is no consensus and I don't see this as a priority, for now.

@brunobat
Copy link
Contributor

brunobat commented Sep 4, 2023

Let's try to move on with this.
@ahreurink Would you like to give it a try to this one?
Please mind that:

  1. This needs to be activated by a config property
  2. Let's try traceresponse and see how it goes. See comments bellow. [EDIT]
  3. This needs to be returned both by Resteasy Classic and Ractive

Try first to create the tests to what you expect.

@Legion2
Copy link

Legion2 commented Sep 5, 2023

As mentioned earlier, there is an open W3C draft for a standardized response header https://w3c.github.io/trace-context/#traceresponse-header

@brunobat
Copy link
Contributor

brunobat commented Sep 5, 2023

You are right @Legion2.
Researched this a bit more and there is no mention of the usage of the traceresponse header in the OTel spec or it's Java implementation.
I guess we should align with the W3C spec for the https://w3c.github.io/trace-context/#trace-context-http-response-headers-format and implement what's in there.
The implementation could draw inspiration from io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator

@brunobat brunobat removed the good first issue Good for newcomers label Sep 13, 2023
wottop added a commit to wottop/quarkus that referenced this issue Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests