Skip to content

Commit

Permalink
Use OTEL component host instead of no op host for prod code (#6085)
Browse files Browse the repository at this point in the history
  • Loading branch information
chahatsagarmain authored Oct 16, 2024
1 parent b26e414 commit bec885e
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
4 changes: 2 additions & 2 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func newStorageExt(config *Config, telset component.TelemetrySettings) *storageE
}
}

func (s *storageExt) Start(_ context.Context, _ component.Host) error {
func (s *storageExt) Start(_ context.Context, host component.Host) error {
baseFactory := otelmetrics.NewFactory(s.telset.MeterProvider)
mf := baseFactory.Namespace(metrics.NSOptions{Name: "jaeger"})
for storageName, cfg := range s.config.Backends {
Expand All @@ -126,7 +126,7 @@ func (s *storageExt) Start(_ context.Context, _ component.Host) error {
factory, err = badger.NewFactoryWithConfig(*cfg.Badger, mf, s.telset.Logger)
case cfg.GRPC != nil:
//nolint: contextcheck
factory, err = grpc.NewFactoryWithConfig(*cfg.GRPC, mf, s.telset.Logger)
factory, err = grpc.NewFactoryWithConfig(*cfg.GRPC, mf, s.telset.Logger, host)
case cfg.Cassandra != nil:
factory, err = cassandra.NewFactoryWithConfig(*cfg.Cassandra, mf, s.telset.Logger)
case cfg.Elasticsearch != nil:
Expand Down
9 changes: 7 additions & 2 deletions plugin/storage/grpc/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,26 @@ type Factory struct {
config Config
services *ClientPluginServices
remoteConn *grpc.ClientConn
host component.Host
}

// NewFactory creates a new Factory.
func NewFactory() *Factory {
return &Factory{}
return &Factory{
host: componenttest.NewNopHost(),
}
}

// NewFactoryWithConfig is used from jaeger(v2).
func NewFactoryWithConfig(
cfg Config,
metricsFactory metrics.Factory,
logger *zap.Logger,
host component.Host,
) (*Factory, error) {
f := NewFactory()
f.config = cfg
f.host = host
if err := f.Initialize(metricsFactory, logger); err != nil {
return nil, err
}
Expand Down Expand Up @@ -98,7 +103,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger)
for _, opt := range opts {
clientOpts = append(clientOpts, configgrpc.WithGrpcDialOption(opt))
}
return f.config.ToClientConn(context.Background(), componenttest.NewNopHost(), telset, clientOpts...)
return f.config.ToClientConn(context.Background(), f.host, telset, clientOpts...)
}

var err error
Expand Down
7 changes: 4 additions & 3 deletions plugin/storage/grpc/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configauth"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/exporter/exporterhelper"
Expand Down Expand Up @@ -105,7 +106,7 @@ func TestNewFactoryError(t *testing.T) {
},
}
t.Run("with_config", func(t *testing.T) {
_, err := NewFactoryWithConfig(*cfg, metrics.NullFactory, zap.NewNop())
_, err := NewFactoryWithConfig(*cfg, metrics.NullFactory, zap.NewNop(), componenttest.NewNopHost())
require.Error(t, err)
assert.Contains(t, err.Error(), "authenticator")
})
Expand All @@ -121,7 +122,7 @@ func TestNewFactoryError(t *testing.T) {

t.Run("client", func(t *testing.T) {
// this is a silly test to verify handling of error from grpc.NewClient, which cannot be induced via params.
f, err := NewFactoryWithConfig(Config{}, metrics.NullFactory, zap.NewNop())
f, err := NewFactoryWithConfig(Config{}, metrics.NullFactory, zap.NewNop(), componenttest.NewNopHost())
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, f.Close()) })
newClientFn := func(_ ...grpc.DialOption) (conn *grpc.ClientConn, err error) {
Expand Down Expand Up @@ -173,7 +174,7 @@ func TestGRPCStorageFactoryWithConfig(t *testing.T) {
Enabled: true,
},
}
f, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop())
f, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop(), componenttest.NewNopHost())
require.NoError(t, err)
require.NoError(t, f.Close())
}
Expand Down

0 comments on commit bec885e

Please sign in to comment.