Skip to content

Use OTEL component host instead of no op host for prod code #6085

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.

Sign up for GitHub

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

Merged
yurishkuro merged 3 commits into from
Oct 16, 2024
Merged

Use OTEL component host instead of no op host for prod code #6085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2794600
OTEL host instead of no op host
chahatsagarmain Oct 14, 2024
93108ec
Merge branch 'main' into componenttestmain
yurishkuro Oct 14, 2024
8254b79
no op host as a default for f.host
chahatsagarmain Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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