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

Get opentelemetry trace id from request headers instead of creating a new trace #2376

Open
ptanov opened this issue Aug 8, 2024 · 14 comments · May be fixed by #2648
Open

Get opentelemetry trace id from request headers instead of creating a new trace #2376

ptanov opened this issue Aug 8, 2024 · 14 comments · May be fixed by #2648
Assignees

Comments

@ptanov
Copy link

ptanov commented Aug 8, 2024

Feature request

Currently a new trace is created per each HTTP request. It would be useful if trace is get (if available) from the request using traceparent header as defined in https://opentelemetry.io/docs/specs/otel/context/api-propagators/#propagators-distribution https://www.w3.org/TR/trace-context/

In this way you will be able to trace the request through microservice invocations.

Motivation

I want to track the request using opentelemetry starting from frontend, backend and then TGI

Your contribution

I don't have enough understanding of TGI code

@ErikKaum
Copy link
Member

ErikKaum commented Aug 8, 2024

Hi @ptanov 👋

Thanks for opening the issue 🙌

Gotcha, I think this is a solid motivation. I might find some time to implement it. But could you help me a bit on the road with understanding the traceparent header.

So a request comes in with a header named traceparent and then we should set this in the span in TGI? Do I understand it correctly?

@ErikKaum ErikKaum self-assigned this Aug 9, 2024
@noyoshi
Copy link

noyoshi commented Aug 15, 2024

@ErikKaum yeah essentially, most frameworks do this automatically and I am surprised the axum opentelemetry integration doesn't just do this automatically.

Specifically, you usually check to see if there is an "active trace" before starting a new one, so wherever the integration is starting a new trace/determining if there is an active trace is where you'd probably need to look to see if it even knows about active traces.

Ideally, there is some middleware layer that parses out the trace from the header and sets the "active trace" immediately.

@ptanov
Copy link
Author

ptanov commented Aug 15, 2024

Hi @ErikKaum ,
sorry for the late reply :(

Yes, it is exactly as described by @noyoshi - it is usually done automatically by the framework.

The format of the content of the header is described here: https://www.w3.org/TR/trace-context/#traceparent-header

@ErikKaum
Copy link
Member

No worries @ptanov, it's been quite a busy week so (un)fortunately I haven't gotten to this yet 😅

Thanks for the clarification @ptanov 🙌 A bit surprising indeed!

I think I won't be able to get to this this week, but most likely end of next week.

@ptanov
Copy link
Author

ptanov commented Aug 16, 2024

Thank you!

One more thing that could be useful to you- usually if trace is not automatically propagated by the framework (e.g. by .layer(OtelInResponseLayer::default()) and .layer(OtelAxumLayer::default()) in ./router/src/server.rs) it could be extracted and reused manually with something like:

    let extractor = HeaderExtractor(headers);
    let propagator = global::get_text_map_propagator(|propagator| propagator.clone());
    let span = propagator.extract(&extractor);
    let _guard = span.make_current();

assuming that global::set_text_map_propagator(TraceContextPropagator::new()); is set in ./router/src/logging.rs

but I couldn't make it work :( Sorry I'm not familiar with TGI codebase nor with rust.

I'm testing it by providing tracepoint header to curl, e.g. curl -v tgihostport/v1/chat/completions -H 'traceparent: 00-aa1274dfd3752a0f67b3bc3f21c71aaa-c8480007d4f1cbbb-01' -H 'Content-Type: application/json' -d '{SOMEQUESTION}'

@ErikKaum
Copy link
Member

Thank you @ptanov 🚀 this already helps a lot!

@ptanov
Copy link
Author

ptanov commented Sep 23, 2024

Hi @ErikKaum,
can I help you with something regarding this ticket (e.g. docker-compose configuration for running collector/kibana/prometheus)?

@ErikKaum
Copy link
Member

Hi @ptanov

Sorry for the radio silence, unfortunately it's been very busy lately and I haven't been able to prioritize this 🙁

@ptanov
Copy link
Author

ptanov commented Sep 26, 2024

Hi @ErikKaum ,
don't worry, take your time

73 ;)

@kozistr
Copy link

kozistr commented Oct 14, 2024

@ptanov, @ErikKaum

Hi, I just opened a PR for this feature in the TEI repo here! If you are available, please check it out and feel free to leave a review!

I'm new to OpenTelemetry things, so maybe my implementation could be incorrect or inefficient.

@ptanov
Copy link
Author

ptanov commented Oct 15, 2024

Thanks @kozistr ,
I'll try to check it tomorrow

@ptanov
Copy link
Author

ptanov commented Oct 16, 2024

Hi @kozistr @ErikKaum ,

I tested this PR and I can confirm that it works in my case - opentelemetry trace is propagated from request to TGI.

Thanks again @kozistr !

@ErikKaum
Copy link
Member

Big thanks @kozistr 🙌

@kozistr
Copy link

kozistr commented Oct 16, 2024

Hi @kozistr @ErikKaum ,

I tested this PR and I can confirm that it works in my case - opentelemetry trace is propagated from request to TGI.

Thanks again @kozistr !

great to hear! thanks for tesing this PR :)

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