Skip to content

Commit 9ba3238

Browse files
authored
Simplify configs organization (jaegertracing#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 <[email protected]>
1 parent 722755e commit 9ba3238

File tree

12 files changed

+33
-68
lines changed

12 files changed

+33
-68
lines changed

cmd/jaeger/internal/exporters/storageexporter/exporter_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import (
3333

3434
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage"
3535
"github.com/jaegertracing/jaeger/model"
36-
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
36+
"github.com/jaegertracing/jaeger/plugin/storage/memory"
3737
"github.com/jaegertracing/jaeger/storage"
3838
factoryMocks "github.com/jaegertracing/jaeger/storage/mocks"
3939
)
@@ -166,7 +166,7 @@ func makeStorageExtension(t *testing.T, memstoreName string) component.Host {
166166
TracerProvider: nooptrace.NewTracerProvider(),
167167
},
168168
},
169-
&jaegerstorage.Config{Memory: map[string]memoryCfg.Configuration{
169+
&jaegerstorage.Config{Memory: map[string]memory.Configuration{
170170
memstoreName: {MaxTraces: 10000},
171171
}})
172172
require.NoError(t, err)

cmd/jaeger/internal/extension/jaegerstorage/config.go

+9-14
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,25 @@ import (
88
"reflect"
99

1010
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
11-
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
12-
badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger"
11+
"github.com/jaegertracing/jaeger/plugin/storage/badger"
1312
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
14-
grpcCfg "github.com/jaegertracing/jaeger/plugin/storage/grpc"
13+
"github.com/jaegertracing/jaeger/plugin/storage/grpc"
14+
"github.com/jaegertracing/jaeger/plugin/storage/memory"
1515
)
1616

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

30-
type MemoryStorage struct {
31-
Name string `mapstructure:"name"`
32-
memoryCfg.Configuration
33-
}
34-
3530
func (cfg *Config) Validate() error {
3631
emptyCfg := createDefaultConfig().(*Config)
3732
if reflect.DeepEqual(*cfg, *emptyCfg) {

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515

1616
"github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage/factoryadapter"
1717
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
18-
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
1918
"github.com/jaegertracing/jaeger/pkg/metrics"
2019
"github.com/jaegertracing/jaeger/plugin/storage/badger"
2120
"github.com/jaegertracing/jaeger/plugin/storage/cassandra"
@@ -107,13 +106,13 @@ func (s *starter[Config, Factory]) build(_ context.Context, _ component.Host) er
107106
}
108107

109108
func (s *storageExt) Start(ctx context.Context, host component.Host) error {
110-
memStarter := &starter[memoryCfg.Configuration, *memory.Factory]{
109+
memStarter := &starter[memory.Configuration, *memory.Factory]{
111110
ext: s,
112111
storageKind: "memory",
113112
cfg: s.config.Memory,
114113
// memory factory does not return an error, so need to wrap it
115114
builder: func(
116-
cfg memoryCfg.Configuration,
115+
cfg memory.Configuration,
117116
metricsFactory metrics.Factory,
118117
logger *zap.Logger,
119118
) (*memory.Factory, error) {

cmd/jaeger/internal/extension/jaegerstorage/extension_test.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ import (
1919
"go.uber.org/zap"
2020

2121
esCfg "github.com/jaegertracing/jaeger/pkg/es/config"
22-
memoryCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
2322
"github.com/jaegertracing/jaeger/pkg/metrics"
2423
"github.com/jaegertracing/jaeger/pkg/testutils"
25-
badgerCfg "github.com/jaegertracing/jaeger/plugin/storage/badger"
24+
"github.com/jaegertracing/jaeger/plugin/storage/badger"
25+
"github.com/jaegertracing/jaeger/plugin/storage/memory"
2626
"github.com/jaegertracing/jaeger/storage"
2727
"github.com/jaegertracing/jaeger/storage/dependencystore"
2828
"github.com/jaegertracing/jaeger/storage/spanstore"
@@ -80,10 +80,10 @@ func TestStorageExtensionConfigError(t *testing.T) {
8080

8181
func TestStorageExtensionNameConflict(t *testing.T) {
8282
storageExtension := makeStorageExtenion(t, &Config{
83-
Memory: map[string]memoryCfg.Configuration{
83+
Memory: map[string]memory.Configuration{
8484
"foo": {MaxTraces: 10000},
8585
},
86-
Badger: map[string]badgerCfg.NamespaceConfig{
86+
Badger: map[string]badger.NamespaceConfig{
8787
"foo": {},
8888
},
8989
})
@@ -134,7 +134,7 @@ func TestStorageExtension(t *testing.T) {
134134

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

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

230230
func startStorageExtension(t *testing.T, memstoreName string) component.Component {
231231
config := &Config{
232-
Memory: map[string]memoryCfg.Configuration{
232+
Memory: map[string]memory.Configuration{
233233
memstoreName: {MaxTraces: 10000},
234234
},
235235
}

pkg/memory/config/empty_test.go

-25
This file was deleted.

plugin/storage/grpc/factory.go

+2
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ type Factory struct {
4545
logger *zap.Logger
4646
tracerProvider trace.TracerProvider
4747

48+
// configV1 is used for backward compatibility. it will be removed in v2.
49+
// In the main initialization logic, only configV2 is used.
4850
configV1 Configuration
4951
configV2 *ConfigV2
5052

pkg/memory/config/config.go plugin/storage/memory/config.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package config
15+
package memory
1616

1717
// Configuration describes the options to customize the storage behavior
1818
type Configuration struct {

plugin/storage/memory/factory.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323

2424
"github.com/jaegertracing/jaeger/internal/safeexpvar"
2525
"github.com/jaegertracing/jaeger/pkg/distributedlock"
26-
"github.com/jaegertracing/jaeger/pkg/memory/config"
2726
"github.com/jaegertracing/jaeger/pkg/metrics"
2827
"github.com/jaegertracing/jaeger/plugin"
2928
"github.com/jaegertracing/jaeger/storage"
@@ -54,7 +53,7 @@ func NewFactory() *Factory {
5453

5554
// NewFactoryWithConfig is used from jaeger(v2).
5655
func NewFactoryWithConfig(
57-
cfg config.Configuration,
56+
cfg Configuration,
5857
metricsFactory metrics.Factory,
5958
logger *zap.Logger,
6059
) *Factory {

plugin/storage/memory/factory_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"go.uber.org/zap"
2525

2626
"github.com/jaegertracing/jaeger/pkg/config"
27-
memCfg "github.com/jaegertracing/jaeger/pkg/memory/config"
2827
"github.com/jaegertracing/jaeger/pkg/metrics"
2928
"github.com/jaegertracing/jaeger/storage"
3029
)
@@ -61,7 +60,7 @@ func TestWithConfiguration(t *testing.T) {
6160
}
6261

6362
func TestNewFactoryWithConfig(t *testing.T) {
64-
cfg := memCfg.Configuration{
63+
cfg := Configuration{
6564
MaxTraces: 42,
6665
}
6766
f := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop())

plugin/storage/memory/memory.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626

2727
"github.com/jaegertracing/jaeger/model"
2828
"github.com/jaegertracing/jaeger/model/adjuster"
29-
"github.com/jaegertracing/jaeger/pkg/memory/config"
3029
"github.com/jaegertracing/jaeger/pkg/tenancy"
3130
"github.com/jaegertracing/jaeger/storage/spanstore"
3231
)
@@ -36,7 +35,7 @@ type Store struct {
3635
sync.RWMutex
3736
// Each tenant gets a copy of default config.
3837
// In the future this can be extended to contain per-tenant configuration.
39-
defaultConfig config.Configuration
38+
defaultConfig Configuration
4039
perTenant map[string]*Tenant
4140
}
4241

@@ -48,24 +47,24 @@ type Tenant struct {
4847
services map[string]struct{}
4948
operations map[string]map[spanstore.Operation]struct{}
5049
deduper adjuster.Adjuster
51-
config config.Configuration
50+
config Configuration
5251
index int
5352
}
5453

5554
// NewStore creates an unbounded in-memory store
5655
func NewStore() *Store {
57-
return WithConfiguration(config.Configuration{MaxTraces: 0})
56+
return WithConfiguration(Configuration{MaxTraces: 0})
5857
}
5958

6059
// WithConfiguration creates a new in memory storage based on the given configuration
61-
func WithConfiguration(configuration config.Configuration) *Store {
60+
func WithConfiguration(cfg Configuration) *Store {
6261
return &Store{
63-
defaultConfig: configuration,
62+
defaultConfig: cfg,
6463
perTenant: make(map[string]*Tenant),
6564
}
6665
}
6766

68-
func newTenant(cfg config.Configuration) *Tenant {
67+
func newTenant(cfg Configuration) *Tenant {
6968
return &Tenant{
7069
ids: make([]*model.TraceID, cfg.MaxTraces),
7170
traces: map[model.TraceID]*model.Trace{},

plugin/storage/memory/memory_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"github.com/stretchr/testify/require"
2525

2626
"github.com/jaegertracing/jaeger/model"
27-
"github.com/jaegertracing/jaeger/pkg/memory/config"
2827
"github.com/jaegertracing/jaeger/pkg/tenancy"
2928
"github.com/jaegertracing/jaeger/storage/spanstore"
3029
)
@@ -170,7 +169,7 @@ func TestStoreWriteSpan(t *testing.T) {
170169

171170
func TestStoreWithLimit(t *testing.T) {
172171
maxTraces := 100
173-
store := WithConfiguration(config.Configuration{MaxTraces: maxTraces})
172+
store := WithConfiguration(Configuration{MaxTraces: maxTraces})
174173

175174
for i := 0; i < maxTraces*2; i++ {
176175
id := model.NewTraceID(1, uint64(i))

plugin/storage/memory/options.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,13 @@ import (
1818
"flag"
1919

2020
"github.com/spf13/viper"
21-
22-
"github.com/jaegertracing/jaeger/pkg/memory/config"
2321
)
2422

2523
const limit = "memory.max-traces"
2624

2725
// Options stores the configuration entries for this storage
2826
type Options struct {
29-
Configuration config.Configuration `mapstructure:",squash"`
27+
Configuration Configuration `mapstructure:",squash"`
3028
}
3129

3230
// AddFlags from this storage to the CLI

0 commit comments

Comments
 (0)