[extension/jaegerremotesampling] gRPC remote mode propagates configured HTTP headers #24414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zcross
changed the title
zcross
commented
Jul 20, 2023
zcross
commented
Jul 20, 2023
zcross
commented
Jul 20, 2023
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
zcross
force-pushed
the
zcross/jaegerremotesampling/support-header-enhancement-outbound-grpc
branch
from
4d4f0b0 to
ec96378
Compare
zcross
changed the title
zcross
force-pushed
the
zcross/jaegerremotesampling/support-header-enhancement-outbound-grpc
branch
3 times, most recently
from
7f96994 to
062e2e9
Compare
zcross
commented
Jul 27, 2023
zcross
commented
Jul 27, 2023
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
zcross
commented
Jul 27, 2023
frzifus
reviewed
Aug 2, 2023
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
zcross
force-pushed
the
zcross/jaegerremotesampling/support-header-enhancement-outbound-grpc
branch
from
f50107c to
7ed58ea
Compare
frzifus
reviewed
Aug 2, 2023
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
zcross
force-pushed
the
zcross/jaegerremotesampling/support-header-enhancement-outbound-grpc
branch
from
0dc14af to
8e5ba20
Compare
atoulme
reviewed
Aug 2, 2023
.chloggen/zcross_jaegerremotesampling_support-header-enhancement-outbound-grpc.yaml
Outdated
Show resolved
Hide resolved
atoulme
approved these changes
Aug 2, 2023
frzifus
reviewed
Aug 2, 2023
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
extension/jaegerremotesampling/internal/remote_strategy_store.go
Outdated
Show resolved
Hide resolved
frzifus
approved these changes
Aug 2, 2023
…TTP headers in GRPCClientSettings
…nt-outbound-grpc.yaml Co-authored-by: Antoine Toulme <[email protected]>
zcross
force-pushed
the
zcross/jaegerremotesampling/support-header-enhancement-outbound-grpc
branch
from
3820148 to
60fd963
Compare
atoulme
added
the
ready to merge
|
I have validated this in a real environment in recent days, and it seems to be working properly (as the integration tests suggested, obviously). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: This could be described as a fix or an improvement to the jaegerremotesampling extension.
Status quo: For usages in which a
remotesource is specified using collector-wide standard configgrpc.GRPCClientSettings, the given HTTP headers are not actually set on outbound calls to the destinationgrpcstore.SamplingManagerendpoint.After this PR: Outbound calls will add any HTTP headers specified in the gRPC client settings for the remote source. This will mean that drop-in extension usage will support use cases in which header additions are necessary for remote interactions. I took an approach that I observed in several other exporters/extensions: "enhancing" the gRPC context.
Link to tracking Issue: N/A
Testing: Existing extension integration tests have been updated to perform a (previously not performed) client-like HTTP call to the extension's running gRPC server, and then to verify that an observed call to a gRPC remote includes configured HTTP header additions (as gRPC metadata).
Documentation: N/A: I assumed the behavior that this PR now implements, because the gRPC client settings config is so "standard" throughout the opentelemetry-collector repo (and other extensions in this contrib repo).