Simplify configs organization (#5690)

## Description of the changes - Move memstore config from pkg/memory to plugin/storage/memory, as the separation was unnecessary - Simplify imports in the extension ## How was this change tested? - CI Signed-off-by: Yuri Shkuro <github@ysh.us>
jaegertracing · Jun 30, 2024 · 9ba3238 · 9ba3238
1 parent 722755e
commit 9ba3238
Show file tree

Hide file tree

Showing 12 changed files with 33 additions and 68 deletions.
diff --git a/cmd/jaeger/internal/exporters/storageexporter/exporter_test.go b/cmd/jaeger/internal/exporters/storageexporter/exporter_test.go
@@ -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"
 )
@@ -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)

diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config.go b/cmd/jaeger/internal/extension/jaegerstorage/config.go
@@ -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) {

diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go
@@ -15,7 +15,6 @@ import (
 
 	"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"
@@ -107,13 +106,13 @@ func (s *starter[Config, Factory]) build(_ context.Context, _ component.Host) er
 }
 
 func (s *storageExt) Start(ctx context.Context, host component.Host) error {
-	memStarter := &starter[memoryCfg.Configuration, *memory.Factory]{
+	memStarter := &starter[memory.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,
+			cfg memory.Configuration,
 			metricsFactory metrics.Factory,
 			logger *zap.Logger,
 		) (*memory.Factory, error) {

diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go b/cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
@@ -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"
@@ -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": {},
 		},
 	})
@@ -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,
@@ -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",
@@ -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},
 		},
 	}

diff --git a/pkg/memory/config/empty_test.go b/pkg/memory/config/empty_test.go
diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go
@@ -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
 

diff --git a/pkg/memory/config/config.go → plugin/storage/memory/config.go b/pkg/memory/config/config.go → plugin/storage/memory/config.go
@@ -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 {

diff --git a/plugin/storage/memory/factory.go b/plugin/storage/memory/factory.go
@@ -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"
@@ -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 {

diff --git a/plugin/storage/memory/factory_test.go b/plugin/storage/memory/factory_test.go
@@ -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"
 )
@@ -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())

diff --git a/plugin/storage/memory/memory.go b/plugin/storage/memory/memory.go
@@ -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"
 )
@@ -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
 }
 
@@ -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{},

diff --git a/plugin/storage/memory/memory_test.go b/plugin/storage/memory/memory_test.go
@@ -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"
 )
@@ -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))

diff --git a/plugin/storage/memory/options.go b/plugin/storage/memory/options.go
@@ -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