diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config.go b/cmd/jaeger/internal/extension/jaegerstorage/config.go index d297bea5b3d..0ae2a59895b 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/config.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/config.go @@ -41,7 +41,7 @@ type Config struct { type Backend struct { Memory *memory.Configuration `mapstructure:"memory"` Badger *badger.Config `mapstructure:"badger"` - GRPC *grpc.ConfigV2 `mapstructure:"grpc"` + GRPC *grpc.Config `mapstructure:"grpc"` Cassandra *cassandra.Options `mapstructure:"cassandra"` Elasticsearch *esCfg.Configuration `mapstructure:"elasticsearch"` Opensearch *esCfg.Configuration `mapstructure:"opensearch"` @@ -66,7 +66,7 @@ func (cfg *Backend) Unmarshal(conf *confmap.Conf) error { cfg.Badger = v } if conf.IsSet("grpc") { - v := grpc.DefaultConfigV2() + v := grpc.DefaultConfig() cfg.GRPC = &v } if conf.IsSet("cassandra") { diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go index 5d8ae6dc71c..765781c1680 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go @@ -143,7 +143,7 @@ func TestGRPC(t *testing.T) { ext := makeStorageExtenion(t, &Config{ Backends: map[string]Backend{ "foo": { - GRPC: &grpc.ConfigV2{ + GRPC: &grpc.Config{ ClientConfig: configgrpc.ClientConfig{ Endpoint: "localhost:12345", }, diff --git a/plugin/storage/grpc/config.go b/plugin/storage/grpc/config.go index dd095d7e57a..3f5834aba0b 100644 --- a/plugin/storage/grpc/config.go +++ b/plugin/storage/grpc/config.go @@ -4,51 +4,28 @@ package grpc import ( - "time" - "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/exporter/exporterhelper" - "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/storage/grpc/shared" ) -// Configuration describes the options to customize the storage behavior. -type Configuration struct { - RemoteServerAddr string `yaml:"server" mapstructure:"server"` - RemoteTLS tlscfg.Options - RemoteConnectTimeout time.Duration `yaml:"connection-timeout" mapstructure:"connection-timeout"` - TenancyOpts tenancy.Options -} - -type ConfigV2 struct { +// Config describes the options to customize the storage behavior +type Config struct { Tenancy tenancy.Options `mapstructure:"multi_tenancy"` configgrpc.ClientConfig `mapstructure:",squash"` exporterhelper.TimeoutSettings `mapstructure:",squash"` } -func DefaultConfigV2() ConfigV2 { - return ConfigV2{ +func DefaultConfig() Config { + return Config{ TimeoutSettings: exporterhelper.TimeoutConfig{ Timeout: defaultConnectionTimeout, }, } } -func (c *Configuration) TranslateToConfigV2() *ConfigV2 { - return &ConfigV2{ - Tenancy: c.TenancyOpts, - ClientConfig: configgrpc.ClientConfig{ - Endpoint: c.RemoteServerAddr, - TLSSetting: c.RemoteTLS.ToOtelClientConfig(), - }, - TimeoutSettings: exporterhelper.TimeoutConfig{ - Timeout: c.RemoteConnectTimeout, - }, - } -} - // ClientPluginServices defines services plugin can expose and its capabilities type ClientPluginServices struct { shared.PluginServices diff --git a/plugin/storage/grpc/config_test.go b/plugin/storage/grpc/config_test.go index 44fcbd7d331..e8279ef8342 100644 --- a/plugin/storage/grpc/config_test.go +++ b/plugin/storage/grpc/config_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestDefaultConfigV2(t *testing.T) { - cfg := DefaultConfigV2() +func TestDefaultConfig(t *testing.T) { + cfg := DefaultConfig() assert.NotEmpty(t, cfg.Timeout) } diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go index 95c51f4490f..49f0f5a45e3 100644 --- a/plugin/storage/grpc/factory.go +++ b/plugin/storage/grpc/factory.go @@ -44,14 +44,9 @@ type Factory struct { metricsFactory metrics.Factory logger *zap.Logger tracerProvider trace.TracerProvider - - // configV1 is used for backward compatibility. it will be removed in v2. - // In the main initialization logic, only configV2 is used. - configV1 Configuration - configV2 *ConfigV2 - - services *ClientPluginServices - remoteConn *grpc.ClientConn + config Config + services *ClientPluginServices + remoteConn *grpc.ClientConn } // NewFactory creates a new Factory. @@ -61,12 +56,12 @@ func NewFactory() *Factory { // NewFactoryWithConfig is used from jaeger(v2). func NewFactoryWithConfig( - cfg ConfigV2, + cfg Config, metricsFactory metrics.Factory, logger *zap.Logger, ) (*Factory, error) { f := NewFactory() - f.configV2 = &cfg + f.config = cfg if err := f.Initialize(metricsFactory, logger); err != nil { return nil, err } @@ -75,12 +70,12 @@ func NewFactoryWithConfig( // AddFlags implements plugin.Configurable func (*Factory) AddFlags(flagSet *flag.FlagSet) { - v1AddFlags(flagSet) + addFlags(flagSet) } // InitFromViper implements plugin.Configurable func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { - if err := v1InitFromViper(&f.configV1, v); err != nil { + if err := initFromViper(&f.config, v); err != nil { logger.Fatal("unable to initialize gRPC storage factory", zap.Error(err)) } } @@ -90,10 +85,6 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) f.metricsFactory, f.logger = metricsFactory, logger f.tracerProvider = otel.GetTracerProvider() - if f.configV2 == nil { - f.configV2 = f.configV1.TranslateToConfigV2() - } - telset := component.TelemetrySettings{ Logger: logger, TracerProvider: f.tracerProvider, @@ -107,7 +98,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) for _, opt := range opts { clientOpts = append(clientOpts, configgrpc.WithGrpcDialOption(opt)) } - return f.configV2.ToClientConnWithOptions(context.Background(), componenttest.NewNopHost(), telset, clientOpts...) + return f.config.ToClientConnWithOptions(context.Background(), componenttest.NewNopHost(), telset, clientOpts...) } var err error @@ -115,14 +106,14 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) if err != nil { return fmt.Errorf("grpc storage builder failed to create a store: %w", err) } - logger.Info("Remote storage configuration", zap.Any("configuration", f.configV2)) + logger.Info("Remote storage configuration", zap.Any("configuration", f.config)) return nil } type newClientFn func(opts ...grpc.DialOption) (*grpc.ClientConn, error) func (f *Factory) newRemoteStorage(telset component.TelemetrySettings, newClient newClientFn) (*ClientPluginServices, error) { - c := f.configV2 + c := f.config opts := []grpc.DialOption{ grpc.WithStatsHandler(otelgrpc.NewClientHandler(otelgrpc.WithTracerProvider(telset.TracerProvider))), } @@ -208,6 +199,5 @@ func (f *Factory) Close() error { if f.remoteConn != nil { errs = append(errs, f.remoteConn.Close()) } - errs = append(errs, f.configV1.RemoteTLS.Close()) return errors.Join(errs...) } diff --git a/plugin/storage/grpc/factory_test.go b/plugin/storage/grpc/factory_test.go index 0c03f6ab0bb..8030ba23386 100644 --- a/plugin/storage/grpc/factory_test.go +++ b/plugin/storage/grpc/factory_test.go @@ -98,7 +98,7 @@ func makeFactory(t *testing.T) *Factory { } func TestNewFactoryError(t *testing.T) { - cfg := &ConfigV2{ + cfg := &Config{ ClientConfig: configgrpc.ClientConfig{ // non-empty Auth is currently not supported Auth: &configauth.Authentication{}, @@ -113,7 +113,7 @@ func TestNewFactoryError(t *testing.T) { t.Run("viper", func(t *testing.T) { f := NewFactory() f.InitFromViper(viper.New(), zap.NewNop()) - f.configV2 = cfg + f.config = *cfg err := f.Initialize(metrics.NullFactory, zap.NewNop()) require.Error(t, err) assert.Contains(t, err.Error(), "authenticator") @@ -121,7 +121,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(ConfigV2{}, metrics.NullFactory, zap.NewNop()) + f, err := NewFactoryWithConfig(Config{}, metrics.NullFactory, zap.NewNop()) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, f.Close()) }) newClientFn := func(_ ...grpc.DialOption) (conn *grpc.ClientConn, err error) { @@ -162,7 +162,7 @@ func TestGRPCStorageFactoryWithConfig(t *testing.T) { }() defer s.Stop() - cfg := ConfigV2{ + cfg := Config{ ClientConfig: configgrpc.ClientConfig{ Endpoint: lis.Addr().String(), }, @@ -265,7 +265,7 @@ func TestWithCLIFlags(t *testing.T) { }) require.NoError(t, err) f.InitFromViper(v, zap.NewNop()) - assert.Equal(t, "foo:1234", f.configV1.RemoteServerAddr) + assert.Equal(t, "foo:1234", f.config.ClientConfig.Endpoint) require.NoError(t, f.Close()) } diff --git a/plugin/storage/grpc/options.go b/plugin/storage/grpc/options.go index 06a05dc7b36..f5a08f4e1ce 100644 --- a/plugin/storage/grpc/options.go +++ b/plugin/storage/grpc/options.go @@ -27,22 +27,22 @@ func tlsFlagsConfig() tlscfg.ClientFlagsConfig { } } -// AddFlags adds flags for Options -func v1AddFlags(flagSet *flag.FlagSet) { +// addFlags adds flags for Options +func addFlags(flagSet *flag.FlagSet) { tlsFlagsConfig().AddFlags(flagSet) flagSet.String(remoteServer, "", "The remote storage gRPC server address as host:port") flagSet.Duration(remoteConnectionTimeout, defaultConnectionTimeout, "The remote storage gRPC server connection timeout") } -func v1InitFromViper(cfg *Configuration, v *viper.Viper) error { - cfg.RemoteServerAddr = v.GetString(remoteServer) - var err error - cfg.RemoteTLS, err = tlsFlagsConfig().InitFromViper(v) +func initFromViper(cfg *Config, v *viper.Viper) error { + cfg.ClientConfig.Endpoint = v.GetString(remoteServer) + remoteTLS, err := tlsFlagsConfig().InitFromViper(v) if err != nil { return fmt.Errorf("failed to parse gRPC storage TLS options: %w", err) } - cfg.RemoteConnectTimeout = v.GetDuration(remoteConnectionTimeout) - cfg.TenancyOpts = tenancy.InitFromViper(v) + cfg.ClientConfig.TLSSetting = remoteTLS.ToOtelClientConfig() + cfg.TimeoutSettings.Timeout = v.GetDuration(remoteConnectionTimeout) + cfg.Tenancy = tenancy.InitFromViper(v) return nil } diff --git a/plugin/storage/grpc/options_test.go b/plugin/storage/grpc/options_test.go index 45665e3b06d..83e52364d16 100644 --- a/plugin/storage/grpc/options_test.go +++ b/plugin/storage/grpc/options_test.go @@ -18,61 +18,60 @@ import ( ) func TestOptionsWithFlags(t *testing.T) { - v, command := config.Viperize(v1AddFlags, tenancy.AddFlags) + v, command := config.Viperize(addFlags, tenancy.AddFlags) err := command.ParseFlags([]string{ "--grpc-storage.server=foo:12345", "--multi-tenancy.header=x-scope-orgid", }) require.NoError(t, err) - var cfg Configuration - require.NoError(t, v1InitFromViper(&cfg, v)) + var cfg Config + require.NoError(t, initFromViper(&cfg, v)) - assert.Equal(t, "foo:12345", cfg.RemoteServerAddr) - assert.False(t, cfg.TenancyOpts.Enabled) - assert.Equal(t, "x-scope-orgid", cfg.TenancyOpts.Header) + assert.Equal(t, "foo:12345", cfg.ClientConfig.Endpoint) + assert.False(t, cfg.Tenancy.Enabled) + assert.Equal(t, "x-scope-orgid", cfg.Tenancy.Header) } func TestRemoteOptionsWithFlags(t *testing.T) { - v, command := config.Viperize(v1AddFlags) + v, command := config.Viperize(addFlags) err := command.ParseFlags([]string{ "--grpc-storage.server=localhost:2001", "--grpc-storage.tls.enabled=true", "--grpc-storage.connection-timeout=60s", }) require.NoError(t, err) - var cfg Configuration - require.NoError(t, v1InitFromViper(&cfg, v)) + var cfg Config + require.NoError(t, initFromViper(&cfg, v)) - assert.Equal(t, "localhost:2001", cfg.RemoteServerAddr) - assert.True(t, cfg.RemoteTLS.Enabled) - assert.Equal(t, 60*time.Second, cfg.RemoteConnectTimeout) + assert.Equal(t, "localhost:2001", cfg.ClientConfig.Endpoint) + assert.False(t, cfg.ClientConfig.TLSSetting.Insecure) + assert.Equal(t, 60*time.Second, cfg.TimeoutSettings.Timeout) } func TestRemoteOptionsNoTLSWithFlags(t *testing.T) { - v, command := config.Viperize(v1AddFlags) + v, command := config.Viperize(addFlags) err := command.ParseFlags([]string{ "--grpc-storage.server=localhost:2001", "--grpc-storage.tls.enabled=false", "--grpc-storage.connection-timeout=60s", }) require.NoError(t, err) - var cfg Configuration - require.NoError(t, v1InitFromViper(&cfg, v)) + var cfg Config + require.NoError(t, initFromViper(&cfg, v)) - assert.Equal(t, "localhost:2001", cfg.RemoteServerAddr) - assert.False(t, cfg.RemoteTLS.Enabled) - assert.Equal(t, 60*time.Second, cfg.RemoteConnectTimeout) + assert.Equal(t, "localhost:2001", cfg.ClientConfig.Endpoint) + assert.True(t, cfg.ClientConfig.TLSSetting.Insecure) + assert.Equal(t, 60*time.Second, cfg.TimeoutSettings.Timeout) } func TestFailedTLSFlags(t *testing.T) { - v, command := config.Viperize(v1AddFlags) + v, command := config.Viperize(addFlags) err := command.ParseFlags([]string{ "--grpc-storage.tls.enabled=false", "--grpc-storage.tls.cert=blah", // invalid unless tls.enabled=true }) require.NoError(t, err) f := NewFactory() - f.configV2 = nil core, logs := observer.New(zap.NewAtomicLevelAt(zapcore.ErrorLevel)) logger := zap.New(core, zap.WithFatalHook(zapcore.WriteThenPanic)) require.Panics(t, func() { f.InitFromViper(v, logger) })