Skip to content

Commit

Permalink
[jaeger-v2] Consolidate Options And NamespaceConfig For Badger Storage (
Browse files Browse the repository at this point in the history
#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 <mahadzaryab1@gmail.com>
  • Loading branch information
mahadzaryab1 authored Sep 5, 2024
1 parent 8d8719a commit 978ecbf
Show file tree
Hide file tree
Showing 14 changed files with 237 additions and 248 deletions.
16 changes: 8 additions & 8 deletions cmd/jaeger/internal/extension/jaegerstorage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
87 changes: 87 additions & 0 deletions plugin/storage/badger/config.go
Original file line number Diff line number Diff line change
@@ -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
}
47 changes: 47 additions & 0 deletions plugin/storage/badger/config_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
4 changes: 2 additions & 2 deletions plugin/storage/badger/dependencystore/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
54 changes: 27 additions & 27 deletions plugin/storage/badger/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -118,27 +118,27 @@ 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")
f.tmpDir = dir
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)
Expand All @@ -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})
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
16 changes: 7 additions & 9 deletions plugin/storage/badger/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 978ecbf

Please sign in to comment.