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

Fix otelhttptrace example to avoid duplicating the generated data #5564

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dmathieu
Copy link
Member

Closes #5562

This example shows both ways to setup otelhttptrace, but WithClientTrace appends tracers, it does not replace them. So someone running the example gets duplicated data, which is confusing.

I have chosen to remove the example from the client, as that's our own option. Passing it to the context is "the httptrace way".

@dmathieu dmathieu force-pushed the otelhttptrace-example-single-tracer branch from 36f5302 to d734223 Compare May 13, 2024 08:40
@dmathieu dmathieu marked this pull request as ready for review May 13, 2024 08:41
@dmathieu dmathieu requested a review from a team as a code owner May 13, 2024 08:41
@zdyj3170101136
Copy link

Closes #5562

This example shows both ways to setup otelhttptrace, but WithClientTrace appends tracers, it does not replace them. So someone running the example gets duplicated data, which is confusing.

I have chosen to remove the example from the client, as that's our own option. Passing it to the context is "the httptrace way".

i recommend you reserve the first way.

first way:

Transport: otelhttp.NewTransport(
			http.DefaultTransport,
			otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
				return otelhttptrace.NewClientTrace(ctx)
			}),
		),

截屏2024-05-14 下午2 38 28

second way:

req, _ := http.NewRequestWithContext(ctx, "GET", *url, nil)

截屏2024-05-14 下午2 38 42

the first way's jaeger ui is easier for user to understand.

and is much easier for user to use(it could just replace global http.Transport)

@dmathieu
Copy link
Member Author

You're right, thank you.
When we add it to the context before the HTTP span is started, the used context doesn't have the proper span parent.

@dmathieu dmathieu requested review from dashpole and MrAlias May 14, 2024 08:40
@dmathieu dmathieu force-pushed the otelhttptrace-example-single-tracer branch from 69ece88 to 99488d8 Compare May 14, 2024 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

why otelhttptrace duplicate?
4 participants