Skip to content

Commit

Permalink
[fix] Use metrics decorator around MetricStorage (#6262)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- The SPM Troubleshooting docs were referring to metrics that are
supposed to be emitted about MetricStore operations, but no such metrics
were present in v2. It turns out the metrics decorator was not applied
to MetricStore.

## Description of the changes
- Apply decorator
- Rename decorator package to `metricstoremetrics`

## How was this change tested?
```
$ curl -s http://localhost:8888/metrics | grep -v '^#' | grep get_call_rates | head -3
metricstore_latency_bucket{operation="get_call_rates",result="ok",service_instance_id="463e3888-a9f8-4b10-8fd7-62aa0ccd6fe3",service_name="jaeger",service_version="v2.0.0",le="0"} 0
metricstore_latency_bucket{operation="get_call_rates",result="ok",service_instance_id="463e3888-a9f8-4b10-8fd7-62aa0ccd6fe3",service_name="jaeger",service_version="v2.0.0",le="5"} 2
metricstore_latency_bucket{operation="get_call_rates",result="ok",service_instance_id="463e3888-a9f8-4b10-8fd7-62aa0ccd6fe3",service_name="jaeger",service_version="v2.0.0",le="10"} 2
```

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
  • Loading branch information
yurishkuro authored Nov 26, 2024
1 parent 16e964d commit 7572524
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 20 deletions.
4 changes: 2 additions & 2 deletions cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import (
"github.com/jaegertracing/jaeger/plugin/storage"
"github.com/jaegertracing/jaeger/ports"
"github.com/jaegertracing/jaeger/storage/dependencystore"
metricsstoreMetrics "github.com/jaegertracing/jaeger/storage/metricsstore/metrics"
"github.com/jaegertracing/jaeger/storage/metricsstore/metricstoremetrics"
"github.com/jaegertracing/jaeger/storage/spanstore"
"github.com/jaegertracing/jaeger/storage/spanstore/spanstoremetrics"
)
Expand Down Expand Up @@ -256,5 +256,5 @@ func createMetricsQueryService(
}

// Decorate the metrics reader with metrics instrumentation.
return metricsstoreMetrics.NewReadMetricsDecorator(reader, metricsReaderMetricsFactory), nil
return metricstoremetrics.NewReaderDecorator(reader, metricsReaderMetricsFactory), nil
}
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: "jaeger_metricstore"})
return metricstoremetrics.NewReaderDecorator(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.NewReaderDecorator(reader, metricsReaderMetricsFactory), nil
}
2 changes: 1 addition & 1 deletion docker-compose/monitor/docker-compose-v2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ services:
prometheus:
networks:
- backend
image: prom/prometheus:latest
image: prom/prometheus:v3.0.0
volumes:
- "./prometheus.yml:/etc/prometheus/prometheus.yml"
ports:
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
package metricstoremetrics

import (
"context"
Expand Down Expand Up @@ -39,7 +39,7 @@ func (q *queryMetrics) emit(err error, latency time.Duration) {
}

// NewReadMetricsDecorator returns a new ReadMetricsDecorator.
func NewReadMetricsDecorator(reader metricsstore.Reader, metricsFactory metrics.Factory) *ReadMetricsDecorator {
func NewReaderDecorator(reader metricsstore.Reader, metricsFactory metrics.Factory) *ReadMetricsDecorator {
return &ReadMetricsDecorator{
reader: reader,
getLatenciesMetrics: buildQueryMetrics("get_latencies", metricsFactory),
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.NewReaderDecorator(&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.NewReaderDecorator(&mockReader, mf)
glParams := &metricsstore.LatenciesQueryParameters{}
mockReader.On("GetLatencies", context.Background(), glParams).
Return(&protometrics.MetricFamily{}, errors.New("failure"))
Expand Down

0 comments on commit 7572524

Please sign in to comment.