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

[extension/headers_setter] Add headers_setter extension. #12892

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

kovrus
Copy link
Member

@kovrus kovrus commented Aug 2, 2022

Description:

Resolves #12922

The headers_setter extension implements ClientAuthenticator
and is used to set request headers in gRPC / HTTP exporters with values provided
via the extension configuration or request metadata (context).

Use cases include but are not limited to enabling multi-tenancy for observability
backends such as Tempo, Mimir, Loki and others by setting the X-Scope-OrgID
header to the value extracted from the context.

Link to tracking Issue:
This change potentially can address the following issues related to multi-tenancy use cases:

Open discussion:

...
exporters:
...
  loki:
    ...
    auth:
      authenticator: headers_setter  
...

Alternatives:
It is also possible to implement the same functionality (setting header values from context) in opentelemetry-collector by implementing a round tripper and interceptor that will set in confighttp.go and configgrpc.go and changing the headers configuration of HTTP / gRPC client settings. This approach seems to be more complex and might impact some exporters in pentelemetry-collector-contrib

Testing:

  • Unit tests
  • Locally with the following Otel collector config:
extensions:
  headers_setter:
    headers:
      key: X-Scope-OrgID
      from_context: "tenant"

receivers:
  otlp/1:
    protocols:
      grpc:
        include_metadata: true
        endpoint: localhost:4000
  otlp/2:
    protocols:
      grpc:
        endpoint: localhost:4318
        include_metadata: true

exporters:
  otlp:
    endpoint: localhost:4318
    tls:
      insecure: true
      insecure_skip_verify: true
    headers:
      tenant: "value1"
  logging:

  loki:
    labels:
      resource:
        container_name: ""
        container_id: ""
    endpoint: https://localhost:3100/loki/api/v1/push
    auth:
      authenticator: headers_setter

service:
  extensions: [headers_setter]
  pipelines:
    logs/1:
      receivers: [otlp/1]
      exporters: [otlp]
    logs/2:
      receivers: [otlp/2]
      exporters: [loki]

Documentation:
See README.md in the new headers_setter extension.

@kovrus kovrus requested review from a team and djaglowski August 2, 2022 15:12
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 2, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: kovrus / name: Ruslan Kovalov (e01947098cbb0140bcfd437e2a1a71f3ef7f184c)

@jpkrohling
Copy link
Member

@kovrus, please open an issue proposing this new component (see the contributing guidelines). I'm willing to sponsor this one.

@kovrus kovrus changed the title [extensions] Add headers_setter extension. [extension/headers_setter] Add headers_setter extension. Aug 3, 2022
@kovrus kovrus force-pushed the header_setter_extension branch 5 times, most recently from e5bd4cd to 048306d Compare August 8, 2022 11:24
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks almost perfect, thanks!

extension/headerssetter/README.md Outdated Show resolved Hide resolved
if headerCfg.Key == nil || *headerCfg.Key == "" {
return errMissingHeader
}
if headerCfg.FromContext == nil && headerCfg.Value == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If FromContext is empty, what would happen?

extension/headerssetter/extension.go Outdated Show resolved Hide resolved
},
},
expectedHeaders: map[string]string{
"header_name": "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this answers the question I had earlier about empty FromContext values.

Copy link
Member Author

@kovrus kovrus Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantic is following:

from_context: "key" -> if key is missing from context then header value set to ""
from_context: "key" -> if key from context is "" then header value set to ""
value: "" -> header value set to ""

I was thinking of not setting headers at all if their value is "" but then decided to make it arguably more explicit. Also, "" is a valid header value.

extension/headerssetter/source/context.go Outdated Show resolved Hide resolved
internal/components/extensions_test.go Outdated Show resolved Hide resolved
@kovrus kovrus force-pushed the header_setter_extension branch 2 times, most recently from 2f4e8b8 to a5ade8e Compare August 9, 2022 10:18
@kovrus kovrus requested a review from jpkrohling August 9, 2022 11:07
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just double-check the go version we are using in other modules, which might have been changed since this PR was opened.

extension/headerssetter/go.mod Outdated Show resolved Hide resolved
The `headers_setter` extension implements `ClientAuthenticator`
and is used to set request headers in gRPC / HTTP exporters with values provided
via the extension configuration or request metatdata (context).
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.

New component: Headers Setter extension for exporters
2 participants