Skip to content

Commit

Permalink
Call scopeInfoMetrics only once even if it returns an error (#4499)
Browse files Browse the repository at this point in the history
  • Loading branch information
dmathieu authored Sep 14, 2023
1 parent e3d6c8e commit ac56391
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ This release drops the compatibility guarantee of [Go 1.19].
- Removed the deprecated internal packages in `go.opentelemetry.io/otel/exporters/otlp` and its sub-packages. (#4469)
- Dropped guaranteed support for versions of Go less than 1.20. (#4481)

### Fixed

- In `go.opentelemetry.op/otel/exporters/prometheus`, don't try to create the prometheus metric on every `Collect` if we know the scope is invalid. (#4499)

## [1.17.0/0.40.0/0.0.5] 2023-08-28

### Added
Expand Down
17 changes: 16 additions & 1 deletion exporters/prometheus/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ const (
scopeInfoDescription = "Instrumentation Scope metadata"
)

var scopeInfoKeys = [2]string{"otel_scope_name", "otel_scope_version"}
var (
scopeInfoKeys = [2]string{"otel_scope_name", "otel_scope_version"}

errScopeInvalid = errors.New("invalid scope")
)

// Exporter is a Prometheus Exporter that embeds the OTel metric.Reader
// interface for easy instantiation with a MeterProvider.
Expand Down Expand Up @@ -87,6 +91,7 @@ type collector struct {
disableTargetInfo bool
targetInfo prometheus.Metric
scopeInfos map[instrumentation.Scope]prometheus.Metric
scopeInfosInvalid map[instrumentation.Scope]struct{}
metricFamilies map[string]*dto.MetricFamily
}

Expand All @@ -110,6 +115,7 @@ func New(opts ...Option) (*Exporter, error) {
withoutCounterSuffixes: cfg.withoutCounterSuffixes,
disableScopeInfo: cfg.disableScopeInfo,
scopeInfos: make(map[instrumentation.Scope]prometheus.Metric),
scopeInfosInvalid: make(map[instrumentation.Scope]struct{}),
metricFamilies: make(map[string]*dto.MetricFamily),
namespace: cfg.namespace,
}
Expand Down Expand Up @@ -177,6 +183,10 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {

if !c.disableScopeInfo {
scopeInfo, err := c.scopeInfo(scopeMetrics.Scope)
if err == errScopeInvalid {
// Do not report the same error multiple times.
continue
}
if err != nil {
otel.Handle(err)
continue
Expand Down Expand Up @@ -469,8 +479,13 @@ func (c *collector) scopeInfo(scope instrumentation.Scope) (prometheus.Metric, e
return scopeInfo, nil
}

if _, ok := c.scopeInfosInvalid[scope]; ok {
return nil, errScopeInvalid
}

scopeInfo, err := createScopeInfoMetric(scope)
if err != nil {
c.scopeInfosInvalid[scope] = struct{}{}
return nil, fmt.Errorf("cannot create scope info metric: %w", err)
}

Expand Down
19 changes: 19 additions & 0 deletions exporters/prometheus/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package prometheus

import (
"context"
"io"
"os"
"sync"
"testing"
Expand All @@ -25,6 +26,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
otelmetric "go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/sdk/metric"
Expand Down Expand Up @@ -790,6 +792,14 @@ func TestCollectorConcurrentSafe(t *testing.T) {
}

func TestIncompatibleMeterName(t *testing.T) {
defer func(orig otel.ErrorHandler) {
otel.SetErrorHandler(orig)
}(otel.GetErrorHandler())

errs := []error{}
eh := otel.ErrorHandlerFunc(func(e error) { errs = append(errs, e) })
otel.SetErrorHandler(eh)

// This test checks that Prometheus exporter ignores
// when it encounters incompatible meter name.

Expand All @@ -815,4 +825,13 @@ func TestIncompatibleMeterName(t *testing.T) {

err = testutil.GatherAndCompare(registry, file)
require.NoError(t, err)

assert.Equal(t, 1, len(errs))

// A second collect shouldn't trigger new errors
_, err = file.Seek(0, io.SeekStart)
assert.NoError(t, err)
err = testutil.GatherAndCompare(registry, file)
require.NoError(t, err)
assert.Equal(t, 1, len(errs))
}

0 comments on commit ac56391

Please sign in to comment.