From af2c2ef40ee290a8b14409ec190a92c8d003f468 Mon Sep 17 00:00:00 2001 From: Alex Kats <56042997+akats7@users.noreply.github.com> Date: Mon, 4 Nov 2024 13:25:58 -0500 Subject: [PATCH] updated grpc config to use DialContext (#11575) #### Description This is related #11537 and [this grpc issue](https://github.com/grpc/grpc-go/issues/7779), to reiterate there is a bug in the grpc-go NewClient that makes the way the hostname is resolved incompatible with the way proxy setting are applied. Due to this domain based NO_PROXY setting do not work for grpc connections. They are working on a fix in the grpc library but until then it seems like it might be a good idea to revert to using DialContext instead of NewClient. If there's a workaround for this that anyone is aware of that could be suitable as well, the most clear one of using passthrough doesn't work since we sanitize the endpoint. #### Link to tracking issue Fixes #11537 #### Testing I added some logging where the proxy setting are evaluated to show the behavior. **DialContext** ______ InMatch host : otel******* m.host : hasSuffix : true m.matchHost : true ______ **NewClient** ______ InMatch host : 10.*.*.*.* m.host : hasSuffix : false m.matchHost : false ______ --------- Co-authored-by: Yang Song --- .chloggen/configgrpc-client-update.yaml | 25 +++++++++++++++++++++++++ config/configgrpc/configgrpc.go | 3 ++- 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 .chloggen/configgrpc-client-update.yaml diff --git a/.chloggen/configgrpc-client-update.yaml b/.chloggen/configgrpc-client-update.yaml new file mode 100644 index 00000000000..d85b4a28ffb --- /dev/null +++ b/.chloggen/configgrpc-client-update.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: config/configgrpc + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Patch for bug in the grpc-go NewClient that makes the way the hostname is resolved incompatible with the way proxy setting are applied. + +# One or more tracking issues or pull requests related to the change +issues: [11537] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 07d6c8c7675..1b1b15963ca 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -261,7 +261,8 @@ func (gcs *ClientConfig) ToClientConn( if err != nil { return nil, err } - return grpc.NewClient(gcs.sanitizedEndpoint(), grpcOpts...) + //nolint:staticcheck //SA1019 see https://github.com/open-telemetry/opentelemetry-collector/pull/11575 + return grpc.DialContext(ctx, gcs.sanitizedEndpoint(), grpcOpts...) } func (gcs *ClientConfig) getGrpcDialOptions(