DRY code, rename public methods to be more consistent

Signed-off-by: Yuri Shkuro <[email protected]>
jaegertracing · yurishkuro · Jul 24, 2024 · Jun 26, 2024 · Jun 27, 2024 · Jun 27, 2024
commit 9d2f9b21b8b374f74135ba1bc424e17dca745cff
@@ -44,6 +44,8 @@ type mockStorageExt struct {
 	metricsFactory *factoryMocks.MetricsFactory
 }
 
+var _ jaegerstorage.Extension = (*mockStorageExt)(nil)
+
 func (*mockStorageExt) Start(context.Context, component.Host) error {
 	panic("not implemented")
 }
@@ -52,14 +54,14 @@ func (*mockStorageExt) Shutdown(context.Context) error {
 	panic("not implemented")
 }
 
-func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) {
+func (m *mockStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
 	if m.name == name {
 		return m.factory, true
 	}
 	return nil, false
 }
 
-func (m *mockStorageExt) MetricsFactory(name string) (storage.MetricsFactory, bool) {
+func (m *mockStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
 	if m.name == name {
 		return m.metricsFactory, true
 	}

@@ -87,14 +87,14 @@ type fakeStorageExt struct{}
 
 var _ jaegerstorage.Extension = (*fakeStorageExt)(nil)
 
-func (fakeStorageExt) Factory(name string) (storage.Factory, bool) {
+func (fakeStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
 	if name == "need-factory-error" {
 		return nil, false
 	}
 	return fakeFactory{name: name}, true
 }
 
-func (fakeStorageExt) MetricsFactory(name string) (storage.MetricsFactory, bool) {
+func (fakeStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
 	if name == "need-factory-error" {
 		return nil, false
 	}

@@ -28,8 +28,8 @@
 
 type Extension interface {
 	extension.Extension
-	Factory(name string) (storage.Factory, bool)
-	MetricsFactory(name string) (storage.MetricsFactory, bool)
+	TraceStorageFactory(name string) (storage.Factory, bool)
+	MetricStorageFactory(name string) (storage.MetricsFactory, bool)
 }
 
 type storageExt struct {
@@ -41,20 +41,11 @@
 
 // GetStorageFactory locates the extension in Host and retrieves a storage factory from it with the given name.
 func GetStorageFactory(name string, host component.Host) (storage.Factory, error) {
-	var comp component.Component
-	for id, ext := range host.GetExtensions() {
-		if id.Type() == componentType {
-			comp = ext
-			break
-		}
-	}
-	if comp == nil {
-		return nil, fmt.Errorf(
-			"cannot find extension '%s' (make sure it's defined earlier in the config)",
-			componentType,
-		)
+	ext, err := findExtension(host)
+	if err != nil {
+		return nil, err
 	}
-	f, ok := comp.(Extension).Factory(name)
+	f, ok := ext.TraceStorageFactory(name)
 	if !ok {
 		return nil, fmt.Errorf(
 			"cannot find definition of storage '%s' in the configuration for extension '%s'",
@@ -65,28 +56,19 @@
 }
 
 // GetMetricsFactory locates the extension in Host and retrieves a metrics factory from it with the given name.
 func GetMetricsFactory(name string, host component.Host) (storage.MetricsFactory, error) {
-	var comp component.Component
-	for id, ext := range host.GetExtensions() {
-		if id.Type() == componentType {
-			comp = ext
-			break
-		}
-	}
-	if comp == nil {
-		return nil, fmt.Errorf(
-			"cannot find extension '%s' (make sure it's defined earlier in the config)",
-			componentType,
-		)
+	ext, err := findExtension(host)
+	if err != nil {
+		return nil, err
 	}
-	mf, ok := comp.(Extension).MetricsFactory(name)
+	mf, ok := ext.MetricStorageFactory(name)
 	if !ok {
 		return nil, fmt.Errorf(
 			"cannot find metric storage '%s' declared by '%s' extension",
 			name, componentType,
 		)
 	}
 	return mf, nil
 }

 func GetStorageFactoryV2(name string, host component.Host) (spanstore.Factory, error) {
@@ -98,6 +80,28 @@
 	return factoryadapter.NewFactory(f), nil
 }
 
+func findExtension(host component.Host) (Extension, error) {
+	var id component.ID
+	var comp component.Component
+	for i, ext := range host.GetExtensions() {
+		if i.Type() == componentType {
+			id, comp = i, ext
+			break
+		}
+	}
+	if comp == nil {
+		return nil, fmt.Errorf(
+			"cannot find extension '%s' (make sure it's defined earlier in the config)",
+			componentType,
+		)
+	}
+	ext, ok := comp.(Extension)
+	if !ok {
+		return nil, fmt.Errorf("extension '%s' is not of expected type '%s'", id, componentType)
+	}
+	return ext, nil
+}
+
 func newStorageExt(config *Config, telset component.TelemetrySettings) *storageExt {
 	return &storageExt{
 		config:           config,
@@ -135,16 +139,16 @@
 	}

 	for metricStorageName, cfg := range s.config.MetricBackends {
 		s.telset.Logger.Sugar().Infof("Initializing metrics storage '%s'", metricStorageName)
 		var metricsFactory storage.MetricsFactory
 		var err error
 		if cfg.Prometheus != nil {
 			metricsFactory, err = prometheus.NewFactoryWithConfig(*cfg.Prometheus, s.telset.Logger)
 		}
 		if err != nil {
 			return fmt.Errorf("failed to initialize metrics storage '%s': %w", metricStorageName, err)
 		}
 		s.metricsFactories[metricStorageName] = metricsFactory
 	}

 	return nil
@@ -163,12 +167,12 @@
 	return errors.Join(errs...)
 }
 
-func (s *storageExt) Factory(name string) (storage.Factory, bool) {
+func (s *storageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
 	f, ok := s.factories[name]
 	return f, ok
 }
 
-func (s *storageExt) MetricsFactory(name string) (storage.MetricsFactory, bool) {
+func (s *storageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
 	mf, ok := s.metricsFactories[name]
 	return mf, ok
 }
@@ -50,14 +50,14 @@ func (*mockStorageExt) Shutdown(context.Context) error {
 	panic("not implemented")
 }
 
-func (m *mockStorageExt) Factory(name string) (storage.Factory, bool) {
+func (m *mockStorageExt) TraceStorageFactory(name string) (storage.Factory, bool) {
 	if m.name == name {
 		return m.factory, true
 	}
 	return nil, false
 }
 
-func (m *mockStorageExt) MetricsFactory(name string) (storage.MetricsFactory, bool) {
+func (m *mockStorageExt) MetricStorageFactory(name string) (storage.MetricsFactory, bool) {
 	if m.name == name {
 		return m.metricsFactory, true
 	}

diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go

 	mqs, err := s.createMetricStorage(host)
 	if err != nil {
 		return err
 	}

 	tm := tenancy.NewManager(&s.config.Tenancy)
 		return disabled.NewMetricsReader()
 	}

 	mf, err := jaegerstorage.GetMetricsFactory(s.config.MetricStorage, host)
 	if err != nil {
 		return nil, fmt.Errorf("cannot find metrics storage factory: %w", err)
 	}

 	metricsReader, err := mf.CreateMetricsReader()
 	if errors.Is(err, storage.ErrMetricStorageNotConfigured) || errors.Is(err, storage.ErrMetricStorageNotSupported) {
 		s.telset.Logger.Info("Metric storage not created", zap.Error(err))
 		return disabled.NewMetricsReader()
 	}
 	return metricsReader, err
 }

 func (s *server) makeQueryOptions() *queryApp.QueryOptions {
diff --git a/cmd/jaeger/internal/extension/jaegerstorage/config.go b/cmd/jaeger/internal/extension/jaegerstorage/config.go
 	return nil
 }

 func (cfg *MetricBackends) Unmarshal(conf *confmap.Conf) error {
 	// apply defaults
 	if conf.IsSet("prometheus") {
 		v := prometheus.DefaultConfig()
 		cfg.Prometheus = &v
 	}
 	return conf.Unmarshal(cfg)
 }
diff --git a/pkg/prometheus/config/config.go b/pkg/prometheus/config/config.go
 	NormalizeDuration bool   `mapstructure:"normalize_duration"`
 }

 func (c *Configuration) Validate() error {
 	_, err := govalidator.ValidateStruct(c)
 	return err
 }
diff --git a/plugin/metrics/prometheus/factory.go b/plugin/metrics/prometheus/factory.go
 func NewFactoryWithConfig(
 	cfg config.Configuration,
 	logger *zap.Logger,
 ) (*Factory, error) {
 	if err := cfg.Validate(); err != nil {
 		return nil, err
 	}

 	f := NewFactory()
 	f.options = &Options{
 		Configuration: cfg,
 	}
 	err := f.Initialize(logger)
 	if err != nil {
 		return nil, err
 	}
 	return f, nil
 }

	mqs, err := s.createMetricStorage(host)
	if err != nil {
	return err
Check warning on line 76 in cmd/jaeger/internal/extension/jaegerquery/server.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerquery/server.go#L76 `Added line #L76 was not covered by tests`
	}

	tm := tenancy.NewManager(&s.config.Tenancy)
	return disabled.NewMetricsReader()
	}

	mf, err := jaegerstorage.GetMetricsFactory(s.config.MetricStorage, host)
	if err != nil {
	return nil, fmt.Errorf("cannot find metrics storage factory: %w", err)
Check warning on line 141 in cmd/jaeger/internal/extension/jaegerquery/server.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerquery/server.go#L139-L141 `Added lines #L139 - L141 were not covered by tests`
	}

	metricsReader, err := mf.CreateMetricsReader()
	if errors.Is(err, storage.ErrMetricStorageNotConfigured) \|\| errors.Is(err, storage.ErrMetricStorageNotSupported) {
	s.telset.Logger.Info("Metric storage not created", zap.Error(err))
	return disabled.NewMetricsReader()
Check warning on line 147 in cmd/jaeger/internal/extension/jaegerquery/server.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerquery/server.go#L144-L147 `Added lines #L144 - L147 were not covered by tests`
	}
	return metricsReader, err
Check warning on line 149 in cmd/jaeger/internal/extension/jaegerquery/server.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerquery/server.go#L149 `Added line #L149 was not covered by tests`
	}

	func (s server) makeQueryOptions() queryApp.QueryOptions {
	return nil
	}

	func (cfg MetricBackends) Unmarshal(conf confmap.Conf) error {
Check warning on line 110 in cmd/jaeger/internal/extension/jaegerstorage/config.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerstorage/config.go#L110 `Added line #L110 was not covered by tests`
	// apply defaults
	if conf.IsSet("prometheus") {
	v := prometheus.DefaultConfig()
	cfg.Prometheus = &v
Check warning on line 114 in cmd/jaeger/internal/extension/jaegerstorage/config.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerstorage/config.go#L112-L114 `Added lines #L112 - L114 were not covered by tests`
	}
	return conf.Unmarshal(cfg)
Check warning on line 116 in cmd/jaeger/internal/extension/jaegerstorage/config.go Codecov / codecov/patch cmd/jaeger/internal/extension/jaegerstorage/config.go#L116 `Added line #L116 was not covered by tests`
	}
	NormalizeDuration bool `mapstructure:"normalize_duration"`
	}

	func (c *Configuration) Validate() error {
	_, err := govalidator.ValidateStruct(c)
	return err
Check warning on line 42 in pkg/prometheus/config/config.go Codecov / codecov/patch pkg/prometheus/config/config.go#L40-L42 `Added lines #L40 - L42 were not covered by tests`
	}
	func NewFactoryWithConfig(
	cfg config.Configuration,
	logger *zap.Logger,
	) (*Factory, error) {
	if err := cfg.Validate(); err != nil {
	return nil, err
Check warning on line 76 in plugin/metrics/prometheus/factory.go Codecov / codecov/patch plugin/metrics/prometheus/factory.go#L74-L76 `Added lines #L74 - L76 were not covered by tests`
	}

	f := NewFactory()
	f.options = &Options{
	Configuration: cfg,
Check warning on line 81 in plugin/metrics/prometheus/factory.go Codecov / codecov/patch plugin/metrics/prometheus/factory.go#L79-L81 `Added lines #L79 - L81 were not covered by tests`
	}
	err := f.Initialize(logger)
	if err != nil {
	return nil, err
Check warning on line 85 in plugin/metrics/prometheus/factory.go Codecov / codecov/patch plugin/metrics/prometheus/factory.go#L83-L85 `Added lines #L83 - L85 were not covered by tests`
	}
	return f, nil
Check warning on line 87 in plugin/metrics/prometheus/factory.go Codecov / codecov/patch plugin/metrics/prometheus/factory.go#L87 `Added line #L87 was not covered by tests`
	}