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

otelhttp.NewHandler adds backpropagation #2163

Closed
pellared opened this issue Apr 6, 2022 · 2 comments · Fixed by #2180
Closed

otelhttp.NewHandler adds backpropagation #2163

pellared opened this issue Apr 6, 2022 · 2 comments · Fixed by #2180
Assignees
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects

Comments

@pellared
Copy link
Member

pellared commented Apr 6, 2022

This code makes a "backpropagation" by adding the propagation headers to the HTTP response.

w.props.Inject(w.ctx, propagation.HeaderCarrier(w.Header()))

I feel that it is not right as it is a leakage of the internals.

I am not aware of any other OTel instrumentation (in whatever language/ecosystem) that adds "backpropagation".

I think that it should not work that way by default. Additionally, such an option should first be described in the spec. I understand that such a feature may be useful for implementing "Real User Monitoring".

@pellared pellared added bug Something isn't working area: instrumentation Related to an instrumentation package labels Apr 6, 2022
@pellared pellared added this to Needs triage in Bugs via automation Apr 6, 2022
@MrAlias
Copy link
Contributor

MrAlias commented Apr 7, 2022

It looks like this was introduced quite a while ago: open-telemetry/opentelemetry-go#218. There isn't an explicit reason it was added there.

Let's remove this as it is unspecified behavior.

@MrAlias MrAlias moved this from Needs triage to Low priority in Bugs Apr 7, 2022
@MrAlias MrAlias added good first issue Good for newcomers help wanted Extra attention is needed labels Apr 7, 2022
@TylerHelmuth
Copy link
Member

I can take a look at this.

Bugs automation moved this from Low priority to Closed Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: instrumentation Related to an instrumentation package bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
Bugs
Closed
Development

Successfully merging a pull request may close this issue.

3 participants