Skip to content
This repository has been archived by the owner on Nov 7, 2023. It is now read-only.

Support decoding optional headers in the OTLP-Arrow receiver #34

Merged
merged 9 commits into from
Jan 19, 2023

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Jan 12, 2023

Description:

This sets the incoming gRPC headers from the new optional bytes field added here: f5/otel-arrow-adapter#85. The behavior of this change matches #35 in the exporter.

The gRPC stream's incoming context is merged with the per-request metadata calculated for the individual data item. This addresses a problem described in greater detail in open-telemetry/opentelemetry-collector#6965. There is a suggestion that OTC would be better off with a new extension type for non-auth-related metadata, but for now the Arrow receiver expects the exporter to run the Auth extension on the request context and serialize the result; the best the receiver can do is merge the two metadatas.

Part of #33

@jmacd jmacd requested review from lquerel and jaronoff97 January 12, 2023 23:44
@jmacd jmacd changed the title Support decoding optional headers in the OTLP receiver Support decoding optional headers in the OTLP-Arrow receiver Jan 12, 2023
Copy link

@lquerel lquerel left a comment

Choose a reason for hiding this comment

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

LGTM

@jmacd jmacd requested a review from paivagustavo January 13, 2023 21:25
@jmacd
Copy link
Contributor Author

jmacd commented Jan 13, 2023

Updated the receiver to use client.Info.Metadata which is the Collector's standard mechanism for receivers to convey headers. PTAL especially @paivagustavo, thanks.

@jmacd jmacd requested a review from kristinapathak January 13, 2023 21:34
receiver/otlpreceiver/config.go Outdated Show resolved Hide resolved
@jmacd
Copy link
Contributor Author

jmacd commented Jan 18, 2023

Note: I've updated this PR with a slight fix. This code now respects the GRPCServerSettings.IncludeMetadata flag. This must be set for headers to be set into the client context.

@jmacd
Copy link
Contributor Author

jmacd commented Jan 19, 2023

Due to the changes in #35 I will now update this PR to merge the incoming context (from the stream, including the static metadata and any the Auth plugin returns for a background context) with the per-request metadata that is transmitted per batch by the exporter.

@jmacd
Copy link
Contributor Author

jmacd commented Jan 19, 2023

This PR now matches #35: when IncludeMetadata is set the stream context's gRPC incoming metadata is merged with the per-request context. this means the context constructed by the OTLP-Arrow receiver contains the results of evaluating the Auth extension twice, once on the background context and once on the per-request context. PTAL.

@jmacd
Copy link
Contributor Author

jmacd commented Jan 19, 2023

Did some refactoring of the new header-decoding logic to make it more readable, added a few tests and corrected lint errors. 88f4210

@jmacd jmacd merged commit d24bb53 into open-telemetry:main Jan 19, 2023
@jmacd jmacd deleted the jmacd/hpack branch January 19, 2023 18:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants