Skip to content

Commit

Permalink
feat: custom changes in configgrpc, confighttp and service
Browse files Browse the repository at this point in the history
  • Loading branch information
tim-mwangi committed Jun 11, 2024
1 parent 7218b4c commit 9af37d4
Show file tree
Hide file tree
Showing 7 changed files with 116 additions and 23 deletions.
24 changes: 24 additions & 0 deletions FORK.md
Original file line number Diff line number Diff line change
@@ -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 `[email protected]:hypertrace/opentelemetry-collector.git` and `upstream` is `[email protected]: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`.
30 changes: 25 additions & 5 deletions config/configgrpc/configgrpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,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
Expand Down Expand Up @@ -97,6 +100,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.
Expand Down Expand Up @@ -306,6 +312,15 @@ func (gcs *ClientConfig) toDialOptions(host component.Host, settings component.T
// Enable OpenTelemetry observability plugin.
opts = append(opts, grpc.WithStatsHandler(otelgrpc.NewClientHandler(otelOpts...)))

if !gcs.SkipGlobalClientOption {
for _, handler := range clientOptionHandlerList {
opt := handler()
if opt != nil {
opts = append(opts, opt)
}
}
}

return opts, nil
}

Expand All @@ -324,10 +339,11 @@ func (gss *ServerConfig) ToServer(_ context.Context, host component.Host, settin
}

func (gss *ServerConfig) toServerOption(host component.Host, settings component.TelemetrySettings) ([]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

Expand Down Expand Up @@ -493,3 +509,7 @@ func authStreamServerInterceptor(srv any, stream grpc.ServerStream, _ *grpc.Stre

return handler(srv, wrapServerStream(ctx, stream))
}

func RegisterClientDialOptionHandlers(handlers ...ClientDialOptionHandler) {
clientOptionHandlerList = append(clientOptionHandlerList, handlers...)
}
19 changes: 10 additions & 9 deletions config/configgrpc/configgrpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,15 +442,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{
Expand Down
46 changes: 46 additions & 0 deletions config/configgrpc/configgrpcclientdialoptionhandler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// 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.toDialOptions(
&mockHost{ext: map[component.ID]extension.Extension{}},
tt.TelemetrySettings(),
)
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.toDialOptions(
&mockHost{ext: map[component.ID]extension.Extension{}},
tt.TelemetrySettings(),
)
assert.NoError(t, err)
assert.Len(t, opts, defaultOptsLen+1)
}
4 changes: 2 additions & 2 deletions config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,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"
)

Expand Down Expand Up @@ -334,7 +333,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 {
Expand Down
13 changes: 7 additions & 6 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,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",
Expand Down
3 changes: 2 additions & 1 deletion internal/localhostgate/featuregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ func EndpointForPort(port int) string {
// LogAboutUseLocalHostAsDefault logs about the upcoming change from 0.0.0.0 to localhost on server-like components.
func LogAboutUseLocalHostAsDefault(logger *zap.Logger) {
if !UseLocalHostAsDefaultHostfeatureGate.IsEnabled() {
logger.Warn(
// Warning changed to Debug for now. ENG-27501
logger.Debug(
"The default endpoints for all servers in components will change to use localhost instead of 0.0.0.0 in a future version. Use the feature gate to preview the new default.",
zap.String("feature gate ID", UseLocalHostAsDefaultHostID),
)
Expand Down

0 comments on commit 9af37d4

Please sign in to comment.