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

Access headers in processor #2401

Closed
pavolloffay opened this issue Jan 25, 2021 · 26 comments · Fixed by #4547
Closed

Access headers in processor #2401

pavolloffay opened this issue Jan 25, 2021 · 26 comments · Fixed by #4547

Comments

@pavolloffay
Copy link
Member

Is your feature request related to a problem? Please describe.

I would like to access HTTP headers in the processor pipeline and add attribute to span/resource - e.g. tenant ID.

Describe the solution you'd like

Read the headers from the context object in a processor. The headers which should be injected into the context could be specified in the receiver config.

Describe alternatives you've considered

A custom implementation of Zipkin/OTLP receiver that does that.

Additional context

Related issue #2101

@pavolloffay
Copy link
Member Author

import "google.golang.org/grpc/metadata"
md, ok := metadata.FromIncomingContext(ctx)

returns the HTTP headers for any gRPC receiver. However the processor cannot be used after the batch processor. The same approach uses https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/master/processor/routingprocessor

However, I also need to access the headers from Jaeger HTTP thrift (14268) receiver.

@pavolloffay
Copy link
Member Author

@bogdandrutu @tigrannajaryan could we add HTTP headers to the context in the HTTP based receivers?

@jcchavezs
Copy link
Contributor

jcchavezs commented Jan 26, 2021

I think if this goes forward there should be a way to access headers (metadata) decoupled from the receiver (e.g. a method to access it instead of the need to call grpc metadata.FromIncomingContext or protocol specific), that is probably the less implementation aware way.

@bogdandrutu
Copy link
Member

@pavolloffay we were just discussing this, and I don't know what is the right approach. The problem with adding it to the Context is that if batch span processor is the first processor that will drop the context, so most likely we have to carry this with the data itself and not the Context.

@pavolloffay
Copy link
Member Author

The problem with adding it to the Context is that if batch span processor is the first processor that will drop the context, so most likely we have to carry this with the data itself and not the Context.

Correct, I understand this as a limitation of the current design which is fine for us since the processor that access the headers will run immediately after the receiver.
The https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/master/processor/routingprocessor uses it as well and documents how the processor should be used.

Shall we start thinking on merging contexts in the batch processor or changing the API to bind context to each original tracedata?

@tigrannajaryan
Copy link
Member

Related to #2495 which has similar requirements.

@bogdandrutu
Copy link
Member

@jpkrohling is this related to the auth extensions pass-through? Can you take this?

@jpkrohling
Copy link
Member

jpkrohling commented May 17, 2021

This was requested earlier, and some aspects of this might be doable with the auth work. Once the passthrough PR is done, it should be easy to use the same technique to implement this, by storing the headers in the Resource* structs.

@PatrikSteuer
Copy link

Are there any thoughts on this? Or ideas how an implementation should look like?

@jpkrohling
Copy link
Member

We have the client package which is just being finished to propagate the auth information. This here could use the same facility in the future.

@bogdandrutu
Copy link
Member

@jpkrohling I do believe that this does not provide enough information about why this is needed. If somebody random comes and say I need "foo" are you going to implement it?

I am not suggesting that this is not important and we should not do it, but would be good to understand why this is needed.

@jpkrohling
Copy link
Member

If somebody random comes and say I need "foo" are you going to implement it?

No, this one has been requested a few times in the past already though. The typical reasoning is using the incoming HTTP headers down the pipeline to make tenancy decisions, either at the processor level (routing processor, for instance), or exporter level (passthrough of API keys).

@bogdandrutu
Copy link
Member

@jpkrohling awesome, but because of unnecessary clone cost in some cases I would recommend to do this opt-in. Have a Boolean in the HttpClientSettings to enable this

@jpkrohling
Copy link
Member

Sounds reasonable to me.

@jcchavezs
Copy link
Contributor

@jpkrohling @bogdandrutu maybe I am missing something but the first step here is to receive the headers from the receiver and then pass to the processors, how is HTTPClientSettings involved?

@jpkrohling
Copy link
Member

I believe he meant the server settings, used by most receivers

@disq
Copy link
Contributor

disq commented Jan 3, 2022

Updated the PR now cloning headers optionally.

@yashvardhan-zs
Copy link

@jpkrohling @bogdandrutu maybe I am missing something but the first step here is to receive the headers from the receiver and then pass to the processors, how is HTTPClientSettings involved?

how can we receive the headers and send same to exporter for multi tennat?

@jpkrohling
Copy link
Member

Take a look at the header setter extension for one of the possible solutions: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/headerssetter

@yashvardhan-zs
Copy link

yashvardhan-zs commented Sep 1, 2022

Take a look at the header setter extension for one of the possible solutions: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/headerssetter

I tried to use but then collector stops. also please let me know what from_context means

@jpkrohling
Copy link
Member

It's a new extension, I believe it might be available starting from 0.59.0.

@yashvardhan-zs
Copy link

It's a new extension, I believe it might be available starting from 0.59.0.

is there any available method to access headers from receiver and use it in exporter as tennat?

@jpkrohling
Copy link
Member

What's your concrete use case? Without knowing that, I can only provide general information.

HTTP headers are part of the client metadata. You can access them via the attributes or resources processor, adding that information to the data points. Exporters would then be responsible for accessing this data. The header setter helps in this last part by allowing the headers to be used as values for headers on outgoing HTTP calls.

@yashvardhan-zs
Copy link

yashvardhan-zs commented Sep 1, 2022

What's your concrete use case? Without knowing that, I can only provide general information.

HTTP headers are part of the client metadata. You can access them via the attributes or resources processor, adding that information to the data points. Exporters would then be responsible for accessing this data. The header setter helps in this last part by allowing the headers to be used as values for headers on outgoing HTTP calls.

i am using zipkin as receiver and taking headers from application, also metatdata: true and now i want to use this headers in exporter to diffrentiate tennat on the basis of headers.

receivers:
  otlp:
    protocols:
      grpc:
  zipkin:
exporters:
  otlp:
    endpoint: tempo:4317
    tls:
      insecure: true
    headers:
      X-Scope-OrgID: 1234   //////////////////////////// I want to take this either from receiver or dynamically
service:
  pipelines:
    traces:
      receivers: [otlp, zipkin]
      exporters: [otlp]

@jpkrohling
Copy link
Member

Your best bet is indeed the header setter. That's exactly the use-case it's solving.

@yash1539
Copy link

yash1539 commented Sep 1, 2022

What is from context here in headerssetter

Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this issue Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment