From 4173849d98c65438418c0d8a7b470fc73cbed611 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 6 Feb 2024 12:48:05 -0500 Subject: [PATCH] [jaeger-v2] Streamline storage initialization Signed-off-by: Yuri Shkuro --- .../extension/jaegerstorage/extension.go | 71 +++++--- .../extension/jaegerstorage/extension_test.go | 163 +++++++----------- 2 files changed, 117 insertions(+), 117 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go index ae743313046..a1ac028e772 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension.go @@ -13,8 +13,10 @@ import ( "go.opentelemetry.io/collector/extension" "go.uber.org/zap" + memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin/storage/badger" + badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger" "github.com/jaegertracing/jaeger/plugin/storage/memory" "github.com/jaegertracing/jaeger/storage" ) @@ -60,35 +62,62 @@ func newStorageExt(config *Config, otel component.TelemetrySettings) *storageExt } } -func (s *storageExt) Start(ctx context.Context, host component.Host) error { - for name, mem := range s.config.Memory { - if _, ok := s.factories[name]; ok { - return fmt.Errorf("duplicate memory storage name %s", name) - } - s.factories[name] = memory.NewFactoryWithConfig( - mem, - metrics.NullFactory, - s.logger.With(zap.String("storage_name", name)), - ) - } +type starter[Config any, Factory storage.Factory] struct { + ext *storageExt + storageKind string + cfg map[string]Config + builder func(Config, metrics.Factory, *zap.Logger) (Factory, error) +} - for name, b := range s.config.Badger { - if _, ok := s.factories[name]; ok { - return fmt.Errorf("duplicate badger storage name %s", name) +func (s *starter[Config, Factory]) build(ctx context.Context, host component.Host) error { + for name, cfg := range s.cfg { + if _, ok := s.ext.factories[name]; ok { + return fmt.Errorf("duplicate %s storage name %s", s.storageKind, name) } - var err error - factory, err := badger.NewFactoryWithConfig( - b, + factory, err := s.builder( + cfg, metrics.NullFactory, - s.logger.With(zap.String("storage_name", name)), + s.ext.logger.With(zap.String("storage_name", name)), ) if err != nil { - return fmt.Errorf("failed to initialize badger storage: %w", err) + return fmt.Errorf("failed to initialize %s storage %s: %w", s.storageKind, name, err) } - s.factories[name] = factory + s.ext.factories[name] = factory } + return nil +} - // TODO add support for other backends +func (s *storageExt) Start(ctx context.Context, host component.Host) error { + memStarter := &starter[memoryCfg.Configuration, *memory.Factory]{ + ext: s, + storageKind: "memory", + cfg: s.config.Memory, + // memory factory does not return an error, so need to wrap it + builder: func( + cfg memoryCfg.Configuration, + metricsFactory metrics.Factory, + logger *zap.Logger, + ) (*memory.Factory, error) { + return memory.NewFactoryWithConfig(cfg, metricsFactory, logger), nil + }, + } + badgerStarter := &starter[badgerCfg.NamespaceConfig, *badger.Factory]{ + ext: s, + storageKind: "badger", + cfg: s.config.Badger, + builder: badger.NewFactoryWithConfig, + } + + builders := []func(ctx context.Context, host component.Host) error{ + memStarter.build, + badgerStarter.build, + // TODO add support for other backends + } + for _, builder := range builders { + if err := builder(ctx, host); err != nil { + return err + } + } return nil } diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go index 2ec6c05306e..9f65f782895 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go @@ -24,11 +24,6 @@ import ( "github.com/jaegertracing/jaeger/storage/spanstore" ) -const ( - memstoreName = "memstore" - badgerName = "badgerstore" -) - type storageHost struct { t *testing.T storageExtension component.Component @@ -79,143 +74,119 @@ func (e errorFactory) Close() error { func TestStorageExtensionConfigError(t *testing.T) { config := createDefaultConfig().(*Config) err := config.Validate() - require.EqualError(t, err, fmt.Sprintf("%s: no storage type present in config", ID)) + require.ErrorContains(t, err, "no storage type present in config") } -func TestStorageExtensionStartTwiceError(t *testing.T) { - ctx := context.Background() - - storageExtension := makeStorageExtension(t, memstoreName) - - host := componenttest.NewNopHost() - err := storageExtension.Start(ctx, host) - require.Error(t, err) - require.EqualError(t, err, fmt.Sprintf("duplicate memory storage name %s", memstoreName)) +func TestStorageExtensionNameConflict(t *testing.T) { + storageExtension := makeStorageExtenion(t, &Config{ + Memory: map[string]memoryCfg.Configuration{ + "foo": {MaxTraces: 10000}, + }, + Badger: map[string]badgerCfg.NamespaceConfig{ + "foo": {}, + }, + }) + err := storageExtension.Start(context.Background(), componenttest.NewNopHost()) + require.ErrorContains(t, err, "duplicate") } func TestStorageFactoryBadHostError(t *testing.T) { - makeStorageExtension(t, memstoreName) - host := componenttest.NewNopHost() - _, err := GetStorageFactory(memstoreName, host) - require.Error(t, err) - require.EqualError(t, err, fmt.Sprintf("cannot find extension '%s' (make sure it's defined earlier in the config)", ID)) + _, err := GetStorageFactory("something", host) + require.ErrorContains(t, err, "cannot find extension") } func TestStorageFactoryBadNameError(t *testing.T) { - storageExtension := makeStorageExtension(t, memstoreName) - - host := storageHost{t: t, storageExtension: storageExtension} - const badMemstoreName = "test" - - _, err := GetStorageFactory(badMemstoreName, host) - require.Error(t, err) - require.EqualError(t, err, fmt.Sprintf("cannot find storage '%s' declared with '%s' extension", badMemstoreName, ID)) + host := storageHost{t: t, storageExtension: startStorageExtension(t, "foo")} + _, err := GetStorageFactory("bar", host) + require.ErrorContains(t, err, "cannot find storage 'bar'") } func TestStorageFactoryBadShutdownError(t *testing.T) { + shutdownError := fmt.Errorf("shutdown error") storageExtension := storageExt{ - factories: make(map[string]storage.Factory), + factories: map[string]storage.Factory{ + "foo": errorFactory{closeErr: shutdownError}, + }, } - badFactoryError := fmt.Errorf("error factory") - storageExtension.factories[memstoreName] = errorFactory{closeErr: badFactoryError} - err := storageExtension.Shutdown(context.Background()) - require.ErrorIs(t, err, badFactoryError) + require.ErrorIs(t, err, shutdownError) } func TestStorageExtension(t *testing.T) { - storageExtension := makeStorageExtension(t, memstoreName) - - host := storageHost{t: t, storageExtension: storageExtension} - - _, err := GetStorageFactory(memstoreName, host) + const name = "foo" + host := storageHost{t: t, storageExtension: startStorageExtension(t, name)} + f, err := GetStorageFactory(name, host) require.NoError(t, err) + require.NotNil(t, f) } func TestBadgerStorageExtension(t *testing.T) { - ctx := context.Background() - telemetrySettings := component.TelemetrySettings{ - Logger: zap.NewNop(), - TracerProvider: nooptrace.NewTracerProvider(), - MeterProvider: noopmetric.NewMeterProvider(), - } - - config := &Config{ + storageExtension := makeStorageExtenion(t, &Config{ Badger: map[string]badgerCfg.NamespaceConfig{ - badgerName: { + "foo": { Ephemeral: true, MaintenanceInterval: 5, MetricsUpdateInterval: 10, }, }, - } - - require.NoError(t, config.Validate()) - - extensionFactory := NewFactory() - storageExtension, err := extensionFactory.CreateExtension(ctx, extension.CreateSettings{ - ID: ID, - TelemetrySettings: telemetrySettings, - BuildInfo: component.NewDefaultBuildInfo(), - }, config) - require.NoError(t, err) - - host := componenttest.NewNopHost() - err = storageExtension.Start(ctx, host) + }) + ctx := context.Background() + err := storageExtension.Start(ctx, componenttest.NewNopHost()) require.NoError(t, err) - - err = storageExtension.Start(ctx, host) - t.Cleanup(func() { require.NoError(t, storageExtension.Shutdown(ctx)) }) - require.Error(t, err) - require.EqualError(t, err, fmt.Sprintf("duplicate badger storage name %s", badgerName)) + require.NoError(t, storageExtension.Shutdown(ctx)) } func TestBadgerStorageExtensionError(t *testing.T) { - ctx := context.Background() - factory, _ := badgerCfg.NewFactoryWithConfig(badgerCfg.NamespaceConfig{}, metrics.NullFactory, zap.NewNop()) - ext := storageExt{ - config: &Config{ - Badger: map[string]badgerCfg.NamespaceConfig{ - badgerName: {}, + ext := makeStorageExtenion(t, &Config{ + Badger: map[string]badgerCfg.NamespaceConfig{ + "foo": { + KeyDirectory: "/bad/path", + ValueDirectory: "/bad/path", }, }, - logger: zap.NewNop(), - factories: map[string]storage.Factory{"badger": factory}, - } - err := ext.Start(ctx, componenttest.NewNopHost()) - require.Error(t, err) - require.EqualError(t, err, "failed to initialize badger storage: Error Creating Dir: \"\" error: mkdir : no such file or directory") + }) + err := ext.Start(context.Background(), componenttest.NewNopHost()) + require.ErrorContains(t, err, "failed to initialize badger storage") + require.ErrorContains(t, err, "/bad/path") } -func makeStorageExtension(t *testing.T, memstoreName string) component.Component { - extensionFactory := NewFactory() - - ctx := context.Background() - telemetrySettings := component.TelemetrySettings{ +func noopTelemetrySettings() component.TelemetrySettings { + return component.TelemetrySettings{ Logger: zap.L(), TracerProvider: nooptrace.NewTracerProvider(), MeterProvider: noopmetric.NewMeterProvider(), } +} + +func makeStorageExtenion(t *testing.T, config *Config) component.Component { + extensionFactory := NewFactory() + ctx := context.Background() + storageExtension, err := extensionFactory.CreateExtension(ctx, + extension.CreateSettings{ + ID: ID, + TelemetrySettings: noopTelemetrySettings(), + BuildInfo: component.NewDefaultBuildInfo(), + }, + config, + ) + require.NoError(t, err) + return storageExtension +} + +func startStorageExtension(t *testing.T, memstoreName string) component.Component { config := &Config{ Memory: map[string]memoryCfg.Configuration{ memstoreName: {MaxTraces: 10000}, }, } - err := config.Validate() - require.NoError(t, err) - - storageExtension, err := extensionFactory.CreateExtension(ctx, extension.CreateSettings{ - ID: ID, - TelemetrySettings: telemetrySettings, - BuildInfo: component.NewDefaultBuildInfo(), - }, config) - require.NoError(t, err) + require.NoError(t, config.Validate()) - host := componenttest.NewNopHost() - err = storageExtension.Start(ctx, host) + storageExtension := makeStorageExtenion(t, config) + err := storageExtension.Start(context.Background(), componenttest.NewNopHost()) require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, storageExtension.Shutdown(ctx)) }) - + t.Cleanup(func() { + require.NoError(t, storageExtension.Shutdown(context.Background())) + }) return storageExtension }