Skip to content

Commit

Permalink
Removed reader config can be added after (open-telemetry#3244)
Browse files Browse the repository at this point in the history
  • Loading branch information
MadVikingGod committed Sep 29, 2022
1 parent 406817d commit cbb8001
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 57 deletions.
39 changes: 1 addition & 38 deletions exporters/prometheus/confg_test.go
Expand Up @@ -15,62 +15,33 @@
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"
)

type testExporter struct{}

func (e testExporter) Export(_ context.Context, _ metricdata.ResourceMetrics) error {
return nil
}

func (e testExporter) ForceFlush(_ context.Context) error {
return nil
}

func (e testExporter) Shutdown(_ context.Context) error {
return nil
}

func TestNewConfig(t *testing.T) {
registry := prometheus.NewRegistry()

testCases := []struct {
name string
options []Option
wantReaderType metric.Reader
wantRegisterer prometheus.Registerer
wantGatherer prometheus.Gatherer
}{
{
name: "Default",
options: nil,
wantReaderType: metric.NewManualReader(),
wantRegisterer: prometheus.DefaultRegisterer,
wantGatherer: prometheus.DefaultGatherer,
},
{
name: "WithReader",
options: []Option{
WithReader(metric.NewPeriodicReader(testExporter{})),
},
wantReaderType: metric.NewPeriodicReader(testExporter{}),
wantRegisterer: prometheus.DefaultRegisterer,
wantGatherer: prometheus.DefaultGatherer,
},

{
name: "WithGatherer",
options: []Option{
WithGatherer(registry),
},
wantReaderType: metric.NewManualReader(),
wantRegisterer: prometheus.DefaultRegisterer,
wantGatherer: registry,
},
Expand All @@ -79,29 +50,24 @@ func TestNewConfig(t *testing.T) {
options: []Option{
WithRegisterer(registry),
},
wantReaderType: metric.NewManualReader(),
wantRegisterer: registry,
wantGatherer: prometheus.DefaultGatherer,
},
{
name: "Multiple Options",
options: []Option{
WithReader(metric.NewPeriodicReader(testExporter{})),
WithGatherer(registry),
WithRegisterer(registry),
},
wantReaderType: metric.NewPeriodicReader(testExporter{}),
wantRegisterer: registry,
wantGatherer: registry,
},
{
name: "nil options do nothing",
options: []Option{
WithReader(nil),
WithGatherer(nil),
WithRegisterer(nil),
},
wantReaderType: metric.NewManualReader(),
wantRegisterer: prometheus.DefaultRegisterer,
wantGatherer: prometheus.DefaultGatherer,
},
Expand All @@ -110,9 +76,6 @@ func TestNewConfig(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
cfg := newConfig(tt.options...)

// If no reader is provided you should get a new ManualReader.
assert.IsType(t, tt.wantReaderType, cfg.reader)

// If no Registry is provided you should get the DefaultRegisterer and DefaultGatherer.
assert.Equal(t, tt.wantRegisterer, cfg.registerer)
assert.Equal(t, tt.wantGatherer, cfg.gatherer)
Expand Down
17 changes: 0 additions & 17 deletions exporters/prometheus/config.go
Expand Up @@ -16,12 +16,8 @@ package prometheus // import "go.opentelemetry.io/otel/exporters/prometheus"

import (
"github.com/prometheus/client_golang/prometheus"

"go.opentelemetry.io/otel/sdk/metric"
) // config is added here to allow for options expansion in the future.
type config struct {
reader metric.Reader

registerer prometheus.Registerer
gatherer prometheus.Gatherer
}
Expand All @@ -32,10 +28,6 @@ func newConfig(opts ...Option) config {
cfg = opt.apply(cfg)
}

if cfg.reader == nil {
cfg.reader = metric.NewManualReader()
}

if cfg.gatherer == nil {
cfg.gatherer = prometheus.DefaultGatherer
}
Expand All @@ -57,15 +49,6 @@ func (fn optionFunc) apply(cfg config) config {
return fn(cfg)
}

// WithReader controls where the Exporter reader Collects() from. If no reader
// is passed a ManualReader will be used.
func WithReader(rdr metric.Reader) Option {
return optionFunc(func(cfg config) config {
cfg.reader = rdr
return cfg
})
}

// WithRegisterer configures which prometheus Registerer the Exporter will
// register with. If no registerer is used the prometheus DefaultRegisterer is
// used.
Expand Down
7 changes: 5 additions & 2 deletions exporters/prometheus/exporter.go
Expand Up @@ -52,17 +52,20 @@ type collector struct {
func New(opts ...Option) (*Exporter, error) {
cfg := newConfig(opts...)

// TODO (#????): Enable some way to configure the reader, but not change temporality.
reader := metric.NewManualReader()

handler := promhttp.HandlerFor(cfg.gatherer, promhttp.HandlerOpts{})
collector := &collector{
reader: cfg.reader,
reader: reader,
}

if err := cfg.registerer.Register(collector); err != nil {
return nil, fmt.Errorf("cannot register the collector: %w", err)
}

e := &Exporter{
Reader: cfg.reader,
Reader: reader,
Collector: collector,

handler: handler,
Expand Down

0 comments on commit cbb8001

Please sign in to comment.