Skip to content
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.

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

[jaeger-v2] Add Validation And Comments to Badger Storage Config #5927

Merged
merged 10 commits into from
Sep 5, 2024
41 changes: 31 additions & 10 deletions plugin/storage/badger/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"time"

"github.com/asaskevich/govalidator"
"github.com/spf13/viper"
"go.uber.org/zap"
)
Expand All @@ -19,18 +20,33 @@ type Options struct {
// This storage plugin does not support additional namespaces
}

// NamespaceConfig is badger's internal configuration data
// NamespaceConfig is badger's internal configuration data.
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
type NamespaceConfig struct {
namespace string
SpanStoreTTL time.Duration `mapstructure:"span_store_ttl"`
ValueDirectory string `mapstructure:"directory_value"`
KeyDirectory string `mapstructure:"directory_key"`
// Setting this to true will ignore ValueDirectory and KeyDirectory
Ephemeral bool `mapstructure:"ephemeral"`
SyncWrites bool `mapstructure:"consistency"`
MaintenanceInterval time.Duration `mapstructure:"maintenance_interval"`
namespace string
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to make this field configurable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not needed, see #5929

// SpanStoreTTL holds the amount of time that the span store data is stored.
// Once this duration has passed for a given key, it will no longer be accessible.
SpanStoreTTL time.Duration `mapstructure:"span_store_ttl"`
// KeyDirectory is the directory in which the keys are stored. Ephemeral must be
// set to false for this configuration to take effect.
KeyDirectory string `mapstructure:"directory_key"`
// ValueDirectory is the directory in which the values should be stored. Ephemeral must be
// set to false for this configuration to take effect.
ValueDirectory string `mapstructure:"directory_value"`
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to group these?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could, e.g.

directories:
  keys: ...
  values: ...

// Ephemeral, if set to true, will store data in a temporary file system.
// If set to true, KeyDirectory and ValueDirectory are 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 bool `mapstructure:"read_only"`
// 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"`
}

const (
Expand Down Expand Up @@ -152,3 +168,8 @@ func initFromViper(cfg *NamespaceConfig, v *viper.Viper, _ *zap.Logger) {
func (opt *Options) GetPrimary() NamespaceConfig {
return opt.Primary
}

func (c *NamespaceConfig) Validate() error {
_, err := govalidator.ValidateStruct(c)
return err
}
33 changes: 33 additions & 0 deletions plugin/storage/badger/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/config"
Expand Down Expand Up @@ -53,3 +54,35 @@ func TestReadOnlyOptions(t *testing.T) {
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{
SpanStoreTTL: time.Second,
KeyDirectory: "some-key-directory",
ValueDirectory: "some-value-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)
})
}
}
Loading