Skip to content

Commit

Permalink
Accept telemetry.Settings Instead of Metrics Provider
Browse files Browse the repository at this point in the history
Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
mahadzaryab1 committed Dec 1, 2024
1 parent aa0323d commit f2f6714
Show file tree
Hide file tree
Showing 12 changed files with 49 additions and 52 deletions.
9 changes: 4 additions & 5 deletions cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion cmd/jaeger/internal/extension/jaegerquery/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions plugin/metrics/disabled/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions plugin/metrics/disabled/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ 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"
)

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)
Expand Down
8 changes: 5 additions & 3 deletions plugin/metrics/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions plugin/metrics/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 11 additions & 15 deletions plugin/metrics/prometheus/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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(),
}
}
Expand All @@ -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
}

Check warning on line 60 in plugin/metrics/prometheus/factory.go

View check run for this annotation

Codecov / codecov/patch

plugin/metrics/prometheus/factory.go#L59-L60

Added lines #L59 - L60 were not covered by tests
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
Expand All @@ -76,6 +72,6 @@ func NewFactoryWithConfig(
f.options = &Options{
Configuration: cfg,
}
f.Initialize(metricsFactory, logger)
f.Initialize(telset)
return f, nil
}
10 changes: 5 additions & 5 deletions plugin/metrics/prometheus/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down Expand Up @@ -127,15 +127,15 @@ 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)
}

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)
}

Expand Down
3 changes: 2 additions & 1 deletion storage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 6 additions & 8 deletions storage/mocks/MetricsFactory.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f2f6714

Please sign in to comment.