From 978ecbf0b4c7d3179a3f14a4d78988cb21184031 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com> Date: Wed, 4 Sep 2024 23:26:02 -0400 Subject: [PATCH] [jaeger-v2] Consolidate Options And NamespaceConfig For Badger Storage (#5937) ## Which problem is this PR solving? - Part of #5926 ## Description of the changes - Removed the `Options` struct and renamed `NamespaceConfig` to `Namespace` for Badger Storage - Moved v2-related configuration items to `config.go` while leaving v1 cli flags in `options.go` ## How was this change tested? - CI ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Mahad Zaryab --- .../extension/jaegerstorage/config.go | 16 +- .../extension/jaegerstorage/extension_test.go | 2 +- plugin/storage/badger/config.go | 87 ++++++++++ plugin/storage/badger/config_test.go | 47 ++++++ .../badger/dependencystore/storage_test.go | 4 +- plugin/storage/badger/factory.go | 54 +++---- plugin/storage/badger/factory_test.go | 16 +- plugin/storage/badger/options.go | 150 +++--------------- plugin/storage/badger/options_test.go | 83 +++------- .../badger/spanstore/read_write_test.go | 12 +- plugin/storage/badger/stats_linux.go | 4 +- plugin/storage/badger/stats_linux_test.go | 4 +- plugin/storage/badger/stats_test.go | 4 +- .../storage/integration/badgerstore_test.go | 2 +- 14 files changed, 237 insertions(+), 248 deletions(-) create mode 100644 plugin/storage/badger/config.go create mode 100644 plugin/storage/badger/config_test.go diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config.go b/cmd/jaeger/internal/extension/jaegerstorage/config.go index 98d19d08270..d297bea5b3d 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/config.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/config.go @@ -39,12 +39,12 @@ type Config struct { } type Backend struct { - Memory *memory.Configuration `mapstructure:"memory"` - Badger *badger.NamespaceConfig `mapstructure:"badger"` - GRPC *grpc.ConfigV2 `mapstructure:"grpc"` - Cassandra *cassandra.Options `mapstructure:"cassandra"` - Elasticsearch *esCfg.Configuration `mapstructure:"elasticsearch"` - Opensearch *esCfg.Configuration `mapstructure:"opensearch"` + Memory *memory.Configuration `mapstructure:"memory"` + Badger *badger.Config `mapstructure:"badger"` + GRPC *grpc.ConfigV2 `mapstructure:"grpc"` + Cassandra *cassandra.Options `mapstructure:"cassandra"` + Elasticsearch *esCfg.Configuration `mapstructure:"elasticsearch"` + Opensearch *esCfg.Configuration `mapstructure:"opensearch"` } type MetricBackends struct { @@ -62,8 +62,8 @@ func (cfg *Backend) Unmarshal(conf *confmap.Conf) error { } } if conf.IsSet("badger") { - v := badger.DefaultNamespaceConfig() - cfg.Badger = &v + v := badger.DefaultConfig() + cfg.Badger = v } if conf.IsSet("grpc") { v := grpc.DefaultConfigV2() diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go index 472b160fb13..3e1ef000f7e 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go @@ -123,7 +123,7 @@ func TestBadger(t *testing.T) { ext := makeStorageExtenion(t, &Config{ Backends: map[string]Backend{ "foo": { - Badger: &badger.NamespaceConfig{ + Badger: &badger.Config{ Ephemeral: true, MaintenanceInterval: 5, MetricsUpdateInterval: 10, diff --git a/plugin/storage/badger/config.go b/plugin/storage/badger/config.go new file mode 100644 index 00000000000..aacf9ec153b --- /dev/null +++ b/plugin/storage/badger/config.go @@ -0,0 +1,87 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package badger + +import ( + "os" + "path/filepath" + "time" + + "github.com/asaskevich/govalidator" +) + +const ( + defaultMaintenanceInterval time.Duration = 5 * time.Minute + defaultMetricsUpdateInterval time.Duration = 10 * time.Second + defaultTTL time.Duration = time.Hour * 72 + defaultDataDir string = string(os.PathSeparator) + "data" + defaultValueDir string = defaultDataDir + string(os.PathSeparator) + "values" + defaultKeysDir string = defaultDataDir + string(os.PathSeparator) + "keys" +) + +// Config is badger's internal configuration data. +type Config struct { + // TTL holds time-to-live configuration for the badger store. + TTL TTL `mapstructure:"ttl"` + // Directories contains the configuration for where items are stored. Ephemeral must be + // set to false for this configuration to take effect. + Directories Directories `mapstructure:"directories"` + // Ephemeral, if set to true, will store data in a temporary file system. + // If set to true, the configuration in Directories is ignored. + Ephemeral bool `mapstructure:"ephemeral"` + // SyncWrites, if set to true, will immediately sync all writes to disk. Note that + // setting this field to true will affect write performance. + SyncWrites bool `mapstructure:"consistency"` + // MaintenanceInterval is the regular interval after which a maintenance job is + // run on the values in the store. + MaintenanceInterval time.Duration `mapstructure:"maintenance_interval"` + // MetricsUpdateInterval is the regular interval after which metrics are collected + // by Jaeger. + MetricsUpdateInterval time.Duration `mapstructure:"metrics_update_interval"` + // ReadOnly opens the data store in read-only mode. Multiple instances can open the same + // store in read-only mode. Values still in the write-ahead-log must be replayed before opening. + ReadOnly bool `mapstructure:"read_only"` +} + +type TTL struct { + // SpanStore holds the amount of time that the span store data is stored. + // Once this duration has passed for a given key, span store data will + // no longer be accessible. + Spans time.Duration `mapstructure:"spans"` +} + +type Directories struct { + // Keys contains the directory in which the keys are stored. + Keys string `mapstructure:"keys"` + // Values contains the directory in which the values are stored. + Values string `mapstructure:"values"` +} + +func DefaultConfig() *Config { + defaultBadgerDataDir := getCurrentExecutableDir() + return &Config{ + TTL: TTL{ + Spans: defaultTTL, + }, + SyncWrites: false, // Performance over durability + Ephemeral: true, // Default is ephemeral storage + Directories: Directories{ + Keys: defaultBadgerDataDir + defaultKeysDir, + Values: defaultBadgerDataDir + defaultValueDir, + }, + MaintenanceInterval: defaultMaintenanceInterval, + MetricsUpdateInterval: defaultMetricsUpdateInterval, + } +} + +func getCurrentExecutableDir() string { + // We ignore the error, this will fail later when trying to start the store + exec, _ := os.Executable() + return filepath.Dir(exec) +} + +func (c *Config) Validate() error { + _, err := govalidator.ValidateStruct(c) + return err +} diff --git a/plugin/storage/badger/config_test.go b/plugin/storage/badger/config_test.go new file mode 100644 index 00000000000..8b8fbf2d1c4 --- /dev/null +++ b/plugin/storage/badger/config_test.go @@ -0,0 +1,47 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package badger + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestValidate_DoesNotReturnErrorWhenValid(t *testing.T) { + tests := []struct { + name string + cfg *Config + }{ + { + name: "non-required fields not set", + cfg: &Config{}, + }, + { + name: "all fields are set", + cfg: &Config{ + TTL: TTL{ + Spans: time.Second, + }, + Directories: Directories{ + Keys: "some-key-directory", + Values: "some-values-directory", + }, + Ephemeral: false, + SyncWrites: false, + MaintenanceInterval: time.Second, + MetricsUpdateInterval: time.Second, + ReadOnly: false, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := test.cfg.Validate() + require.NoError(t, err) + }) + } +} diff --git a/plugin/storage/badger/dependencystore/storage_test.go b/plugin/storage/badger/dependencystore/storage_test.go index f2e7502a4a5..187e524a0b9 100644 --- a/plugin/storage/badger/dependencystore/storage_test.go +++ b/plugin/storage/badger/dependencystore/storage_test.go @@ -28,8 +28,8 @@ func runFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, require.NoError(tb, f.Close()) }() - opts := badger.NewOptions() - v, command := config.Viperize(opts.AddFlags) + cfg := badger.DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{ "--badger.ephemeral=true", "--badger.consistency=false", diff --git a/plugin/storage/badger/factory.go b/plugin/storage/badger/factory.go index 1fdb2e8872c..63d05612c22 100644 --- a/plugin/storage/badger/factory.go +++ b/plugin/storage/badger/factory.go @@ -50,10 +50,10 @@ var ( // interface comformance checks // Factory implements storage.Factory for Badger backend. type Factory struct { - Options *Options - store *badger.DB - cache *badgerStore.CacheStore - logger *zap.Logger + Config *Config + store *badger.DB + cache *badgerStore.CacheStore + logger *zap.Logger tmpDir string maintenanceDone chan bool @@ -77,18 +77,18 @@ type Factory struct { // NewFactory creates a new Factory. func NewFactory() *Factory { return &Factory{ - Options: NewOptions(), + Config: DefaultConfig(), maintenanceDone: make(chan bool), } } func NewFactoryWithConfig( - cfg NamespaceConfig, + cfg Config, metricsFactory metrics.Factory, logger *zap.Logger, ) (*Factory, error) { f := NewFactory() - f.configureFromOptions(&Options{Primary: cfg}) + f.configure(&cfg) err := f.Initialize(metricsFactory, logger) if err != nil { return nil, err @@ -98,18 +98,18 @@ func NewFactoryWithConfig( // AddFlags implements plugin.Configurable func (f *Factory) AddFlags(flagSet *flag.FlagSet) { - f.Options.AddFlags(flagSet) + f.Config.AddFlags(flagSet) } // InitFromViper implements plugin.Configurable func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { - f.Options.InitFromViper(v, logger) - f.configureFromOptions(f.Options) + f.Config.InitFromViper(v, logger) + f.configure(f.Config) } -// InitFromOptions initializes Factory from supplied options -func (f *Factory) configureFromOptions(opts *Options) { - f.Options = opts +// configure initializes Factory from supplied Config. +func (f *Factory) configure(config *Config) { + f.Config = config } // Initialize implements storage.Factory @@ -118,7 +118,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) opts := badger.DefaultOptions("") - if f.Options.Primary.Ephemeral { + if f.Config.Ephemeral { opts.SyncWrites = false // Error from TempDir is ignored to satisfy Codecov dir, _ := os.MkdirTemp("", "badger") @@ -126,19 +126,19 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) opts.Dir = f.tmpDir opts.ValueDir = f.tmpDir - f.Options.Primary.Directories.Keys = f.tmpDir - f.Options.Primary.Directories.Values = f.tmpDir + f.Config.Directories.Keys = f.tmpDir + f.Config.Directories.Values = f.tmpDir } else { // Errors are ignored as they're caught in the Open call - initializeDir(f.Options.Primary.Directories.Keys) - initializeDir(f.Options.Primary.Directories.Values) + initializeDir(f.Config.Directories.Keys) + initializeDir(f.Config.Directories.Values) - opts.SyncWrites = f.Options.Primary.SyncWrites - opts.Dir = f.Options.Primary.Directories.Keys - opts.ValueDir = f.Options.Primary.Directories.Values + opts.SyncWrites = f.Config.SyncWrites + opts.Dir = f.Config.Directories.Keys + opts.ValueDir = f.Config.Directories.Values // These options make no sense with ephemeral data - opts.ReadOnly = f.Options.Primary.ReadOnly + opts.ReadOnly = f.Config.ReadOnly } store, err := badger.Open(opts) @@ -147,7 +147,7 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) } f.store = store - f.cache = badgerStore.NewCacheStore(f.store, f.Options.Primary.TTL.Spans, true) + f.cache = badgerStore.NewCacheStore(f.store, f.Config.TTL.Spans, true) f.metrics.ValueLogSpaceAvailable = metricsFactory.Gauge(metrics.Options{Name: valueLogSpaceAvailableName}) f.metrics.KeyLogSpaceAvailable = metricsFactory.Gauge(metrics.Options{Name: keyLogSpaceAvailableName}) @@ -178,7 +178,7 @@ func (f *Factory) CreateSpanReader() (spanstore.Reader, error) { // CreateSpanWriter implements storage.Factory func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { - return badgerStore.NewSpanWriter(f.store, f.cache, f.Options.Primary.TTL.Spans), nil + return badgerStore.NewSpanWriter(f.store, f.cache, f.Config.TTL.Spans), nil } // CreateDependencyReader implements storage.Factory @@ -206,7 +206,7 @@ func (f *Factory) Close() error { err := f.store.Close() // Remove tmp files if this was ephemeral storage - if f.Options.Primary.Ephemeral { + if f.Config.Ephemeral { errSecondary := os.RemoveAll(f.tmpDir) if err == nil { err = errSecondary @@ -218,7 +218,7 @@ func (f *Factory) Close() error { // Maintenance starts a background maintenance job for the badger K/V store, such as ValueLogGC func (f *Factory) maintenance() { - maintenanceTicker := time.NewTicker(f.Options.Primary.MaintenanceInterval) + maintenanceTicker := time.NewTicker(f.Config.MaintenanceInterval) defer maintenanceTicker.Stop() for { select { @@ -244,7 +244,7 @@ func (f *Factory) maintenance() { } func (f *Factory) metricsCopier() { - metricsTicker := time.NewTicker(f.Options.Primary.MetricsUpdateInterval) + metricsTicker := time.NewTicker(f.Config.MetricsUpdateInterval) defer metricsTicker.Stop() for { select { diff --git a/plugin/storage/badger/factory_test.go b/plugin/storage/badger/factory_test.go index 62a13207900..6190f1510b6 100644 --- a/plugin/storage/badger/factory_test.go +++ b/plugin/storage/badger/factory_test.go @@ -183,26 +183,24 @@ func TestBadgerMetrics(t *testing.T) { require.NoError(t, err) } -func TestConfigureFromOptions(t *testing.T) { +func TestConfigure(t *testing.T) { f := NewFactory() - opts := &Options{ - Primary: NamespaceConfig{ - MaintenanceInterval: 42 * time.Second, - }, + config := &Config{ + MaintenanceInterval: 42 * time.Second, } - f.configureFromOptions(opts) - assert.Equal(t, opts, f.Options) + f.configure(config) + assert.Equal(t, config, f.Config) } func TestBadgerStorageFactoryWithConfig(t *testing.T) { - cfg := NamespaceConfig{} + cfg := Config{} _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop()) require.Error(t, err) require.ErrorContains(t, err, "Error Creating Dir") tmp := os.TempDir() defer os.Remove(tmp) - cfg = NamespaceConfig{ + cfg = Config{ Directories: Directories{ Keys: tmp, Values: tmp, diff --git a/plugin/storage/badger/options.go b/plugin/storage/badger/options.go index 6ebdcc78b26..1bfa611de17 100644 --- a/plugin/storage/badger/options.go +++ b/plugin/storage/badger/options.go @@ -5,65 +5,11 @@ package badger import ( "flag" - "os" - "path/filepath" - "time" - "github.com/asaskevich/govalidator" "github.com/spf13/viper" "go.uber.org/zap" ) -// Options store storage plugin related configs -type Options struct { - Primary NamespaceConfig `mapstructure:",squash"` - // This storage plugin does not support additional namespaces -} - -// NamespaceConfig is badger's internal configuration data. -type NamespaceConfig struct { - // TTL holds time-to-live configuration for the badger store. - TTL TTL `mapstructure:"ttl"` - // Directories contains the configuration for where items are stored. Ephemeral must be - // set to false for this configuration to take effect. - Directories Directories `mapstructure:"directories"` - // Ephemeral, if set to true, will store data in a temporary file system. - // If set to true, the configuration in Directories is ignored. - Ephemeral bool `mapstructure:"ephemeral"` - // SyncWrites, if set to true, will immediately sync all writes to disk. Note that - // setting this field to true will affect write performance. - SyncWrites bool `mapstructure:"consistency"` - // MaintenanceInterval is the regular interval after which a maintenance job is - // run on the values in the store. - MaintenanceInterval time.Duration `mapstructure:"maintenance_interval"` - // MetricsUpdateInterval is the regular interval after which metrics are collected - // by Jaeger. - MetricsUpdateInterval time.Duration `mapstructure:"metrics_update_interval"` - // ReadOnly opens the data store in read-only mode. Multiple instances can open the same - // store in read-only mode. Values still in the write-ahead-log must be replayed before opening. - ReadOnly bool `mapstructure:"read_only"` -} - -type TTL struct { - // SpanStore holds the amount of time that the span store data is stored. - // Once this duration has passed for a given key, span store data will - // no longer be accessible. - Spans time.Duration `mapstructure:"spans"` -} - -type Directories struct { - // Keys contains the directory in which the keys are stored. - Keys string `mapstructure:"keys"` - // Values contains the directory in which the values are stored. - Values string `mapstructure:"values"` -} - -const ( - defaultMaintenanceInterval time.Duration = 5 * time.Minute - defaultMetricsUpdateInterval time.Duration = 10 * time.Second - defaultTTL time.Duration = time.Hour * 72 -) - const ( prefix = "badger" suffixKeyDirectory = ".directory-key" @@ -74,116 +20,64 @@ const ( suffixMaintenanceInterval = ".maintenance-interval" suffixMetricsInterval = ".metrics-update-interval" // Intended only for testing purposes suffixReadOnly = ".read-only" - defaultDataDir = string(os.PathSeparator) + "data" - defaultValueDir = defaultDataDir + string(os.PathSeparator) + "values" - defaultKeysDir = defaultDataDir + string(os.PathSeparator) + "keys" ) -func DefaultNamespaceConfig() NamespaceConfig { - defaultBadgerDataDir := getCurrentExecutableDir() - return NamespaceConfig{ - TTL: TTL{ - Spans: defaultTTL, - }, - SyncWrites: false, // Performance over durability - Ephemeral: true, // Default is ephemeral storage - Directories: Directories{ - Keys: defaultBadgerDataDir + defaultKeysDir, - Values: defaultBadgerDataDir + defaultValueDir, - }, - MaintenanceInterval: defaultMaintenanceInterval, - MetricsUpdateInterval: defaultMetricsUpdateInterval, - } -} - -// NewOptions creates a new Options struct. -// @nocommit func NewOptions(primaryNamespace string, _ ...string /* otherNamespaces */) *Options { -func NewOptions() *Options { - defaultConfig := DefaultNamespaceConfig() - - options := &Options{ - Primary: defaultConfig, - } - - return options -} - -func getCurrentExecutableDir() string { - // We ignore the error, this will fail later when trying to start the store - exec, _ := os.Executable() - return filepath.Dir(exec) -} - -// AddFlags adds flags for Options -func (opt *Options) AddFlags(flagSet *flag.FlagSet) { - addFlags(flagSet, opt.Primary) -} - -func addFlags(flagSet *flag.FlagSet, nsConfig NamespaceConfig) { +// AddFlags adds flags for Config. +func (c *Config) AddFlags(flagSet *flag.FlagSet) { flagSet.Bool( prefix+suffixEphemeral, - nsConfig.Ephemeral, + c.Ephemeral, "Mark this storage ephemeral, data is stored in tmpfs.", ) flagSet.Duration( prefix+suffixSpanstoreTTL, - nsConfig.TTL.Spans, + c.TTL.Spans, "How long to store the data. Format is time.Duration (https://golang.org/pkg/time/#Duration)", ) flagSet.String( prefix+suffixKeyDirectory, - nsConfig.Directories.Keys, + c.Directories.Keys, "Path to store the keys (indexes), this directory should reside in SSD disk. Set ephemeral to false if you want to define this setting.", ) flagSet.String( prefix+suffixValueDirectory, - nsConfig.Directories.Values, + c.Directories.Values, "Path to store the values (spans). Set ephemeral to false if you want to define this setting.", ) flagSet.Bool( prefix+suffixSyncWrite, - nsConfig.SyncWrites, + c.SyncWrites, "If all writes should be synced immediately to physical disk. This will impact write performance.", ) flagSet.Duration( prefix+suffixMaintenanceInterval, - nsConfig.MaintenanceInterval, + c.MaintenanceInterval, "How often the maintenance thread for values is ran. Format is time.Duration (https://golang.org/pkg/time/#Duration)", ) flagSet.Duration( prefix+suffixMetricsInterval, - nsConfig.MetricsUpdateInterval, + c.MetricsUpdateInterval, "How often the badger metrics are collected by Jaeger. Format is time.Duration (https://golang.org/pkg/time/#Duration)", ) flagSet.Bool( prefix+suffixReadOnly, - nsConfig.ReadOnly, + c.ReadOnly, "Allows to open badger database in read only mode. Multiple instances can open same database in read-only mode. Values still in the write-ahead-log must be replayed before opening.", ) } -// InitFromViper initializes Options with properties from viper -func (opt *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) { - initFromViper(&opt.Primary, v, logger) -} - -func initFromViper(cfg *NamespaceConfig, v *viper.Viper, _ *zap.Logger) { - cfg.Ephemeral = v.GetBool(prefix + suffixEphemeral) - cfg.Directories.Keys = v.GetString(prefix + suffixKeyDirectory) - cfg.Directories.Values = v.GetString(prefix + suffixValueDirectory) - cfg.SyncWrites = v.GetBool(prefix + suffixSyncWrite) - cfg.TTL.Spans = v.GetDuration(prefix + suffixSpanstoreTTL) - cfg.MaintenanceInterval = v.GetDuration(prefix + suffixMaintenanceInterval) - cfg.MetricsUpdateInterval = v.GetDuration(prefix + suffixMetricsInterval) - cfg.ReadOnly = v.GetBool(prefix + suffixReadOnly) -} - -// GetPrimary returns the primary namespace configuration -func (opt *Options) GetPrimary() NamespaceConfig { - return opt.Primary +// InitFromViper initializes Config with properties from viper. +func (c *Config) InitFromViper(v *viper.Viper, logger *zap.Logger) { + initFromViper(c, v, logger) } -func (c *NamespaceConfig) Validate() error { - _, err := govalidator.ValidateStruct(c) - return err +func initFromViper(config *Config, v *viper.Viper, _ *zap.Logger) { + config.Ephemeral = v.GetBool(prefix + suffixEphemeral) + config.Directories.Keys = v.GetString(prefix + suffixKeyDirectory) + config.Directories.Values = v.GetString(prefix + suffixValueDirectory) + config.SyncWrites = v.GetBool(prefix + suffixSyncWrite) + config.TTL.Spans = v.GetDuration(prefix + suffixSpanstoreTTL) + config.MaintenanceInterval = v.GetDuration(prefix + suffixMaintenanceInterval) + config.MetricsUpdateInterval = v.GetDuration(prefix + suffixMetricsInterval) + config.ReadOnly = v.GetBool(prefix + suffixReadOnly) } diff --git a/plugin/storage/badger/options_test.go b/plugin/storage/badger/options_test.go index ef1d6e26cc9..8f1a051063b 100644 --- a/plugin/storage/badger/options_test.go +++ b/plugin/storage/badger/options_test.go @@ -8,26 +8,25 @@ import ( "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" ) -func TestDefaultOptionsParsing(t *testing.T) { - opts := NewOptions() - v, command := config.Viperize(opts.AddFlags) +func TestDefaultConfigParsing(t *testing.T) { + cfg := DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{}) - opts.InitFromViper(v, zap.NewNop()) + cfg.InitFromViper(v, zap.NewNop()) - assert.True(t, opts.GetPrimary().Ephemeral) - assert.False(t, opts.GetPrimary().SyncWrites) - assert.Equal(t, time.Duration(72*time.Hour), opts.GetPrimary().TTL.Spans) + assert.True(t, cfg.Ephemeral) + assert.False(t, cfg.SyncWrites) + assert.Equal(t, time.Duration(72*time.Hour), cfg.TTL.Spans) } -func TestParseOptions(t *testing.T) { - opts := NewOptions() - v, command := config.Viperize(opts.AddFlags) +func TestParseConfig(t *testing.T) { + cfg := DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{ "--badger.ephemeral=false", "--badger.consistency=true", @@ -35,58 +34,22 @@ func TestParseOptions(t *testing.T) { "--badger.directory-value=/mnt/slow/badger", "--badger.span-store-ttl=168h", }) - opts.InitFromViper(v, zap.NewNop()) - - assert.False(t, opts.GetPrimary().Ephemeral) - assert.True(t, opts.GetPrimary().SyncWrites) - assert.Equal(t, time.Duration(168*time.Hour), opts.GetPrimary().TTL.Spans) - assert.Equal(t, "/var/lib/badger", opts.GetPrimary().Directories.Keys) - assert.Equal(t, "/mnt/slow/badger", opts.GetPrimary().Directories.Values) - assert.False(t, opts.GetPrimary().ReadOnly) + cfg.InitFromViper(v, zap.NewNop()) + + assert.False(t, cfg.Ephemeral) + assert.True(t, cfg.SyncWrites) + assert.Equal(t, time.Duration(168*time.Hour), cfg.TTL.Spans) + assert.Equal(t, "/var/lib/badger", cfg.Directories.Keys) + assert.Equal(t, "/mnt/slow/badger", cfg.Directories.Values) + assert.False(t, cfg.ReadOnly) } -func TestReadOnlyOptions(t *testing.T) { - opts := NewOptions() - v, command := config.Viperize(opts.AddFlags) +func TestReadOnlyConfig(t *testing.T) { + cfg := DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{ "--badger.read-only=true", }) - opts.InitFromViper(v, zap.NewNop()) - assert.True(t, opts.GetPrimary().ReadOnly) -} - -func TestValidate_DoesNotReturnErrorWhenValid(t *testing.T) { - tests := []struct { - name string - config *NamespaceConfig - }{ - { - name: "non-required fields not set", - config: &NamespaceConfig{}, - }, - { - name: "all fields are set", - config: &NamespaceConfig{ - TTL: TTL{ - Spans: time.Second, - }, - Directories: Directories{ - Keys: "some-key-directory", - Values: "some-values-directory", - }, - Ephemeral: false, - SyncWrites: false, - MaintenanceInterval: time.Second, - MetricsUpdateInterval: time.Second, - ReadOnly: false, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - err := test.config.Validate() - require.NoError(t, err) - }) - } + cfg.InitFromViper(v, zap.NewNop()) + assert.True(t, cfg.ReadOnly) } diff --git a/plugin/storage/badger/spanstore/read_write_test.go b/plugin/storage/badger/spanstore/read_write_test.go index e79c3958961..6fa17c497a8 100644 --- a/plugin/storage/badger/spanstore/read_write_test.go +++ b/plugin/storage/badger/spanstore/read_write_test.go @@ -373,8 +373,8 @@ func TestPersist(t *testing.T) { require.NoError(t, f.Close()) }() - opts := badger.NewOptions() - v, command := config.Viperize(opts.AddFlags) + cfg := badger.DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) keyParam := fmt.Sprintf("--badger.directory-key=%s", dir) valueParam := fmt.Sprintf("--badger.directory-value=%s", dir) @@ -438,8 +438,8 @@ func runFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, require.NoError(tb, f.Close()) }() - opts := badger.NewOptions() - v, command := config.Viperize(opts.AddFlags) + cfg := badger.DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{ "--badger.ephemeral=true", "--badger.consistency=false", @@ -598,8 +598,8 @@ func BenchmarkServiceIndexLimitFetch(b *testing.B) { func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader)) { assertion := require.New(tb) f := badger.NewFactory() - opts := badger.NewOptions() - v, command := config.Viperize(opts.AddFlags) + cfg := badger.DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) dir := filepath.Join(tb.TempDir(), "badger-testRun") err := os.MkdirAll(dir, 0o700) diff --git a/plugin/storage/badger/stats_linux.go b/plugin/storage/badger/stats_linux.go index 00341da12b3..3cb919ce639 100644 --- a/plugin/storage/badger/stats_linux.go +++ b/plugin/storage/badger/stats_linux.go @@ -12,11 +12,11 @@ func (f *Factory) diskStatisticsUpdate() error { // In case of ephemeral these are the same, but we'll report them separately for consistency var keyDirStatfs unix.Statfs_t // Error ignored to satisfy Codecov - _ = unix.Statfs(f.Options.GetPrimary().Directories.Keys, &keyDirStatfs) + _ = unix.Statfs(f.Config.Directories.Keys, &keyDirStatfs) var valDirStatfs unix.Statfs_t // Error ignored to satisfy Codecov - _ = unix.Statfs(f.Options.GetPrimary().Directories.Values, &valDirStatfs) + _ = unix.Statfs(f.Config.Directories.Values, &valDirStatfs) // Using Bavail instead of Bfree to get non-priviledged user space available //nolint: gosec // G115 diff --git a/plugin/storage/badger/stats_linux_test.go b/plugin/storage/badger/stats_linux_test.go index 685eaf0f92b..d8af2aa4bcd 100644 --- a/plugin/storage/badger/stats_linux_test.go +++ b/plugin/storage/badger/stats_linux_test.go @@ -16,8 +16,8 @@ import ( func TestDiskStatisticsUpdate(t *testing.T) { f := NewFactory() - opts := NewOptions() - v, command := config.Viperize(opts.AddFlags) + cfg := DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{ "--badger.ephemeral=true", "--badger.consistency=false", diff --git a/plugin/storage/badger/stats_test.go b/plugin/storage/badger/stats_test.go index a318bae9af7..b98c3e645cb 100644 --- a/plugin/storage/badger/stats_test.go +++ b/plugin/storage/badger/stats_test.go @@ -18,8 +18,8 @@ import ( func TestDiskStatisticsUpdate(t *testing.T) { f := NewFactory() - opts := NewOptions() - v, command := config.Viperize(opts.AddFlags) + cfg := DefaultConfig() + v, command := config.Viperize(cfg.AddFlags) command.ParseFlags([]string{ "--badger.ephemeral=true", "--badger.consistency=false", diff --git a/plugin/storage/integration/badgerstore_test.go b/plugin/storage/integration/badgerstore_test.go index ec5030b33dc..d7ac474e27f 100644 --- a/plugin/storage/integration/badgerstore_test.go +++ b/plugin/storage/integration/badgerstore_test.go @@ -22,7 +22,7 @@ type BadgerIntegrationStorage struct { func (s *BadgerIntegrationStorage) initialize(t *testing.T) { s.factory = badger.NewFactory() - s.factory.Options.Primary.Ephemeral = false + s.factory.Config.Ephemeral = false logger := zaptest.NewLogger(t, zaptest.WrapOptions(zap.AddCaller())) err := s.factory.Initialize(metrics.NullFactory, logger)