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

ctx propagation in otlp/http vs otlp/grpc #10093

Open
grandwizard28 opened this issue May 6, 2024 · 10 comments
Open

ctx propagation in otlp/http vs otlp/grpc #10093

grandwizard28 opened this issue May 6, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@grandwizard28
Copy link

grandwizard28 commented May 6, 2024

Context
I have written a custom auth extension which derives a key called consumer. I want to batch requests based on consumer by using the batch processor and use the same in a custom exporter.
Issue 1: Batch processor uses the incoming metadata keys to batch. I can set the client.Metadata in my custom auth extension but the otlp/grpc receiver overrides client.Metadata.
Issue 2: For the exporter, I tried setting client.Auth as well in my custom auth extension but the batch processor does not respect it. It creates a new context with only metadata, discarding the Auth (if set) by any authenticators.

Describe the bug
I have created a custom server authenticator extension. At the end I am calling the following:

return client.NewContext(ctx, client.Info{
		Metadata: client.NewMetadata(map[string][]string{
		      MetadataKeyId:      {key},
	        }),
	}), nil

Basically, I am trying to set some information in the metadata.

Now in one of my subsequent processors, I am trying to access this key. It works fine for http requests but the key is not present for grpc requests.

Steps to reproduce

What did you expect to see?
Context should be preserved for both otlp/http and otlp/grpc

What did you see instead?

What version did you use?
v0.99.0

What config did you use?

extensions:
  customauthextension:
receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317
        include_metadata: true
        max_recv_msg_size_mib: 16
        auth:
          authenticator: customauthextension
      http:
        cors:
          allowed_origins:
            - '*'
        endpoint: 0.0.0.0:4318
        include_metadata: true
        auth:
          authenticator: customauthextension
exporters:
  logging:
processors:
  customprocessor:
service:
  extensions:
    - customauthextension
  pipelines:
    logs:
      receivers: [otlp]
      processors: [customprocessor, batch]
      exporters: [logging]

*** Additional Context ***
I have been trying for a way to propagate context data from my custom authenticator to all processors and exporters for some time now. Here is a related PR regarding the same: #10002

@grandwizard28 grandwizard28 added the bug Something isn't working label May 6, 2024
@grandwizard28
Copy link
Author

Update:
If I set a custom context key in my authenticator like :

return context.WithValue(ctx, mycustomKey, key), nil

and then access it with

ctx.Value(mycustomKey)

in my processor, it worksss!!

@grandwizard28
Copy link
Author

I guess the root cause is that in grpc an interceptor to enrich the incoming context is called after the extension's auth method which is overwriting the Metadata set by the extension.

I need the batch processor to run after my customprocessor which requires a set of metadata keys to be set. However I am setting the metadata key in the auth extension which is being overidden in grpc requests.

@TylerHelmuth
Copy link
Member

@grandwizard28 do you have link to where the override is happening?

@grandwizard28
Copy link
Author

grandwizard28 commented May 7, 2024

@TylerHelmuth
Amend the following line located at https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configgrpc/configgrpc.go#L413:

- uInterceptors = append(uInterceptors, enhanceWithClientInformation(gss.IncludeMetadata))
+ uInterceptors = append(enhanceWithClientInformation(gss.IncludeMetadata), uInterceptors...)

This gives us control over playing with the Metadata in our interceptors.

@grandwizard28
Copy link
Author

@TylerHelmuth
Any thoughts?

@jpkrohling
Copy link
Member

@grandwizard28 , are you able to create a new test case here, demonstrating the problem? By looking at the code, I'd say that there's no override of these values, but perhaps I'm missing some information.

https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configgrpc/configgrpc_test.go

@grandwizard28
Copy link
Author

grandwizard28 commented May 15, 2024

Sample test here #10162

@jpkrohling
Copy link
Member

I left a comment on that PR (and marked it as draft).

@grandwizard28
Copy link
Author

Commented on the PR itself!

@grandwizard28
Copy link
Author

@jpkrohling have commented the PR, can you please check?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants