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

Add support for :authority pseudo-header for grpc client #8228

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

enuret
Copy link
Contributor

@enuret enuret commented Aug 14, 2023

This adds support for configuring authority dial option for grpc exporter

@enuret enuret requested review from a team and jpkrohling August 14, 2023 14:39
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 14, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@enuret enuret force-pushed the otlp_with_authority branch from 4355c53 to 77e8f9c Compare August 14, 2023 14:43
@enuret enuret changed the title add support for :authority pseudo-header for grpc client Add support for :authority pseudo-header for grpc client Aug 14, 2023
@enuret enuret force-pushed the otlp_with_authority branch from 77e8f9c to fc879dc Compare August 14, 2023 14:56
This adds support for configuring authority dial option in grpc client
@enuret enuret force-pushed the otlp_with_authority branch from fc879dc to df80854 Compare August 15, 2023 11:25
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @enuret. The change looks ok, i'm not familiar w/ the :authority header can you tell me a bit more about the use-case for setting it?

@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.04% ⚠️

Comparison is base (81242fa) 90.30% compared to head (df80854) 90.27%.
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8228      +/-   ##
==========================================
- Coverage   90.30%   90.27%   -0.04%     
==========================================
  Files         301      301              
  Lines       15581    15584       +3     
==========================================
- Hits        14071    14068       -3     
- Misses       1222     1227       +5     
- Partials      288      289       +1     
Files Changed Coverage Δ
config/configgrpc/configgrpc.go 91.59% <100.00%> (+0.11%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@enuret
Copy link
Contributor Author

enuret commented Aug 16, 2023

Thanks for the contribution @enuret. The change looks ok, i'm not familiar w/ the :authority header can you tell me a bit more about the use-case for setting it?

Hey @codeboten! Thanks for doing the review! This header is a destination host-name by default.

My particular case is configuring envoy to balance traffic between two opentelemetry collectors or between opentelemetry collector and data backend and using envoy virtual hosts to match backends.

So, envoy uses ":authority" header for http/2 requests to match them to virtual hosts: https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/route_matching

In my case the example call will look like:
collector (otlpexporter endpoint = envoyhost) -> envoy -> backend

By default, :authority header will be equal "envoyhost" because it is the endpoint address but if "backend" will be specified in :authority header it will be possible to match it based on Virtual host domain name in envoy.

For example, it will let routing based on domain name with configuration like it :

route_config:
  name: local_route
  virtual_hosts:
  - name: backend1
     domains: ["backend1"]
     routes:
     - match:
          prefix: /
        route:
          cluster: backend1

  - name: backend2
     domains: ["backend2"]
     routes:
     - match:
          prefix: /
        route:
          cluster: backend2

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks for the info 👍🏻

@codeboten codeboten merged commit 2e4f1ef into open-telemetry:main Aug 17, 2023
@codeboten codeboten added this to the next release milestone Aug 24, 2023
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.

2 participants