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.

Sign up for GitHub

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

Simplify configs organization #5690

Merged
merged 1 commit into from
Jun 30, 2024
Merged

Simplify configs organization #5690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/jaeger/internal/exporters/storageexporter/exporter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
"github.com/jaegertracing/jaeger/model"
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/plugin/storage/memory"
"github.com/jaegertracing/jaeger/storage"
factoryMocks "github.com/jaegertracing/jaeger/storage/mocks"
)
Expand Down Expand Up @@ -166,7 +166,7 @@ func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
TracerProvider: nooptrace.NewTracerProvider(),
},
},
&jaegerstorage.Config{Memory: map[string]memoryCfg.Configuration{
&jaegerstorage.Config{Memory: map[string]memory.Configuration{
memstoreName: {MaxTraces: 10000},
}})
require.NoError(t, err)
Expand Down
23 changes: 9 additions & 14 deletions cmd/jaeger/internal/extension/jaegerstorage/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,25 @@ import (
"reflect"

esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
grpcCfg "github.com/jaegertracing/jaeger/plugin/storage/grpc"
"github.com/jaegertracing/jaeger/plugin/storage/grpc"
"github.com/jaegertracing/jaeger/plugin/storage/memory"
)

// Config has the configuration for jaeger-query,
type Config struct {
Memory map[string]memoryCfg.Configuration `mapstructure:"memory"`
Badger map[string]badgerCfg.NamespaceConfig `mapstructure:"badger"`
GRPC map[string]grpcCfg.ConfigV2 `mapstructure:"grpc"`
Opensearch map[string]esCfg.Configuration `mapstructure:"opensearch"`
Elasticsearch map[string]esCfg.Configuration `mapstructure:"elasticsearch"`
Cassandra map[string]cassandra.Options `mapstructure:"cassandra"`
Memory map[string]memory.Configuration `mapstructure:"memory"`
Badger map[string]badger.NamespaceConfig `mapstructure:"badger"`
GRPC map[string]grpc.ConfigV2 `mapstructure:"grpc"`
Opensearch map[string]esCfg.Configuration `mapstructure:"opensearch"`
Elasticsearch map[string]esCfg.Configuration `mapstructure:"elasticsearch"`
Cassandra map[string]cassandra.Options `mapstructure:"cassandra"`
// TODO add other storage types here
// TODO how will this work with 3rd party storage implementations?
// Option: instead of looking for specific name, check interface.
}

type MemoryStorage struct {
Name string `mapstructure:"name"`
memoryCfg.Configuration
}

func (cfg *Config) Validate() error {
emptyCfg := createDefaultConfig().(*Config)
if reflect.DeepEqual(*cfg, *emptyCfg) {
Expand Down
5 changes: 2 additions & 3 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage/factoryadapter"
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
Expand Down Expand Up @@ -107,13 +106,13 @@
}

func (s *storageExt) Start(ctx context.Context, host component.Host) error {
memStarter := &starter[memoryCfg.Configuration, *memory.Factory]{
memStarter := &starter[memory.Configuration, *memory.Factory]{

Check warning on line 109 in cmd/jaeger/internal/extension/jaegerstorage/extension.go

View check run for this annotation

Codecov / codecov/patch

cmd/jaeger/internal/extension/jaegerstorage/extension.go#L109 

Added line #L109 was not covered by tests
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,
cfg memory.Configuration,

Check warning on line 115 in cmd/jaeger/internal/extension/jaegerstorage/extension.go

View check run for this annotation

Codecov / codecov/patch

cmd/jaeger/internal/extension/jaegerstorage/extension.go#L115 

Added line #L115 was not covered by tests
metricsFactory metrics.Factory,
logger *zap.Logger,
) (*memory.Factory, error) {
Expand Down
14 changes: 7 additions & 7 deletions cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (
"go.uber.org/zap"

esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/pkg/testutils"
badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/badger"
"github.com/jaegertracing/jaeger/plugin/storage/memory"
"github.com/jaegertracing/jaeger/storage"
"github.com/jaegertracing/jaeger/storage/dependencystore"
"github.com/jaegertracing/jaeger/storage/spanstore"
Expand Down Expand Up @@ -80,10 +80,10 @@ func TestStorageExtensionConfigError(t *testing.T) {

func TestStorageExtensionNameConflict(t *testing.T) {
storageExtension := makeStorageExtenion(t, &Config{
Memory: map[string]memoryCfg.Configuration{
Memory: map[string]memory.Configuration{
"foo": {MaxTraces: 10000},
},
Badger: map[string]badgerCfg.NamespaceConfig{
Badger: map[string]badger.NamespaceConfig{
"foo": {},
},
})
Expand Down Expand Up @@ -134,7 +134,7 @@ func TestStorageExtension(t *testing.T) {

func TestBadgerStorageExtension(t *testing.T) {
storageExtension := makeStorageExtenion(t, &Config{
Badger: map[string]badgerCfg.NamespaceConfig{
Badger: map[string]badger.NamespaceConfig{
"foo": {
Ephemeral: true,
MaintenanceInterval: 5,
Expand All @@ -150,7 +150,7 @@ func TestBadgerStorageExtension(t *testing.T) {

func TestBadgerStorageExtensionError(t *testing.T) {
ext := makeStorageExtenion(t, &Config{
Badger: map[string]badgerCfg.NamespaceConfig{
Badger: map[string]badger.NamespaceConfig{
"foo": {
KeyDirectory: "/bad/path",
ValueDirectory: "/bad/path",
Expand Down Expand Up @@ -229,7 +229,7 @@ func makeStorageExtenion(t *testing.T, config *Config) component.Component {

func startStorageExtension(t *testing.T, memstoreName string) component.Component {
config := &Config{
Memory: map[string]memoryCfg.Configuration{
Memory: map[string]memory.Configuration{
memstoreName: {MaxTraces: 10000},
},
}
Expand Down
25 changes: 0 additions & 25 deletions pkg/memory/config/empty_test.go
View file
Open in desktop

This file was deleted.

2 changes: 2 additions & 0 deletions plugin/storage/grpc/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ type Factory struct {
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

Expand Down
2 changes: 1 addition & 1 deletion pkg/memory/config/config.go → plugin/storage/memory/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package config
package memory

// Configuration describes the options to customize the storage behavior
type Configuration struct {
Expand Down
3 changes: 1 addition & 2 deletions plugin/storage/memory/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/jaegertracing/jaeger/internal/safeexpvar"
"github.com/jaegertracing/jaeger/pkg/distributedlock"
"github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/plugin"
"github.com/jaegertracing/jaeger/storage"
Expand Down Expand Up @@ -54,7 +53,7 @@ func NewFactory() *Factory {

// NewFactoryWithConfig is used from jaeger(v2).
func NewFactoryWithConfig(
cfg config.Configuration,
cfg Configuration,
metricsFactory metrics.Factory,
logger *zap.Logger,
) *Factory {
Expand Down
3 changes: 1 addition & 2 deletions plugin/storage/memory/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/config"
memCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/metrics"
"github.com/jaegertracing/jaeger/storage"
)
Expand Down Expand Up @@ -61,7 +60,7 @@ func TestWithConfiguration(t *testing.T) {
}

func TestNewFactoryWithConfig(t *testing.T) {
cfg := memCfg.Configuration{
cfg := Configuration{
MaxTraces: 42,
}
f := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop())
Expand Down
13 changes: 6 additions & 7 deletions plugin/storage/memory/memory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/model/adjuster"
"github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/storage/spanstore"
)
Expand All @@ -36,7 +35,7 @@ type Store struct {
sync.RWMutex
// Each tenant gets a copy of default config.
// In the future this can be extended to contain per-tenant configuration.
defaultConfig config.Configuration
defaultConfig Configuration
perTenant map[string]*Tenant
}

Expand All @@ -48,24 +47,24 @@ type Tenant struct {
services map[string]struct{}
operations map[string]map[spanstore.Operation]struct{}
deduper adjuster.Adjuster
config config.Configuration
config Configuration
index int
}

// NewStore creates an unbounded in-memory store
func NewStore() *Store {
return WithConfiguration(config.Configuration{MaxTraces: 0})
return WithConfiguration(Configuration{MaxTraces: 0})
}

// WithConfiguration creates a new in memory storage based on the given configuration
func WithConfiguration(configuration config.Configuration) *Store {
func WithConfiguration(cfg Configuration) *Store {
return &Store{
defaultConfig: configuration,
defaultConfig: cfg,
perTenant: make(map[string]*Tenant),
}
}

func newTenant(cfg config.Configuration) *Tenant {
func newTenant(cfg Configuration) *Tenant {
return &Tenant{
ids: make([]*model.TraceID, cfg.MaxTraces),
traces: map[model.TraceID]*model.Trace{},
Expand Down
3 changes: 1 addition & 2 deletions plugin/storage/memory/memory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/stretchr/testify/require"

"github.com/jaegertracing/jaeger/model"
"github.com/jaegertracing/jaeger/pkg/memory/config"
"github.com/jaegertracing/jaeger/pkg/tenancy"
"github.com/jaegertracing/jaeger/storage/spanstore"
)
Expand Down Expand Up @@ -170,7 +169,7 @@ func TestStoreWriteSpan(t *testing.T) {

func TestStoreWithLimit(t *testing.T) {
maxTraces := 100
store := WithConfiguration(config.Configuration{MaxTraces: maxTraces})
store := WithConfiguration(Configuration{MaxTraces: maxTraces})

for i := 0; i < maxTraces*2; i++ {
id := model.NewTraceID(1, uint64(i))
Expand Down
4 changes: 1 addition & 3 deletions plugin/storage/memory/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ import (
"flag"

"github.com/spf13/viper"

"github.com/jaegertracing/jaeger/pkg/memory/config"
)

const limit = "memory.max-traces"

// Options stores the configuration entries for this storage
type Options struct {
Configuration config.Configuration `mapstructure:",squash"`
Configuration Configuration `mapstructure:",squash"`
}

// AddFlags from this storage to the CLI
Expand Down
Loading