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

otelhttptrace is not concurrent-safe #3893

Closed
pellared opened this issue May 30, 2023 · 3 comments
Closed

otelhttptrace is not concurrent-safe #3893

pellared opened this issue May 30, 2023 · 3 comments
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@pellared
Copy link
Member

From https://pkg.go.dev/net/http/httptrace#ClientTrace:

Functions may be called concurrently from different goroutines and some may be called after the request has completed or failed.

If I understand this correctly it may even mean that the current implementation may contain logical race conditions as I do not see how activeHooks synchronizes multiple concurrent HTTP requests.

Originally posted by @pellared in #3892 (review)

@pellared
Copy link
Member Author

It would be good to first write a test that would confirm the issue.

@pellared pellared added the bug Something isn't working label May 30, 2023
@Alkorin
Copy link
Contributor

Alkorin commented May 30, 2023

Except if I missed something, each http request allocate its own httptrace.ClientTrace

Either with

	ctx = httptrace.WithClientTrace(ctx, otelhttptrace.NewClientTrace(ctx))
	req, err := http.NewRequestWithContext(ctx, ...)

Or with otelhttp.WithClientTrace option of otelhttp.NewTransport which call httptrace.WithClientTrace(ctx, t.clientTrace(ctx)) inside Transport.RoundTrip

otelhttptrace.NewClientTrace must not be used on several http requests

@pellared
Copy link
Member Author

You are right. It is me who missed something 😉 Thanks ❤️

@pellared pellared added the invalid This doesn't seem right label May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants