From 02616a25c68e674a04df01a1b7ba185a2db8cd5a Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Thu, 31 Aug 2023 14:17:07 -0400 Subject: [PATCH] add WithProducer to the prometheus exporter (#4473) --- CHANGELOG.md | 4 +++ exporters/prometheus/config.go | 21 ++++++------ exporters/prometheus/config_test.go | 53 ++++++++++++++--------------- exporters/prometheus/exporter.go | 2 +- 4 files changed, 42 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 957e5b4d211..0d2f1fd908f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add `WithProducer` option in `go.opentelemetry.op/otel/exporters/prometheus` to restore the ability to register producers on the prometheus exporter's manual reader. (#4473) + ### Deprecated - The `NewMetricExporter` in `go.opentelemetry.io/otel/bridge/opencensus` was deprecated in `v0.35.0` (#3541). diff --git a/exporters/prometheus/config.go b/exporters/prometheus/config.go index dcaba515900..885fd8b8b39 100644 --- a/exporters/prometheus/config.go +++ b/exporters/prometheus/config.go @@ -28,7 +28,7 @@ type config struct { disableTargetInfo bool withoutUnits bool withoutCounterSuffixes bool - aggregation metric.AggregationSelector + readerOpts []metric.ManualReaderOption disableScopeInfo bool namespace string } @@ -47,14 +47,6 @@ func newConfig(opts ...Option) config { return cfg } -func (cfg config) manualReaderOptions() []metric.ManualReaderOption { - opts := []metric.ManualReaderOption{} - if cfg.aggregation != nil { - opts = append(opts, metric.WithAggregationSelector(cfg.aggregation)) - } - return opts -} - // Option sets exporter option values. type Option interface { apply(config) config @@ -81,7 +73,16 @@ func WithRegisterer(reg prometheus.Registerer) Option { // used. func WithAggregationSelector(agg metric.AggregationSelector) Option { return optionFunc(func(cfg config) config { - cfg.aggregation = agg + cfg.readerOpts = append(cfg.readerOpts, metric.WithAggregationSelector(agg)) + return cfg + }) +} + +// WithProducer configure the metric Producer the exporter will use as a source +// of external metric data. +func WithProducer(producer metric.Producer) Option { + return optionFunc(func(cfg config) config { + cfg.readerOpts = append(cfg.readerOpts, metric.WithProducer(producer)) return cfg }) } diff --git a/exporters/prometheus/config_test.go b/exporters/prometheus/config_test.go index 3e3ba9c1cb0..d209fdf3fbd 100644 --- a/exporters/prometheus/config_test.go +++ b/exporters/prometheus/config_test.go @@ -15,18 +15,21 @@ package prometheus // import "go.opentelemetry.io/otel/exporters/prometheus" import ( + "context" "testing" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" ) func TestNewConfig(t *testing.T) { registry := prometheus.NewRegistry() aggregationSelector := func(metric.InstrumentKind) metric.Aggregation { return nil } + producer := &noopProducer{} testCases := []struct { name string @@ -56,6 +59,17 @@ func TestNewConfig(t *testing.T) { }, wantConfig: config{ registerer: prometheus.DefaultRegisterer, + readerOpts: []metric.ManualReaderOption{metric.WithAggregationSelector(aggregationSelector)}, + }, + }, + { + name: "WithProducer", + options: []Option{ + WithProducer(producer), + }, + wantConfig: config{ + registerer: prometheus.DefaultRegisterer, + readerOpts: []metric.ManualReaderOption{metric.WithProducer(producer)}, }, }, { @@ -63,10 +77,15 @@ func TestNewConfig(t *testing.T) { options: []Option{ WithRegisterer(registry), WithAggregationSelector(aggregationSelector), + WithProducer(producer), }, wantConfig: config{ registerer: registry, + readerOpts: []metric.ManualReaderOption{ + metric.WithAggregationSelector(aggregationSelector), + metric.WithProducer(producer), + }, }, }, { @@ -132,38 +151,18 @@ func TestNewConfig(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { cfg := newConfig(tt.options...) - // tested by TestConfigManualReaderOptions - cfg.aggregation = nil + // only check the length of readerOpts, since they are not compareable + assert.Equal(t, len(tt.wantConfig.readerOpts), len(cfg.readerOpts)) + cfg.readerOpts = nil + tt.wantConfig.readerOpts = nil assert.Equal(t, tt.wantConfig, cfg) }) } } -func TestConfigManualReaderOptions(t *testing.T) { - aggregationSelector := func(metric.InstrumentKind) metric.Aggregation { return nil } - - testCases := []struct { - name string - config config - wantOptionCount int - }{ - { - name: "Default", - config: config{}, - wantOptionCount: 0, - }, +type noopProducer struct{} - { - name: "WithAggregationSelector", - config: config{aggregation: aggregationSelector}, - wantOptionCount: 1, - }, - } - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - opts := tt.config.manualReaderOptions() - assert.Len(t, opts, tt.wantOptionCount) - }) - } +func (*noopProducer) Produce(ctx context.Context) ([]metricdata.ScopeMetrics, error) { + return nil, nil } diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go index 81b12254f85..c88e82a1385 100644 --- a/exporters/prometheus/exporter.go +++ b/exporters/prometheus/exporter.go @@ -101,7 +101,7 @@ func New(opts ...Option) (*Exporter, error) { // this assumes that the default temporality selector will always return cumulative. // we only support cumulative temporality, so building our own reader enforces this. // TODO (#3244): Enable some way to configure the reader, but not change temporality. - reader := metric.NewManualReader(cfg.manualReaderOptions()...) + reader := metric.NewManualReader(cfg.readerOpts...) collector := &collector{ reader: reader,