From a5cfa582618601a4e5659bad39fb986f50f93edb Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 09:41:22 -0500 Subject: [PATCH 1/8] Add MetricsFactory To Initialize Function In Interface Signed-off-by: Mahad Zaryab --- storage/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/factory.go b/storage/factory.go index ca4afe34259..4b73670b332 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -86,7 +86,7 @@ type ArchiveFactory interface { type MetricsFactory interface { // Initialize performs internal initialization of the factory, such as opening connections to the backend store. // It is called after all configuration of the factory itself has been done. - Initialize(logger *zap.Logger) error + Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error // CreateMetricsReader creates a metricsstore.Reader. CreateMetricsReader() (metricsstore.Reader, error) From b7c9f59422fdb44a6c058b6c56726ccae121d973 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 09:42:32 -0500 Subject: [PATCH 2/8] Generate New Mocks Signed-off-by: Mahad Zaryab --- storage/mocks/MetricsFactory.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/storage/mocks/MetricsFactory.go b/storage/mocks/MetricsFactory.go index 4d5d5823450..368c070ded0 100644 --- a/storage/mocks/MetricsFactory.go +++ b/storage/mocks/MetricsFactory.go @@ -8,7 +8,9 @@ package mocks import ( + metrics "github.com/jaegertracing/jaeger/pkg/metrics" metricsstore "github.com/jaegertracing/jaeger/storage/metricsstore" + mock "github.com/stretchr/testify/mock" zap "go.uber.org/zap" @@ -49,17 +51,17 @@ func (_m *MetricsFactory) CreateMetricsReader() (metricsstore.Reader, error) { return r0, r1 } -// Initialize provides a mock function with given fields: logger -func (_m *MetricsFactory) Initialize(logger *zap.Logger) error { - ret := _m.Called(logger) +// Initialize provides a mock function with given fields: metricsFactory, logger +func (_m *MetricsFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { + ret := _m.Called(metricsFactory, logger) if len(ret) == 0 { panic("no return value specified for Initialize") } var r0 error - if rf, ok := ret.Get(0).(func(*zap.Logger) error); ok { - r0 = rf(logger) + if rf, ok := ret.Get(0).(func(metrics.Factory, *zap.Logger) error); ok { + r0 = rf(metricsFactory, logger) } else { r0 = ret.Error(0) } From 449f08a855ce0050659f36b000f8859c913b837f Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 09:43:30 -0500 Subject: [PATCH 3/8] Implement New Interface In Disabled Factory Signed-off-by: Mahad Zaryab --- plugin/metrics/disabled/factory.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin/metrics/disabled/factory.go b/plugin/metrics/disabled/factory.go index fa7aa9f529d..0b407d674d5 100644 --- a/plugin/metrics/disabled/factory.go +++ b/plugin/metrics/disabled/factory.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/viper" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/storage/metricsstore" ) @@ -30,7 +31,7 @@ func (*Factory) AddFlags(_ *flag.FlagSet) {} func (*Factory) InitFromViper(_ *viper.Viper, _ *zap.Logger) {} // Initialize implements storage.MetricsFactory. -func (*Factory) Initialize(_ *zap.Logger) error { +func (*Factory) Initialize(_ metrics.Factory, _ *zap.Logger) error { return nil } From e7eb7cd2968e0395a206e7d51b1be304ff848da8 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 10:35:45 -0500 Subject: [PATCH 4/8] Update Callsite To Pass In Metrics Factory Signed-off-by: Mahad Zaryab --- cmd/all-in-one/main.go | 8 +++---- .../internal/extension/jaegerquery/server.go | 7 +----- .../extension/jaegerquery/server_test.go | 2 +- .../extension/jaegerstorage/extension.go | 5 ++++- cmd/query/main.go | 8 +++---- plugin/metrics/disabled/factory_test.go | 5 +++-- plugin/metrics/factory.go | 13 ++++++++--- plugin/metrics/factory_test.go | 3 ++- plugin/metrics/prometheus/factory.go | 22 +++++++++++++------ plugin/metrics/prometheus/factory_test.go | 7 +++--- 10 files changed, 46 insertions(+), 34 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index bc2c0e48162..056467f0c37 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -39,7 +39,6 @@ import ( "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -118,7 +117,7 @@ by default uses only in-memory database.`, logger.Fatal("Failed to create dependency reader", zap.Error(err)) } - metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger, queryMetricsFactory) + metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger, baseFactory) if err != nil { logger.Fatal("Failed to create metrics reader", zap.Error(err)) } @@ -241,7 +240,7 @@ func createMetricsQueryService( logger *zap.Logger, metricsReaderMetricsFactory metrics.Factory, ) (querysvc.MetricsQueryService, error) { - if err := metricsReaderFactory.Initialize(logger); err != nil { + if err := metricsReaderFactory.Initialize(metricsReaderMetricsFactory, logger); err != nil { return nil, fmt.Errorf("failed to init metrics reader factory: %w", err) } @@ -252,6 +251,5 @@ func createMetricsQueryService( return nil, fmt.Errorf("failed to create metrics reader: %w", err) } - // Decorate the metrics reader with metrics instrumentation. - return metricstoremetrics.NewReaderDecorator(reader, metricsReaderMetricsFactory), nil + return reader, nil } diff --git a/cmd/jaeger/internal/extension/jaegerquery/server.go b/cmd/jaeger/internal/extension/jaegerquery/server.go index 9def6f5fe99..dd81c42f6f1 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server.go @@ -15,14 +15,12 @@ import ( "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/jaegerstorage" queryApp "github.com/jaegertracing/jaeger/cmd/query/app" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" - "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" "github.com/jaegertracing/jaeger/pkg/jtracer" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/pkg/tenancy" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/storage/metricsstore" - "github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics" ) var ( @@ -156,10 +154,7 @@ func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, e return nil, fmt.Errorf("cannot create metrics reader %w", err) } - // Decorate the metrics reader with metrics instrumentation. - mf := otelmetrics.NewFactory(s.telset.MeterProvider) - mf = mf.Namespace(metrics.NSOptions{Name: "jaeger_metricstore"}) - return metricstoremetrics.NewReaderDecorator(metricsReader, mf), nil + return metricsReader, nil } func (s *server) Shutdown(ctx context.Context) error { diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index 54b3733f24c..ecdabc623f4 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -73,7 +73,7 @@ type fakeMetricsFactory struct { } // Initialize implements storage.MetricsFactory. -func (fmf fakeMetricsFactory) Initialize(*zap.Logger) error { +func (fmf fakeMetricsFactory) Initialize(metrics.Factory, *zap.Logger) error { if fmf.name == "need-initialize-error" { return errors.New("test-error") } diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go index 7b3d5f454d9..5a9f9c92aed 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension.go @@ -177,7 +177,10 @@ func (s *storageExt) Start(_ context.Context, host component.Host) error { var metricsFactory storage.MetricsFactory var err error if cfg.Prometheus != nil { - metricsFactory, err = prometheus.NewFactoryWithConfig(*cfg.Prometheus, s.telset.Logger) + metricsFactory, err = prometheus.NewFactoryWithConfig( + *cfg.Prometheus, + getMetricsFactory(metricStorageName, "prometheus"), + s.telset.Logger) } if err != nil { return fmt.Errorf("failed to initialize metrics storage '%s': %w", metricStorageName, err) diff --git a/cmd/query/main.go b/cmd/query/main.go index 6c3e6261f96..df00f37e272 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -32,7 +32,6 @@ import ( metricsPlugin "github.com/jaegertracing/jaeger/plugin/metrics" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" - "github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics" ) func main() { @@ -99,7 +98,7 @@ func main() { logger.Fatal("Failed to create dependency reader", zap.Error(err)) } - metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger, metricsFactory) + metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger, baseFactory) if err != nil { logger.Fatal("Failed to create metrics query service", zap.Error(err)) } @@ -169,7 +168,7 @@ func createMetricsQueryService( logger *zap.Logger, metricsReaderMetricsFactory metrics.Factory, ) (querysvc.MetricsQueryService, error) { - if err := metricsReaderFactory.Initialize(logger); err != nil { + if err := metricsReaderFactory.Initialize(metricsReaderMetricsFactory, logger); err != nil { return nil, fmt.Errorf("failed to init metrics reader factory: %w", err) } @@ -180,6 +179,5 @@ func createMetricsQueryService( return nil, fmt.Errorf("failed to create metrics reader: %w", err) } - // Decorate the metrics reader with metrics instrumentation. - return metricstoremetrics.NewReaderDecorator(reader, metricsReaderMetricsFactory), nil + return reader, nil } diff --git a/plugin/metrics/disabled/factory_test.go b/plugin/metrics/disabled/factory_test.go index fac3e142fc4..5caac5d9fbf 100644 --- a/plugin/metrics/disabled/factory_test.go +++ b/plugin/metrics/disabled/factory_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/storage" ) @@ -17,9 +18,9 @@ var _ storage.MetricsFactory = new(Factory) func TestPrometheusFactory(t *testing.T) { f := NewFactory() - require.NoError(t, f.Initialize(zap.NewNop())) + require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) - err := f.Initialize(nil) + err := f.Initialize(metrics.NullFactory, nil) require.NoError(t, err) f.AddFlags(nil) diff --git a/plugin/metrics/factory.go b/plugin/metrics/factory.go index fd663838a03..2cf96bc2de0 100644 --- a/plugin/metrics/factory.go +++ b/plugin/metrics/factory.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/viper" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/plugin/metrics/prometheus" @@ -63,9 +64,15 @@ func (*Factory) getFactoryOfType(factoryType string) (storage.MetricsFactory, er } // Initialize implements storage.MetricsFactory. -func (f *Factory) Initialize(logger *zap.Logger) error { - for _, factory := range f.factories { - factory.Initialize(logger) +func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { + for kind, factory := range f.factories { + mf := metricsFactory.Namespace(metrics.NSOptions{ + Name: "storage", + Tags: map[string]string{ + "kind": kind, + }, + }) + factory.Initialize(mf, logger) } return nil } diff --git a/plugin/metrics/factory_test.go b/plugin/metrics/factory_test.go index d7f6229b6eb..2e4e306e076 100644 --- a/plugin/metrics/factory_test.go +++ b/plugin/metrics/factory_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/mocks" @@ -53,7 +54,7 @@ func TestCreateMetricsReader(t *testing.T) { require.NoError(t, err) require.NotNil(t, f) - require.NoError(t, f.Initialize(zap.NewNop())) + require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) reader, err := f.CreateMetricsReader() require.NoError(t, err) diff --git a/plugin/metrics/prometheus/factory.go b/plugin/metrics/prometheus/factory.go index 56c63456de0..61290dcf3f9 100644 --- a/plugin/metrics/prometheus/factory.go +++ b/plugin/metrics/prometheus/factory.go @@ -11,19 +11,22 @@ import ( "go.opentelemetry.io/otel/trace" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/prometheus/config" "github.com/jaegertracing/jaeger/plugin" prometheusstore "github.com/jaegertracing/jaeger/plugin/metrics/prometheus/metricsstore" "github.com/jaegertracing/jaeger/storage/metricsstore" + "github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics" ) var _ plugin.Configurable = (*Factory)(nil) // Factory implements storage.Factory and creates storage components backed by memory store. type Factory struct { - options *Options - logger *zap.Logger - tracer trace.TracerProvider + options *Options + logger *zap.Logger + tracer trace.TracerProvider + metricsFactory metrics.Factory } // NewFactory creates a new Factory. @@ -47,18 +50,23 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements storage.MetricsFactory. -func (f *Factory) Initialize(logger *zap.Logger) error { - f.logger = logger +func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { + f.metricsFactory, f.logger = metricsFactory, logger return nil } // CreateMetricsReader implements storage.MetricsFactory. func (f *Factory) CreateMetricsReader() (metricsstore.Reader, error) { - return prometheusstore.NewMetricsReader(f.options.Configuration, f.logger, f.tracer) + mr, err := prometheusstore.NewMetricsReader(f.options.Configuration, f.logger, f.tracer) + if err != nil { + return mr, err + } + return metricstoremetrics.NewReaderDecorator(mr, f.metricsFactory), nil } func NewFactoryWithConfig( cfg config.Configuration, + metricsFactory metrics.Factory, logger *zap.Logger, ) (*Factory, error) { if err := cfg.Validate(); err != nil { @@ -68,6 +76,6 @@ func NewFactoryWithConfig( f.options = &Options{ Configuration: cfg, } - f.Initialize(logger) + f.Initialize(metricsFactory, logger) return f, nil } diff --git a/plugin/metrics/prometheus/factory_test.go b/plugin/metrics/prometheus/factory_test.go index 8662badb54a..8bf663558d8 100644 --- a/plugin/metrics/prometheus/factory_test.go +++ b/plugin/metrics/prometheus/factory_test.go @@ -13,6 +13,7 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/metrics" promCfg "github.com/jaegertracing/jaeger/pkg/prometheus/config" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage" @@ -22,7 +23,7 @@ var _ storage.MetricsFactory = new(Factory) func TestPrometheusFactory(t *testing.T) { f := NewFactory() - require.NoError(t, f.Initialize(zap.NewNop())) + require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) assert.NotNil(t, f.logger) listener, err := net.Listen("tcp", "localhost:") @@ -126,7 +127,7 @@ func TestFailedTLSOptions(t *testing.T) { func TestEmptyFactoryConfig(t *testing.T) { cfg := promCfg.Configuration{} - _, err := NewFactoryWithConfig(cfg, zap.NewNop()) + _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop()) require.Error(t, err) } @@ -134,7 +135,7 @@ func TestFactoryConfig(t *testing.T) { cfg := promCfg.Configuration{ ServerURL: "localhost:1234", } - _, err := NewFactoryWithConfig(cfg, zap.NewNop()) + _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop()) require.NoError(t, err) } From aa0323d9b063f1c6ea909ccafecfac51324ae6cf Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 12:44:00 -0500 Subject: [PATCH 5/8] Change Name of Lambda Helper Signed-off-by: Mahad Zaryab --- .../extension/jaegerstorage/extension.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go index 5a9f9c92aed..4be154595d9 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension.go @@ -117,7 +117,7 @@ func newStorageExt(config *Config, telset component.TelemetrySettings) *storageE func (s *storageExt) Start(_ context.Context, host component.Host) error { telset := telemetry.FromOtelComponent(s.telset, host) telset.Metrics = telset.Metrics.Namespace(metrics.NSOptions{Name: "jaeger"}) - getMetricsFactory := func(name, kind string) metrics.Factory { + scopedMetricsFactory := func(name, kind string) metrics.Factory { return telset.Metrics.Namespace(metrics.NSOptions{ Name: "storage", Tags: map[string]string{ @@ -134,35 +134,35 @@ func (s *storageExt) Start(_ context.Context, host component.Host) error { case cfg.Memory != nil: factory, err = memory.NewFactoryWithConfig( *cfg.Memory, - getMetricsFactory(storageName, "memory"), + scopedMetricsFactory(storageName, "memory"), s.telset.Logger, ), nil case cfg.Badger != nil: factory, err = badger.NewFactoryWithConfig( *cfg.Badger, - getMetricsFactory(storageName, "badger"), + scopedMetricsFactory(storageName, "badger"), s.telset.Logger) case cfg.GRPC != nil: grpcTelset := telset - grpcTelset.Metrics = getMetricsFactory(storageName, "grpc") + grpcTelset.Metrics = scopedMetricsFactory(storageName, "grpc") //nolint: contextcheck factory, err = grpc.NewFactoryWithConfig(*cfg.GRPC, grpcTelset) case cfg.Cassandra != nil: factory, err = cassandra.NewFactoryWithConfig( *cfg.Cassandra, - getMetricsFactory(storageName, "cassandra"), + scopedMetricsFactory(storageName, "cassandra"), s.telset.Logger, ) case cfg.Elasticsearch != nil: factory, err = es.NewFactoryWithConfig( *cfg.Elasticsearch, - getMetricsFactory(storageName, "elasticsearch"), + scopedMetricsFactory(storageName, "elasticsearch"), s.telset.Logger, ) case cfg.Opensearch != nil: factory, err = es.NewFactoryWithConfig( *cfg.Opensearch, - getMetricsFactory(storageName, "opensearch"), + scopedMetricsFactory(storageName, "opensearch"), s.telset.Logger, ) } @@ -179,7 +179,7 @@ func (s *storageExt) Start(_ context.Context, host component.Host) error { if cfg.Prometheus != nil { metricsFactory, err = prometheus.NewFactoryWithConfig( *cfg.Prometheus, - getMetricsFactory(metricStorageName, "prometheus"), + scopedMetricsFactory(metricStorageName, "prometheus"), s.telset.Logger) } if err != nil { From f2f671468a4a5bcaa3e94fda0036dffb2e89b2fc Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 13:04:37 -0500 Subject: [PATCH 6/8] Accept telemetry.Settings Instead of Metrics Provider Signed-off-by: Mahad Zaryab --- cmd/all-in-one/main.go | 9 +++---- .../extension/jaegerquery/server_test.go | 3 ++- .../extension/jaegerstorage/extension.go | 5 ++-- cmd/query/main.go | 9 +++---- plugin/metrics/disabled/factory.go | 4 +-- plugin/metrics/disabled/factory_test.go | 6 ++--- plugin/metrics/factory.go | 8 +++--- plugin/metrics/factory_test.go | 4 +-- plugin/metrics/prometheus/factory.go | 26 ++++++++----------- plugin/metrics/prometheus/factory_test.go | 10 +++---- storage/factory.go | 3 ++- storage/mocks/MetricsFactory.go | 14 +++++----- 12 files changed, 49 insertions(+), 52 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 056467f0c37..8f2dd9f9efa 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -117,7 +117,7 @@ by default uses only in-memory database.`, logger.Fatal("Failed to create dependency reader", zap.Error(err)) } - metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger, baseFactory) + metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, baseTelset) if err != nil { logger.Fatal("Failed to create metrics reader", zap.Error(err)) } @@ -237,15 +237,14 @@ func startQuery( func createMetricsQueryService( metricsReaderFactory *metricsPlugin.Factory, v *viper.Viper, - logger *zap.Logger, - metricsReaderMetricsFactory metrics.Factory, + telset telemetry.Settings, ) (querysvc.MetricsQueryService, error) { - if err := metricsReaderFactory.Initialize(metricsReaderMetricsFactory, logger); err != nil { + if err := metricsReaderFactory.Initialize(telset); err != nil { return nil, fmt.Errorf("failed to init metrics reader factory: %w", err) } // Ensure default parameter values are loaded correctly. - metricsReaderFactory.InitFromViper(v, logger) + metricsReaderFactory.InitFromViper(v, telset.Logger) reader, err := metricsReaderFactory.CreateMetricsReader() if err != nil { return nil, fmt.Errorf("failed to create metrics reader: %w", err) diff --git a/cmd/jaeger/internal/extension/jaegerquery/server_test.go b/cmd/jaeger/internal/extension/jaegerquery/server_test.go index ecdabc623f4..b8a68a119c6 100644 --- a/cmd/jaeger/internal/extension/jaegerquery/server_test.go +++ b/cmd/jaeger/internal/extension/jaegerquery/server_test.go @@ -26,6 +26,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/internal/grpctest" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" @@ -73,7 +74,7 @@ type fakeMetricsFactory struct { } // Initialize implements storage.MetricsFactory. -func (fmf fakeMetricsFactory) Initialize(metrics.Factory, *zap.Logger) error { +func (fmf fakeMetricsFactory) Initialize(telemetry.Settings) error { if fmf.name == "need-initialize-error" { return errors.New("test-error") } diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go index 4be154595d9..13fcbe54abf 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension.go @@ -177,10 +177,11 @@ func (s *storageExt) Start(_ context.Context, host component.Host) error { var metricsFactory storage.MetricsFactory var err error if cfg.Prometheus != nil { + promTelset := telset + promTelset.Metrics = scopedMetricsFactory(metricStorageName, "prometheus") metricsFactory, err = prometheus.NewFactoryWithConfig( *cfg.Prometheus, - scopedMetricsFactory(metricStorageName, "prometheus"), - s.telset.Logger) + promTelset) } if err != nil { return fmt.Errorf("failed to initialize metrics storage '%s': %w", metricStorageName, err) diff --git a/cmd/query/main.go b/cmd/query/main.go index df00f37e272..67425b2c9cc 100644 --- a/cmd/query/main.go +++ b/cmd/query/main.go @@ -98,7 +98,7 @@ func main() { logger.Fatal("Failed to create dependency reader", zap.Error(err)) } - metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, logger, baseFactory) + metricsQueryService, err := createMetricsQueryService(metricsReaderFactory, v, baseTelset) if err != nil { logger.Fatal("Failed to create metrics query service", zap.Error(err)) } @@ -165,15 +165,14 @@ func main() { func createMetricsQueryService( metricsReaderFactory *metricsPlugin.Factory, v *viper.Viper, - logger *zap.Logger, - metricsReaderMetricsFactory metrics.Factory, + telset telemetry.Settings, ) (querysvc.MetricsQueryService, error) { - if err := metricsReaderFactory.Initialize(metricsReaderMetricsFactory, logger); err != nil { + if err := metricsReaderFactory.Initialize(telset); err != nil { return nil, fmt.Errorf("failed to init metrics reader factory: %w", err) } // Ensure default parameter values are loaded correctly. - metricsReaderFactory.InitFromViper(v, logger) + metricsReaderFactory.InitFromViper(v, telset.Logger) reader, err := metricsReaderFactory.CreateMetricsReader() if err != nil { return nil, fmt.Errorf("failed to create metrics reader: %w", err) diff --git a/plugin/metrics/disabled/factory.go b/plugin/metrics/disabled/factory.go index 0b407d674d5..b630760f8a9 100644 --- a/plugin/metrics/disabled/factory.go +++ b/plugin/metrics/disabled/factory.go @@ -9,7 +9,7 @@ import ( "github.com/spf13/viper" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/storage/metricsstore" ) @@ -31,7 +31,7 @@ func (*Factory) AddFlags(_ *flag.FlagSet) {} func (*Factory) InitFromViper(_ *viper.Viper, _ *zap.Logger) {} // Initialize implements storage.MetricsFactory. -func (*Factory) Initialize(_ metrics.Factory, _ *zap.Logger) error { +func (*Factory) Initialize(_ telemetry.Settings) error { return nil } diff --git a/plugin/metrics/disabled/factory_test.go b/plugin/metrics/disabled/factory_test.go index 5caac5d9fbf..d26d97b1010 100644 --- a/plugin/metrics/disabled/factory_test.go +++ b/plugin/metrics/disabled/factory_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/storage" ) @@ -18,9 +18,9 @@ var _ storage.MetricsFactory = new(Factory) func TestPrometheusFactory(t *testing.T) { f := NewFactory() - require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + require.NoError(t, f.Initialize(telemetry.NoopSettings())) - err := f.Initialize(metrics.NullFactory, nil) + err := f.Initialize(telemetry.NoopSettings()) require.NoError(t, err) f.AddFlags(nil) diff --git a/plugin/metrics/factory.go b/plugin/metrics/factory.go index 2cf96bc2de0..d6ecf7214ec 100644 --- a/plugin/metrics/factory.go +++ b/plugin/metrics/factory.go @@ -11,6 +11,7 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/plugin/metrics/prometheus" @@ -64,15 +65,16 @@ func (*Factory) getFactoryOfType(factoryType string) (storage.MetricsFactory, er } // Initialize implements storage.MetricsFactory. -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { +func (f *Factory) Initialize(telset telemetry.Settings) error { for kind, factory := range f.factories { - mf := metricsFactory.Namespace(metrics.NSOptions{ + scopedTelset := telset + scopedTelset.Metrics = telset.Metrics.Namespace(metrics.NSOptions{ Name: "storage", Tags: map[string]string{ "kind": kind, }, }) - factory.Initialize(mf, logger) + factory.Initialize(scopedTelset) } return nil } diff --git a/plugin/metrics/factory_test.go b/plugin/metrics/factory_test.go index 2e4e306e076..555b0c027f2 100644 --- a/plugin/metrics/factory_test.go +++ b/plugin/metrics/factory_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/plugin/metrics/disabled" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/mocks" @@ -54,7 +54,7 @@ func TestCreateMetricsReader(t *testing.T) { require.NoError(t, err) require.NotNil(t, f) - require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + require.NoError(t, f.Initialize(telemetry.NoopSettings())) reader, err := f.CreateMetricsReader() require.NoError(t, err) diff --git a/plugin/metrics/prometheus/factory.go b/plugin/metrics/prometheus/factory.go index 61290dcf3f9..aaa47c7d86f 100644 --- a/plugin/metrics/prometheus/factory.go +++ b/plugin/metrics/prometheus/factory.go @@ -7,12 +7,10 @@ import ( "flag" "github.com/spf13/viper" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/trace" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/prometheus/config" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/plugin" prometheusstore "github.com/jaegertracing/jaeger/plugin/metrics/prometheus/metricsstore" "github.com/jaegertracing/jaeger/storage/metricsstore" @@ -23,16 +21,15 @@ var _ plugin.Configurable = (*Factory)(nil) // Factory implements storage.Factory and creates storage components backed by memory store. type Factory struct { - options *Options - logger *zap.Logger - tracer trace.TracerProvider - metricsFactory metrics.Factory + options *Options + telset telemetry.Settings } // NewFactory creates a new Factory. func NewFactory() *Factory { + telset := telemetry.NoopSettings() return &Factory{ - tracer: otel.GetTracerProvider(), + telset: telset, options: NewOptions(), } } @@ -50,24 +47,23 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements storage.MetricsFactory. -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { - f.metricsFactory, f.logger = metricsFactory, logger +func (f *Factory) Initialize(telset telemetry.Settings) error { + f.telset = telset return nil } // CreateMetricsReader implements storage.MetricsFactory. func (f *Factory) CreateMetricsReader() (metricsstore.Reader, error) { - mr, err := prometheusstore.NewMetricsReader(f.options.Configuration, f.logger, f.tracer) + mr, err := prometheusstore.NewMetricsReader(f.options.Configuration, f.telset.Logger, f.telset.TracerProvider) if err != nil { return mr, err } - return metricstoremetrics.NewReaderDecorator(mr, f.metricsFactory), nil + return metricstoremetrics.NewReaderDecorator(mr, f.telset.Metrics), nil } func NewFactoryWithConfig( cfg config.Configuration, - metricsFactory metrics.Factory, - logger *zap.Logger, + telset telemetry.Settings, ) (*Factory, error) { if err := cfg.Validate(); err != nil { return nil, err @@ -76,6 +72,6 @@ func NewFactoryWithConfig( f.options = &Options{ Configuration: cfg, } - f.Initialize(metricsFactory, logger) + f.Initialize(telset) return f, nil } diff --git a/plugin/metrics/prometheus/factory_test.go b/plugin/metrics/prometheus/factory_test.go index 8bf663558d8..b74c8354d80 100644 --- a/plugin/metrics/prometheus/factory_test.go +++ b/plugin/metrics/prometheus/factory_test.go @@ -13,8 +13,8 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" - "github.com/jaegertracing/jaeger/pkg/metrics" promCfg "github.com/jaegertracing/jaeger/pkg/prometheus/config" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage" ) @@ -23,8 +23,8 @@ var _ storage.MetricsFactory = new(Factory) func TestPrometheusFactory(t *testing.T) { f := NewFactory() - require.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) - assert.NotNil(t, f.logger) + require.NoError(t, f.Initialize(telemetry.NoopSettings())) + assert.NotNil(t, f.telset) listener, err := net.Listen("tcp", "localhost:") require.NoError(t, err) @@ -127,7 +127,7 @@ func TestFailedTLSOptions(t *testing.T) { func TestEmptyFactoryConfig(t *testing.T) { cfg := promCfg.Configuration{} - _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop()) + _, err := NewFactoryWithConfig(cfg, telemetry.NoopSettings()) require.Error(t, err) } @@ -135,7 +135,7 @@ func TestFactoryConfig(t *testing.T) { cfg := promCfg.Configuration{ ServerURL: "localhost:1234", } - _, err := NewFactoryWithConfig(cfg, metrics.NullFactory, zap.NewNop()) + _, err := NewFactoryWithConfig(cfg, telemetry.NoopSettings()) require.NoError(t, err) } diff --git a/storage/factory.go b/storage/factory.go index 4b73670b332..efe4414543b 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -12,6 +12,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/pkg/telemetry" "github.com/jaegertracing/jaeger/storage/dependencystore" "github.com/jaegertracing/jaeger/storage/metricsstore" "github.com/jaegertracing/jaeger/storage/samplingstore" @@ -86,7 +87,7 @@ type ArchiveFactory interface { type MetricsFactory interface { // Initialize performs internal initialization of the factory, such as opening connections to the backend store. // It is called after all configuration of the factory itself has been done. - Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error + Initialize(telset telemetry.Settings) error // CreateMetricsReader creates a metricsstore.Reader. CreateMetricsReader() (metricsstore.Reader, error) diff --git a/storage/mocks/MetricsFactory.go b/storage/mocks/MetricsFactory.go index 368c070ded0..f2a087dee88 100644 --- a/storage/mocks/MetricsFactory.go +++ b/storage/mocks/MetricsFactory.go @@ -8,12 +8,10 @@ package mocks import ( - metrics "github.com/jaegertracing/jaeger/pkg/metrics" metricsstore "github.com/jaegertracing/jaeger/storage/metricsstore" - mock "github.com/stretchr/testify/mock" - zap "go.uber.org/zap" + telemetry "github.com/jaegertracing/jaeger/pkg/telemetry" ) // MetricsFactory is an autogenerated mock type for the MetricsFactory type @@ -51,17 +49,17 @@ func (_m *MetricsFactory) CreateMetricsReader() (metricsstore.Reader, error) { return r0, r1 } -// Initialize provides a mock function with given fields: metricsFactory, logger -func (_m *MetricsFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { - ret := _m.Called(metricsFactory, logger) +// Initialize provides a mock function with given fields: telset +func (_m *MetricsFactory) Initialize(telset telemetry.Settings) error { + ret := _m.Called(telset) if len(ret) == 0 { panic("no return value specified for Initialize") } var r0 error - if rf, ok := ret.Get(0).(func(metrics.Factory, *zap.Logger) error); ok { - r0 = rf(metricsFactory, logger) + if rf, ok := ret.Get(0).(func(telemetry.Settings) error); ok { + r0 = rf(telset) } else { r0 = ret.Error(0) } From 0d0921f99ffcc7825bd76aa6e557182ddb33c99f Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 13:15:17 -0500 Subject: [PATCH 7/8] Add Role Label Signed-off-by: Mahad Zaryab --- .../extension/jaegerstorage/extension.go | 17 +++++++++-------- plugin/metrics/factory.go | 1 + 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cmd/jaeger/internal/extension/jaegerstorage/extension.go b/cmd/jaeger/internal/extension/jaegerstorage/extension.go index 13fcbe54abf..1a227c0199b 100644 --- a/cmd/jaeger/internal/extension/jaegerstorage/extension.go +++ b/cmd/jaeger/internal/extension/jaegerstorage/extension.go @@ -117,12 +117,13 @@ func newStorageExt(config *Config, telset component.TelemetrySettings) *storageE func (s *storageExt) Start(_ context.Context, host component.Host) error { telset := telemetry.FromOtelComponent(s.telset, host) telset.Metrics = telset.Metrics.Namespace(metrics.NSOptions{Name: "jaeger"}) - scopedMetricsFactory := func(name, kind string) metrics.Factory { + scopedMetricsFactory := func(name, kind, role string) metrics.Factory { return telset.Metrics.Namespace(metrics.NSOptions{ Name: "storage", Tags: map[string]string{ "name": name, "kind": kind, + "role": role, }, }) } @@ -134,35 +135,35 @@ func (s *storageExt) Start(_ context.Context, host component.Host) error { case cfg.Memory != nil: factory, err = memory.NewFactoryWithConfig( *cfg.Memory, - scopedMetricsFactory(storageName, "memory"), + scopedMetricsFactory(storageName, "memory", "tracestore"), s.telset.Logger, ), nil case cfg.Badger != nil: factory, err = badger.NewFactoryWithConfig( *cfg.Badger, - scopedMetricsFactory(storageName, "badger"), + scopedMetricsFactory(storageName, "badger", "tracestore"), s.telset.Logger) case cfg.GRPC != nil: grpcTelset := telset - grpcTelset.Metrics = scopedMetricsFactory(storageName, "grpc") + grpcTelset.Metrics = scopedMetricsFactory(storageName, "grpc", "tracestore") //nolint: contextcheck factory, err = grpc.NewFactoryWithConfig(*cfg.GRPC, grpcTelset) case cfg.Cassandra != nil: factory, err = cassandra.NewFactoryWithConfig( *cfg.Cassandra, - scopedMetricsFactory(storageName, "cassandra"), + scopedMetricsFactory(storageName, "cassandra", "tracestore"), s.telset.Logger, ) case cfg.Elasticsearch != nil: factory, err = es.NewFactoryWithConfig( *cfg.Elasticsearch, - scopedMetricsFactory(storageName, "elasticsearch"), + scopedMetricsFactory(storageName, "elasticsearch", "tracestore"), s.telset.Logger, ) case cfg.Opensearch != nil: factory, err = es.NewFactoryWithConfig( *cfg.Opensearch, - scopedMetricsFactory(storageName, "opensearch"), + scopedMetricsFactory(storageName, "opensearch", "tracestore"), s.telset.Logger, ) } @@ -178,7 +179,7 @@ func (s *storageExt) Start(_ context.Context, host component.Host) error { var err error if cfg.Prometheus != nil { promTelset := telset - promTelset.Metrics = scopedMetricsFactory(metricStorageName, "prometheus") + promTelset.Metrics = scopedMetricsFactory(metricStorageName, "prometheus", "metricstore") metricsFactory, err = prometheus.NewFactoryWithConfig( *cfg.Prometheus, promTelset) diff --git a/plugin/metrics/factory.go b/plugin/metrics/factory.go index d6ecf7214ec..1e5e5830247 100644 --- a/plugin/metrics/factory.go +++ b/plugin/metrics/factory.go @@ -72,6 +72,7 @@ func (f *Factory) Initialize(telset telemetry.Settings) error { Name: "storage", Tags: map[string]string{ "kind": kind, + "role": "metricstore", }, }) factory.Initialize(scopedTelset) From df0de7082f583c25a5bbfe15cf37abbeb50a80e1 Mon Sep 17 00:00:00 2001 From: Mahad Zaryab Date: Sun, 1 Dec 2024 14:10:55 -0500 Subject: [PATCH 8/8] Add Test For Error In CreateMetricsReader Signed-off-by: Mahad Zaryab --- plugin/metrics/prometheus/factory.go | 2 +- plugin/metrics/prometheus/factory_test.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/plugin/metrics/prometheus/factory.go b/plugin/metrics/prometheus/factory.go index aaa47c7d86f..8aaa0d2e1af 100644 --- a/plugin/metrics/prometheus/factory.go +++ b/plugin/metrics/prometheus/factory.go @@ -56,7 +56,7 @@ func (f *Factory) Initialize(telset telemetry.Settings) error { func (f *Factory) CreateMetricsReader() (metricsstore.Reader, error) { mr, err := prometheusstore.NewMetricsReader(f.options.Configuration, f.telset.Logger, f.telset.TracerProvider) if err != nil { - return mr, err + return nil, err } return metricstoremetrics.NewReaderDecorator(mr, f.telset.Metrics), nil } diff --git a/plugin/metrics/prometheus/factory_test.go b/plugin/metrics/prometheus/factory_test.go index b74c8354d80..c14778928d5 100644 --- a/plugin/metrics/prometheus/factory_test.go +++ b/plugin/metrics/prometheus/factory_test.go @@ -38,6 +38,15 @@ func TestPrometheusFactory(t *testing.T) { assert.NotNil(t, reader) } +func TestCreateMetricsReaderError(t *testing.T) { + f := NewFactory() + f.options.TLS.CAFile = "/does/not/exist" + require.NoError(t, f.Initialize(telemetry.NoopSettings())) + reader, err := f.CreateMetricsReader() + require.Error(t, err) + require.Nil(t, reader) +} + func TestWithDefaultConfiguration(t *testing.T) { f := NewFactory() assert.Equal(t, "http://localhost:9090", f.options.ServerURL)