From ecd87a593bfb9d2f1165d001f975c669bff0c6c5 Mon Sep 17 00:00:00 2001 From: Tim Mwangi Date: Fri, 20 Oct 2023 09:11:37 -0700 Subject: [PATCH] feat: custom changes in configgrpc, confighttp and service --- FORK.md | 24 +++++++++ config/configgrpc/configgrpc.go | 30 +++++++++-- config/configgrpc/configgrpc_test.go | 19 +++---- .../configgrpcclientdialoptionhandler_test.go | 50 +++++++++++++++++++ config/confighttp/confighttp.go | 4 +- config/confighttp/confighttp_test.go | 13 ++--- 6 files changed, 118 insertions(+), 22 deletions(-) create mode 100644 FORK.md create mode 100644 config/configgrpc/configgrpcclientdialoptionhandler_test.go diff --git a/FORK.md b/FORK.md new file mode 100644 index 00000000000..0136ec61ac4 --- /dev/null +++ b/FORK.md @@ -0,0 +1,24 @@ +# How this fork works + +This fork was created to be able to early patch open-telemetry/opentelemetry-collector versions in cases it is hard to get the change in upstream right away. + +## How do we consume this library + +For every opentelemetry-collector release we create a new release including our own patches. For example for version v0.49.0 we in open-telemetry/opentelemetry-collector we will crease v0.49.0+patches. This make sure we stick to a version in our downstream dependencies. + +Whenever we need a new release on this repository we rebase the branch `latest+patches` version against the new release for open-telemetry/opentelemetry-collector and then get a new release. For example on version `v0.49.0` (asuming `origin` is `git@github.com:hypertrace/opentelemetry-collector.git` and `upstream` is `git@github.com:open-telemetry/opentelemetry-collector.git`): + +```bash +git fetch --all +git checkout latest+patches +git pull --rebase upstream refs/tags/v0.49.0 +# make lint test +git tag -a "v0.49.0+patches" -m "Release v0.49.0" +git push --tags +``` + +## What custom changes are in here +- In `config/configgrpc/configgrpc.go` we added the ability to add extra `ClientDialOptionHandler`. Also a unit test for this in `config/configgrpc/configgrpcclientdialoptionhandler_test.go`. Also commented out warning on servers starting `UnspecifiedHost` aka `0.0.0.0`. +- In `config/configgrpc/configgrpc_test.go` we commented a unit test checking for a warning on servers starting `UnspecifiedHost` aka `0.0.0.0`. +- In `config/confighttp/confighttp.go` we commented out warning on servers starting `UnspecifiedHost`. +- In ` config/confighttp/confighttp_test.go` we commented a unit test checking for a warning on servers starting `UnspecifiedHost`. diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 2bf15063c57..93a373b2333 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -35,12 +35,15 @@ import ( "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/config/configtls" - "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" ) var errMetadataNotFound = errors.New("no request metadata found") +type ClientDialOptionHandler func() grpc.DialOption + +var clientOptionHandlerList = make([]ClientDialOptionHandler, 0) + // KeepaliveClientConfig exposes the keepalive.ClientParameters to be used by the exporter. // Refer to the original data-structure for the meaning of each parameter: // https://godoc.org/google.golang.org/grpc/keepalive#ClientParameters @@ -105,6 +108,9 @@ type ClientConfig struct { // Auth configuration for outgoing RPCs. Auth *configauth.Authentication `mapstructure:"auth"` + + // SkipGlobalClientOption defines config if the global client interceptors need to be used + SkipGlobalClientOption bool `mapstructure:"skip_global_client_option"` } // NewDefaultClientConfig returns a new instance of ClientConfig with default values. @@ -360,6 +366,15 @@ func (gcs *ClientConfig) getGrpcDialOptions( } } + if !gcs.SkipGlobalClientOption { + for _, handler := range clientOptionHandlerList { + opt := handler() + if opt != nil { + opts = append(opts, opt) + } + } + } + return opts, nil } @@ -435,10 +450,11 @@ func (gss *ServerConfig) getGrpcServerOptions( settings component.TelemetrySettings, extraOpts []ToServerOption, ) ([]grpc.ServerOption, error) { - switch gss.NetAddr.Transport { - case confignet.TransportTypeTCP, confignet.TransportTypeTCP4, confignet.TransportTypeTCP6, confignet.TransportTypeUDP, confignet.TransportTypeUDP4, confignet.TransportTypeUDP6: - internal.WarnOnUnspecifiedHost(settings.Logger, gss.NetAddr.Endpoint) - } + // Warning commented out for now. ENG-27501 + // switch gss.NetAddr.Transport { + // case confignet.TransportTypeTCP, confignet.TransportTypeTCP4, confignet.TransportTypeTCP6, confignet.TransportTypeUDP, confignet.TransportTypeUDP4, confignet.TransportTypeUDP6: + // internal.WarnOnUnspecifiedHost(settings.Logger, gss.NetAddr.Endpoint) + // } var opts []grpc.ServerOption @@ -608,3 +624,7 @@ func authStreamServerInterceptor(srv any, stream grpc.ServerStream, _ *grpc.Stre return handler(srv, wrapServerStream(ctx, stream)) } + +func RegisterClientDialOptionHandlers(handlers ...ClientDialOptionHandler) { + clientOptionHandlerList = append(clientOptionHandlerList, handlers...) +} diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index e46c8a546d3..6222cb49283 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -536,15 +536,16 @@ func TestGRPCServerWarning(t *testing.T) { settings ServerConfig len int }{ - { - settings: ServerConfig{ - NetAddr: confignet.AddrConfig{ - Endpoint: "0.0.0.0:1234", - Transport: confignet.TransportTypeTCP, - }, - }, - len: 1, - }, + // Test commented out for now. ENG-27501 + // { + // settings: ServerConfig{ + // NetAddr: confignet.NetAddr{ + // Endpoint: "0.0.0.0:1234", + // Transport: confignet.TransportTypeTCP, + // }, + // }, + // len: 1, + // }, { settings: ServerConfig{ NetAddr: confignet.AddrConfig{ diff --git a/config/configgrpc/configgrpcclientdialoptionhandler_test.go b/config/configgrpc/configgrpcclientdialoptionhandler_test.go new file mode 100644 index 00000000000..29580fff91b --- /dev/null +++ b/config/configgrpc/configgrpcclientdialoptionhandler_test.go @@ -0,0 +1,50 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +// This is a test added by us. +package configgrpc + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/component/componenttest" + "go.opentelemetry.io/collector/extension" +) + +func TestRegisterClientDialOptionHandler(t *testing.T) { + tt, err := componenttest.SetupTelemetry(component.MustNewID("component")) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) }) + + gcs := &ClientConfig{} + opts, err := gcs.getGrpcDialOptions( + context.Background(), + &mockHost{ext: map[component.ID]extension.Extension{}}, + tt.TelemetrySettings(), + []ToClientConnOption{}, + ) + require.NoError(t, err) + + defaultOptsLen := len(opts) + + RegisterClientDialOptionHandlers(func() grpc.DialOption { + return grpc.WithUnaryInterceptor(func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + return invoker(ctx, method, req, reply, cc, opts...) + }) + }) + gcs = &ClientConfig{} + opts, err = gcs.getGrpcDialOptions( + context.Background(), + &mockHost{ext: map[component.ID]extension.Extension{}}, + tt.TelemetrySettings(), + []ToClientConnOption{}, + ) + assert.NoError(t, err) + assert.Len(t, opts, defaultOptsLen+1) +} diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 4c38d3015a7..26cf646351e 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -27,7 +27,6 @@ import ( "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/collector/config/configtls" - "go.opentelemetry.io/collector/config/internal" "go.opentelemetry.io/collector/extension/auth" ) @@ -412,7 +411,8 @@ func WithDecoder(key string, dec func(body io.ReadCloser) (io.ReadCloser, error) // ToServer creates an http.Server from settings object. func (hss *ServerConfig) ToServer(_ context.Context, host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { - internal.WarnOnUnspecifiedHost(settings.Logger, hss.Endpoint) + // Warning commented out for now. ENG-27501 + // internal.WarnOnUnspecifiedHost(settings.Logger, hss.Endpoint) serverOpts := &toServerOptions{} for _, o := range opts { diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 7d16eb12ef3..9323524d811 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -523,12 +523,13 @@ func TestHTTPServerWarning(t *testing.T) { settings ServerConfig len int }{ - { - settings: ServerConfig{ - Endpoint: "0.0.0.0:0", - }, - len: 1, - }, + // Test commented out for now. ENG-27501 + // { + // settings: ServerConfig{ + // Endpoint: "0.0.0.0:0", + // }, + // len: 1, + // }, { settings: ServerConfig{ Endpoint: "127.0.0.1:0",