Skip to content

Commit

Permalink
[fix] Use metrics decorator around MetricStorage
Browse files Browse the repository at this point in the history
Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro committed Nov 26, 2024
1 parent 16e964d commit 9124379
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 16 deletions.
11 changes: 8 additions & 3 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"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"
"github.com/jaegertracing/jaeger/storage/spanstore/spanstoremetrics"
)

Expand Down Expand Up @@ -147,16 +148,20 @@ func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, e
return disabled.NewMetricsReader()
}

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

metricsReader, err := mf.CreateMetricsReader()
metricsReader, err := msf.CreateMetricsReader()
if err != nil {
return nil, fmt.Errorf("cannot create metrics reader %w", err)
}
return metricsReader, err

// Decorate the metrics reader with metrics instrumentation.
mf := otelmetrics.NewFactory(s.telset.MeterProvider)
mf = mf.Namespace(metrics.NSOptions{Name: "metricstore"})
return metricstoremetrics.NewReadMetricsDecorator(metricsReader, mf), nil
}

func (s *server) Shutdown(ctx context.Context) error {
Expand Down
8 changes: 5 additions & 3 deletions cmd/jaeger/internal/extension/jaegerstorage/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ type storageExt struct {
metricsFactories map[string]storage.MetricsFactory
}

// GetStorageFactory locates the extension in Host and retrieves a storage factory from it with the given name.
// GetStorageFactory locates the extension in Host and retrieves
// a trace storage factory from it with the given name.
func GetStorageFactory(name string, host component.Host) (storage.Factory, error) {
ext, err := findExtension(host)
if err != nil {
Expand All @@ -59,8 +60,9 @@ func GetStorageFactory(name string, host component.Host) (storage.Factory, error
return f, nil
}

// 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) {
// GetMetricStorageFactory locates the extension in Host and retrieves
// a metric storage factory from it with the given name.
func GetMetricStorageFactory(name string, host component.Host) (storage.MetricsFactory, error) {
ext, err := findExtension(host)
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions cmd/jaeger/internal/extension/jaegerstorage/extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,13 @@ func TestStorageFactoryBadNameError(t *testing.T) {
}

func TestMetricsFactoryBadHostError(t *testing.T) {
_, err := GetMetricsFactory("something", componenttest.NewNopHost())
_, err := GetMetricStorageFactory("something", componenttest.NewNopHost())
require.ErrorContains(t, err, "cannot find extension")
}

func TestMetricsFactoryBadNameError(t *testing.T) {
host := storagetest.NewStorageHost().WithExtension(ID, startStorageExtension(t, "", "foo"))
_, err := GetMetricsFactory("bar", host)
_, err := GetMetricStorageFactory("bar", host)
require.ErrorContains(t, err, "cannot find metric storage 'bar'")
}

Expand Down Expand Up @@ -116,7 +116,7 @@ func TestGetFactory(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, f2)

f3, err := GetMetricsFactory(metricname, host)
f3, err := GetMetricStorageFactory(metricname, host)
require.NoError(t, err)
require.NotNil(t, f3)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
metricsPlugin "github.com/jaegertracing/jaeger/plugin/metrics"
"github.com/jaegertracing/jaeger/plugin/storage"
"github.com/jaegertracing/jaeger/ports"
metricsstoreMetrics "github.com/jaegertracing/jaeger/storage/metricsstore/metrics"
"github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics"
"github.com/jaegertracing/jaeger/storage/spanstore/spanstoremetrics"
)

Expand Down Expand Up @@ -178,5 +178,5 @@ func createMetricsQueryService(
}

// Decorate the metrics reader with metrics instrumentation.
return metricsstoreMetrics.NewReadMetricsDecorator(reader, metricsReaderMetricsFactory), nil
return metricstoremetrics.NewReadMetricsDecorator(reader, metricsReaderMetricsFactory), nil
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2022 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package metrics
package metricstoremetrics

import (
"context"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2022 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package metrics_test
package metricstoremetrics_test

import (
"context"
Expand All @@ -15,15 +15,15 @@ import (
"github.com/jaegertracing/jaeger/pkg/testutils"
protometrics "github.com/jaegertracing/jaeger/proto-gen/api_v2/metrics"
"github.com/jaegertracing/jaeger/storage/metricsstore"
"github.com/jaegertracing/jaeger/storage/metricsstore/metrics"
"github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics"
"github.com/jaegertracing/jaeger/storage/metricsstore/mocks"
)

func TestSuccessfulUnderlyingCalls(t *testing.T) {
mf := metricstest.NewFactory(0)

mockReader := mocks.Reader{}
mrs := metrics.NewReadMetricsDecorator(&mockReader, mf)
mrs := metricstoremetrics.NewReadMetricsDecorator(&mockReader, mf)
glParams := &metricsstore.LatenciesQueryParameters{}
mockReader.On("GetLatencies", context.Background(), glParams).
Return(&protometrics.MetricFamily{}, nil)
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestFailingUnderlyingCalls(t *testing.T) {
mf := metricstest.NewFactory(0)

mockReader := mocks.Reader{}
mrs := metrics.NewReadMetricsDecorator(&mockReader, mf)
mrs := metricstoremetrics.NewReadMetricsDecorator(&mockReader, mf)
glParams := &metricsstore.LatenciesQueryParameters{}
mockReader.On("GetLatencies", context.Background(), glParams).
Return(&protometrics.MetricFamily{}, errors.New("failure"))
Expand Down

0 comments on commit 9124379

Please sign in to comment.