You've already forked opentelemetry-go
							
							
				mirror of
				https://github.com/open-telemetry/opentelemetry-go.git
				synced 2025-10-31 00:07:40 +02:00 
			
		
		
		
	feat: Improve error handling in prometheus exporter (#7363)
fix #7066 --------- Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
This commit is contained in:
		| @@ -42,6 +42,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | ||||
| - `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/meter` synchronously de-duplicates the passed attributes instead of delegating it to the returned `MeterOption`. (#7266) | ||||
| - `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/log` synchronously de-duplicates the passed attributes instead of delegating it to the returned `LoggerOption`. (#7266) | ||||
| - `Distinct` in `go.opentelemetry.io/otel/attribute` is no longer guaranteed to uniquely identify an attribute set. Collisions between `Distinct` values for different Sets are possible with extremely high cardinality (billions of series per instrument), but are highly unlikely. (#7175) | ||||
| - Improve error handling for dropped data during translation by using `prometheus.NewInvalidMetric` in `go.opentelemetry.io/otel/exporters/prometheus`. | ||||
|   **Breaking Change:** Previously, these cases were only logged and scrapes succeeded. | ||||
|   Now, when translation would drop data (e.g., invalid label/value), the exporter emits a `NewInvalidMetric`, and Prometheus scrapes **fail with HTTP 500** by default. | ||||
|   To preserve the prior behavior (scrapes succeed while errors are logged), configure your Prometheus HTTP handler with: `promhttp.HandlerOpts{ ErrorHandling: promhttp.ContinueOnError }`. (#7363) | ||||
| - The default `TranslationStrategy` in `go.opentelemetry.io/exporters/prometheus` is changed from `otlptranslator.NoUTF8EscapingWithSuffixes` to `otlptranslator.UnderscoreEscapingWithSuffixes`. (#7421) | ||||
| - The `ErrorType` function in `go.opentelemetry.io/otel/semconv/v1.37.0` now handles custom error types. | ||||
|   If an error implements an `ErrorType() string` method, the return value of that method will be used as the error type. (#7442) | ||||
|   | ||||
							
								
								
									
										13
									
								
								exporters/prometheus/errors.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										13
									
								
								exporters/prometheus/errors.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,13 @@ | ||||
| // Copyright The OpenTelemetry Authors | ||||
| // SPDX-License-Identifier: Apache-2.0 | ||||
|  | ||||
| package prometheus // import "go.opentelemetry.io/otel/exporters/prometheus" | ||||
|  | ||||
| import "errors" | ||||
|  | ||||
| // Sentinel errors for consistent error checks in tests. | ||||
| var ( | ||||
| 	errInvalidMetricType = errors.New("invalid metric type") | ||||
| 	errInvalidMetric     = errors.New("invalid metric") | ||||
| 	errEHScaleBelowMin   = errors.New("exponential histogram scale below minimum supported") | ||||
| ) | ||||
| @@ -241,7 +241,7 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { | ||||
|  | ||||
| 			attrKeys, attrVals, e := getAttrs(scopeMetrics.Scope.Attributes, c.labelNamer) | ||||
| 			if e != nil { | ||||
| 				otel.Handle(e) | ||||
| 				reportError(ch, nil, e) | ||||
| 				err = errors.Join(err, fmt.Errorf("failed to getAttrs for ScopeMetrics %d: %w", j, e)) | ||||
| 				continue | ||||
| 			} | ||||
| @@ -258,19 +258,19 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { | ||||
| 		for k, m := range scopeMetrics.Metrics { | ||||
| 			typ := c.metricType(m) | ||||
| 			if typ == nil { | ||||
| 				reportError(ch, nil, errInvalidMetricType) | ||||
| 				continue | ||||
| 			} | ||||
| 			name, e := c.getName(m) | ||||
| 			if e != nil { | ||||
| 				// TODO(#7066): Handle this error better. It's not clear this can be | ||||
| 				// reached, bad metric names should / will be caught at creation time. | ||||
| 				otel.Handle(e) | ||||
| 				reportError(ch, nil, e) | ||||
| 				err = errors.Join(err, fmt.Errorf("failed to getAttrs for ScopeMetrics %d, Metrics %d: %w", j, k, e)) | ||||
| 				continue | ||||
| 			} | ||||
|  | ||||
| 			drop, help := c.validateMetrics(name, m.Description, typ) | ||||
| 			if drop { | ||||
| 				reportError(ch, nil, errInvalidMetric) | ||||
| 				continue | ||||
| 			} | ||||
|  | ||||
| @@ -373,7 +373,7 @@ func addExponentialHistogramMetric[N int64 | float64]( | ||||
| 	for j, dp := range histogram.DataPoints { | ||||
| 		keys, values, e := getAttrs(dp.Attributes, labelNamer) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, nil, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to getAttrs for histogram.DataPoints %d: %w", j, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -386,11 +386,11 @@ func addExponentialHistogramMetric[N int64 | float64]( | ||||
| 		scale := dp.Scale | ||||
| 		if scale < -4 { | ||||
| 			// Reject scales below -4 as they cannot be represented in Prometheus | ||||
| 			e := fmt.Errorf( | ||||
| 				"exponential histogram scale %d is below minimum supported scale -4, skipping data point", | ||||
| 				scale, | ||||
| 			reportError( | ||||
| 				ch, | ||||
| 				desc, | ||||
| 				fmt.Errorf("%w: %d (min -4)", errEHScaleBelowMin, scale), | ||||
| 			) | ||||
| 			otel.Handle(e) | ||||
| 			err = errors.Join(err, e) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -440,7 +440,7 @@ func addExponentialHistogramMetric[N int64 | float64]( | ||||
| 			dp.StartTime, | ||||
| 			values...) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, desc, e) | ||||
| 			err = errors.Join( | ||||
| 				err, | ||||
| 				fmt.Errorf("failed to NewConstNativeHistogram for histogram.DataPoints %d: %w", j, e), | ||||
| @@ -474,7 +474,7 @@ func addHistogramMetric[N int64 | float64]( | ||||
| 	for j, dp := range histogram.DataPoints { | ||||
| 		keys, values, e := getAttrs(dp.Attributes, labelNamer) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, nil, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to getAttrs for histogram.DataPoints %d: %w", j, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -491,7 +491,7 @@ func addHistogramMetric[N int64 | float64]( | ||||
| 		} | ||||
| 		m, e := prometheus.NewConstHistogram(desc, dp.Count, float64(dp.Sum), buckets, values...) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, desc, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to NewConstMetric for histogram.DataPoints %d: %w", j, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -527,7 +527,7 @@ func addSumMetric[N int64 | float64]( | ||||
| 	for i, dp := range sum.DataPoints { | ||||
| 		keys, values, e := getAttrs(dp.Attributes, labelNamer) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, nil, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to getAttrs for sum.DataPoints %d: %w", i, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -537,7 +537,7 @@ func addSumMetric[N int64 | float64]( | ||||
| 		desc := prometheus.NewDesc(name, m.Description, keys, nil) | ||||
| 		m, e := prometheus.NewConstMetric(desc, valueType, float64(dp.Value), values...) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, desc, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to NewConstMetric for sum.DataPoints %d: %w", i, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -572,7 +572,7 @@ func addGaugeMetric[N int64 | float64]( | ||||
| 	for i, dp := range gauge.DataPoints { | ||||
| 		keys, values, e := getAttrs(dp.Attributes, labelNamer) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, nil, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to getAttrs for gauge.DataPoints %d: %w", i, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -582,7 +582,7 @@ func addGaugeMetric[N int64 | float64]( | ||||
| 		desc := prometheus.NewDesc(name, m.Description, keys, nil) | ||||
| 		m, e := prometheus.NewConstMetric(desc, prometheus.GaugeValue, float64(dp.Value), values...) | ||||
| 		if e != nil { | ||||
| 			otel.Handle(e) | ||||
| 			reportError(ch, desc, e) | ||||
| 			err = errors.Join(err, fmt.Errorf("failed to NewConstMetric for gauge.DataPoints %d: %w", i, e)) | ||||
| 			continue | ||||
| 		} | ||||
| @@ -803,3 +803,10 @@ func attributesToLabels(attrs []attribute.KeyValue, labelNamer otlptranslator.La | ||||
| 	} | ||||
| 	return labels, nil | ||||
| } | ||||
|  | ||||
| func reportError(ch chan<- prometheus.Metric, desc *prometheus.Desc, err error) { | ||||
| 	if desc == nil { | ||||
| 		desc = prometheus.NewInvalidDesc(err) | ||||
| 	} | ||||
| 	ch <- prometheus.NewInvalidMetric(desc, err) | ||||
| } | ||||
|   | ||||
| @@ -8,12 +8,15 @@ import ( | ||||
| 	"errors" | ||||
| 	"fmt" | ||||
| 	"math" | ||||
| 	"net/http" | ||||
| 	"net/http/httptest" | ||||
| 	"os" | ||||
| 	"sync" | ||||
| 	"testing" | ||||
| 	"time" | ||||
|  | ||||
| 	"github.com/prometheus/client_golang/prometheus" | ||||
| 	"github.com/prometheus/client_golang/prometheus/promhttp" | ||||
| 	"github.com/prometheus/client_golang/prometheus/testutil" | ||||
| 	dto "github.com/prometheus/client_model/go" | ||||
| 	"github.com/prometheus/otlptranslator" | ||||
| @@ -32,6 +35,27 @@ import ( | ||||
| 	"go.opentelemetry.io/otel/trace" | ||||
| ) | ||||
|  | ||||
| // producerFunc adapts a function to implement metric.Producer. | ||||
| type producerFunc func(context.Context) ([]metricdata.ScopeMetrics, error) | ||||
|  | ||||
| func (f producerFunc) Produce(ctx context.Context) ([]metricdata.ScopeMetrics, error) { return f(ctx) } | ||||
|  | ||||
| // Helper: scrape with ContinueOnError and return body + status. | ||||
| func scrapeWithContinueOnError(reg *prometheus.Registry) (int, string) { | ||||
| 	h := promhttp.HandlerFor( | ||||
| 		reg, | ||||
| 		promhttp.HandlerOpts{ | ||||
| 			ErrorHandling: promhttp.ContinueOnError, | ||||
| 		}, | ||||
| 	) | ||||
|  | ||||
| 	rr := httptest.NewRecorder() | ||||
| 	req := httptest.NewRequest(http.MethodGet, "/metrics", http.NoBody) | ||||
| 	h.ServeHTTP(rr, req) | ||||
|  | ||||
| 	return rr.Code, rr.Body.String() | ||||
| } | ||||
|  | ||||
| func TestPrometheusExporter(t *testing.T) { | ||||
| 	testCases := []struct { | ||||
| 		name                string | ||||
| @@ -786,6 +810,7 @@ func TestDuplicateMetrics(t *testing.T) { | ||||
| 		recordMetrics         func(ctx context.Context, meterA, meterB otelmetric.Meter) | ||||
| 		options               []Option | ||||
| 		possibleExpectedFiles []string | ||||
| 		expectGatherError     bool | ||||
| 	}{ | ||||
| 		{ | ||||
| 			name: "no_conflict_two_counters", | ||||
| @@ -972,6 +997,7 @@ func TestDuplicateMetrics(t *testing.T) { | ||||
| 				"testdata/conflict_type_counter_and_updowncounter_1.txt", | ||||
| 				"testdata/conflict_type_counter_and_updowncounter_2.txt", | ||||
| 			}, | ||||
| 			expectGatherError: true, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "conflict_type_histogram_and_updowncounter", | ||||
| @@ -992,6 +1018,7 @@ func TestDuplicateMetrics(t *testing.T) { | ||||
| 				"testdata/conflict_type_histogram_and_updowncounter_1.txt", | ||||
| 				"testdata/conflict_type_histogram_and_updowncounter_2.txt", | ||||
| 			}, | ||||
| 			expectGatherError: true, | ||||
| 		}, | ||||
| 	} | ||||
|  | ||||
| @@ -1032,19 +1059,43 @@ func TestDuplicateMetrics(t *testing.T) { | ||||
|  | ||||
| 			tc.recordMetrics(ctx, meterA, meterB) | ||||
|  | ||||
| 			match := false | ||||
| 			for _, filename := range tc.possibleExpectedFiles { | ||||
| 				file, ferr := os.Open(filename) | ||||
| 				require.NoError(t, ferr) | ||||
| 				t.Cleanup(func() { require.NoError(t, file.Close()) }) | ||||
| 			if tc.expectGatherError { | ||||
| 				// With improved error handling, conflicting instrument types emit an invalid metric. | ||||
| 				// Gathering should surface an error instead of silently dropping. | ||||
| 				_, err := registry.Gather() | ||||
| 				require.Error(t, err) | ||||
|  | ||||
| 				err = testutil.GatherAndCompare(registry, file) | ||||
| 				if err == nil { | ||||
| 					match = true | ||||
| 					break | ||||
| 				// 2) Also assert what users will see if they opt into ContinueOnError. | ||||
| 				// Compare the HTTP body to an expected file that contains only the valid series | ||||
| 				// (e.g., "target_info" and any non-conflicting families). | ||||
| 				status, body := scrapeWithContinueOnError(registry) | ||||
| 				require.Equal(t, http.StatusOK, status) | ||||
|  | ||||
| 				matched := false | ||||
| 				for _, filename := range tc.possibleExpectedFiles { | ||||
| 					want, ferr := os.ReadFile(filename) | ||||
| 					require.NoError(t, ferr) | ||||
| 					if body == string(want) { | ||||
| 						matched = true | ||||
| 						break | ||||
| 					} | ||||
| 				} | ||||
| 				require.Truef(t, matched, "expected export not produced under ContinueOnError; got:\n%s", body) | ||||
| 			} else { | ||||
| 				match := false | ||||
| 				for _, filename := range tc.possibleExpectedFiles { | ||||
| 					file, ferr := os.Open(filename) | ||||
| 					require.NoError(t, ferr) | ||||
| 					t.Cleanup(func() { require.NoError(t, file.Close()) }) | ||||
|  | ||||
| 					err = testutil.GatherAndCompare(registry, file) | ||||
| 					if err == nil { | ||||
| 						match = true | ||||
| 						break | ||||
| 					} | ||||
| 				} | ||||
| 				require.Truef(t, match, "expected export not produced: %v", err) | ||||
| 			} | ||||
| 			require.Truef(t, match, "expected export not produced: %v", err) | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
| @@ -1378,14 +1429,18 @@ func TestExponentialHistogramScaleValidation(t *testing.T) { | ||||
| 			nil, | ||||
| 			t.Context(), | ||||
| 		) | ||||
| 		assert.Error(t, capturedError) | ||||
| 		assert.Contains(t, capturedError.Error(), "scale -5 is below minimum") | ||||
| 		// Expect an invalid metric to be sent that carries the scale error. | ||||
| 		var pm prometheus.Metric | ||||
| 		select { | ||||
| 		case <-ch: | ||||
| 			t.Error("Expected no metrics to be produced for invalid scale") | ||||
| 		case pm = <-ch: | ||||
| 		default: | ||||
| 			// No metrics were produced for the invalid scale | ||||
| 			t.Fatalf("expected an invalid metric to be emitted for invalid scale, but channel was empty") | ||||
| 		} | ||||
| 		var dtoMetric dto.Metric | ||||
| 		werr := pm.Write(&dtoMetric) | ||||
| 		require.ErrorIs(t, werr, errEHScaleBelowMin) | ||||
| 		// The exporter reports via invalid metric, not the global otel error handler. | ||||
| 		assert.NoError(t, capturedError) | ||||
| 	}) | ||||
| } | ||||
|  | ||||
| @@ -1784,16 +1839,114 @@ func TestDownscaleExponentialBucketEdgeCases(t *testing.T) { | ||||
| // TestEscapingErrorHandling increases test coverage by exercising some error | ||||
| // conditions. | ||||
| func TestEscapingErrorHandling(t *testing.T) { | ||||
| 	// Helper to create a producer that emits a Summary (unsupported) metric. | ||||
| 	makeSummaryProducer := func() metric.Producer { | ||||
| 		return producerFunc(func(_ context.Context) ([]metricdata.ScopeMetrics, error) { | ||||
| 			return []metricdata.ScopeMetrics{ | ||||
| 				{ | ||||
| 					Metrics: []metricdata.Metrics{ | ||||
| 						{ | ||||
| 							Name:        "summary_metric", | ||||
| 							Description: "unsupported summary", | ||||
| 							Data:        metricdata.Summary{}, | ||||
| 						}, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, nil | ||||
| 		}) | ||||
| 	} | ||||
| 	// Helper to create a producer that emits a metric with an invalid name, to | ||||
| 	// force getName() to fail and exercise reportError at that branch. | ||||
| 	makeBadNameProducer := func() metric.Producer { | ||||
| 		return producerFunc(func(_ context.Context) ([]metricdata.ScopeMetrics, error) { | ||||
| 			return []metricdata.ScopeMetrics{ | ||||
| 				{ | ||||
| 					Metrics: []metricdata.Metrics{ | ||||
| 						{ | ||||
| 							Name:        "$%^&", // intentionally invalid; translation should fail normalization | ||||
| 							Description: "bad name for translation", | ||||
| 							// Any supported type is fine; getName runs before add* functions. | ||||
| 							Data: metricdata.Gauge[float64]{ | ||||
| 								DataPoints: []metricdata.DataPoint[float64]{ | ||||
| 									{Value: 1}, | ||||
| 								}, | ||||
| 							}, | ||||
| 						}, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, nil | ||||
| 		}) | ||||
| 	} | ||||
| 	// Helper to create a producer that emits an ExponentialHistogram with a bad | ||||
| 	// label, to exercise addExponentialHistogramMetric getAttrs error path. | ||||
| 	makeBadEHProducer := func() metric.Producer { | ||||
| 		return producerFunc(func(_ context.Context) ([]metricdata.ScopeMetrics, error) { | ||||
| 			return []metricdata.ScopeMetrics{ | ||||
| 				{ | ||||
| 					Metrics: []metricdata.Metrics{ | ||||
| 						{ | ||||
| 							Name:        "exp_hist_metric", | ||||
| 							Description: "bad label", | ||||
| 							Data: metricdata.ExponentialHistogram[float64]{ | ||||
| 								DataPoints: []metricdata.ExponentialHistogramDataPoint[float64]{ | ||||
| 									{ | ||||
| 										Attributes:    attribute.NewSet(attribute.Key("$%^&").String("B")), | ||||
| 										Scale:         0, | ||||
| 										Count:         1, | ||||
| 										ZeroThreshold: 0, | ||||
| 									}, | ||||
| 								}, | ||||
| 							}, | ||||
| 						}, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, nil | ||||
| 		}) | ||||
| 	} | ||||
| 	// Helper to create a producer that emits an ExponentialHistogram with | ||||
| 	// inconsistent bucket counts vs total Count to trigger constructor error in addExponentialHistogramMetric. | ||||
| 	makeBadEHCountProducer := func() metric.Producer { | ||||
| 		return producerFunc(func(_ context.Context) ([]metricdata.ScopeMetrics, error) { | ||||
| 			return []metricdata.ScopeMetrics{ | ||||
| 				{ | ||||
| 					Metrics: []metricdata.Metrics{ | ||||
| 						{ | ||||
| 							Name: "exp_hist_metric_bad", | ||||
| 							Data: metricdata.ExponentialHistogram[float64]{ | ||||
| 								DataPoints: []metricdata.ExponentialHistogramDataPoint[float64]{ | ||||
| 									{ | ||||
| 										Scale:         0, | ||||
| 										Count:         0, | ||||
| 										ZeroThreshold: 0, | ||||
| 										PositiveBucket: metricdata.ExponentialBucket{ | ||||
| 											Offset: 0, | ||||
| 											Counts: []uint64{1}, | ||||
| 										}, | ||||
| 									}, | ||||
| 								}, | ||||
| 							}, | ||||
| 						}, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, nil | ||||
| 		}) | ||||
| 	} | ||||
|  | ||||
| 	testCases := []struct { | ||||
| 		name                string | ||||
| 		namespace           string | ||||
| 		counterName         string | ||||
| 		customScopeAttrs    []attribute.KeyValue | ||||
| 		customResourceAttrs []attribute.KeyValue | ||||
| 		labelName           string | ||||
| 		expectNewErr        string | ||||
| 		expectMetricErr     string | ||||
| 		checkMetricFamilies func(t testing.TB, dtos []*dto.MetricFamily) | ||||
| 		name                    string | ||||
| 		namespace               string | ||||
| 		counterName             string | ||||
| 		customScopeAttrs        []attribute.KeyValue | ||||
| 		customResourceAttrs     []attribute.KeyValue | ||||
| 		labelName               string | ||||
| 		producer                metric.Producer | ||||
| 		skipInstrument          bool | ||||
| 		record                  func(ctx context.Context, meter otelmetric.Meter) error | ||||
| 		expectNewErr            string | ||||
| 		expectMetricErr         string | ||||
| 		expectGatherErrContains string | ||||
| 		expectGatherErrIs       error | ||||
| 		checkMetricFamilies     func(t testing.TB, dtos []*dto.MetricFamily) | ||||
| 	}{ | ||||
| 		{ | ||||
| 			name:        "simple happy path", | ||||
| @@ -1814,6 +1967,29 @@ func TestEscapingErrorHandling(t *testing.T) { | ||||
| 			counterName:  "foo", | ||||
| 			expectNewErr: `normalization for label name "$%^&" resulted in invalid name "_"`, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "bad translated metric name via producer", | ||||
| 			// Use a producer to emit a metric with an invalid name to trigger getName error. | ||||
| 			producer:       makeBadNameProducer(), | ||||
| 			skipInstrument: true, | ||||
| 			// Error message comes from normalization in the translator; match on a stable substring. | ||||
| 			expectGatherErrContains: "normalization", | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				// target_info should still be exported. | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:        "good namespace, names should be escaped", | ||||
| 			namespace:   "my-strange-namespace", | ||||
| @@ -1845,9 +2021,23 @@ func TestEscapingErrorHandling(t *testing.T) { | ||||
| 			customScopeAttrs: []attribute.KeyValue{ | ||||
| 				attribute.Key("$%^&").String("B"), | ||||
| 			}, | ||||
| 			// With improved error handling, invalid scope label names result in an invalid metric | ||||
| 			// and Gather returns an error containing the normalization failure. | ||||
| 			expectGatherErrContains: `normalization for label name "$%^&" resulted in invalid name "_"`, | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.Len(t, mfs, 1) | ||||
| 				require.Equal(t, "target_info", mfs[0].GetName()) | ||||
| 				// target_info should still be exported; metric with bad scope label dropped. | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| @@ -1857,14 +2047,203 @@ func TestEscapingErrorHandling(t *testing.T) { | ||||
| 		}, | ||||
| 		{ | ||||
| 			// label names are not translated and therefore not checked until | ||||
| 			// collection time, and there is no place to catch and return this error. | ||||
| 			// Instead we drop the metric. | ||||
| 			name:        "bad translated label name", | ||||
| 			counterName: "foo", | ||||
| 			labelName:   "$%^&", | ||||
| 			// collection time; with improved error handling, we emit an invalid metric and | ||||
| 			// surface the error during Gather. | ||||
| 			name:                    "bad translated label name", | ||||
| 			counterName:             "foo", | ||||
| 			labelName:               "$%^&", | ||||
| 			expectGatherErrContains: `normalization for label name "$%^&" resulted in invalid name "_"`, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "unsupported data type via producer", | ||||
| 			// Use a producer to emit a Summary data point; no SDK instruments. | ||||
| 			producer:          makeSummaryProducer(), | ||||
| 			skipInstrument:    true, | ||||
| 			expectGatherErrIs: errInvalidMetricType, | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.Len(t, mfs, 1) | ||||
| 				require.Equal(t, "target_info", mfs[0].GetName()) | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:                    "bad exponential histogram label name via producer", | ||||
| 			producer:                makeBadEHProducer(), | ||||
| 			skipInstrument:          true, | ||||
| 			expectGatherErrContains: `normalization for label name "$%^&" resulted in invalid name "_"`, | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name:                    "exponential histogram constructor error via producer (count mismatch)", | ||||
| 			producer:                makeBadEHCountProducer(), | ||||
| 			skipInstrument:          true, | ||||
| 			expectGatherErrContains: "count", | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "sum constructor error via duplicate label name", | ||||
| 			record: func(ctx context.Context, meter otelmetric.Meter) error { | ||||
| 				c, err := meter.Int64Counter("sum_metric_dup") | ||||
| 				if err != nil { | ||||
| 					return err | ||||
| 				} | ||||
| 				// Duplicate variable label name with scope label to make Desc invalid. | ||||
| 				c.Add(ctx, 1, otelmetric.WithAttributes(attribute.String(scopeNameLabel, "x"))) | ||||
| 				return nil | ||||
| 			}, | ||||
| 			expectGatherErrContains: "duplicate label", | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "gauge constructor error via duplicate label name", | ||||
| 			record: func(ctx context.Context, meter otelmetric.Meter) error { | ||||
| 				g, err := meter.Float64Gauge("gauge_metric_dup") | ||||
| 				if err != nil { | ||||
| 					return err | ||||
| 				} | ||||
| 				g.Record(ctx, 1.0, otelmetric.WithAttributes(attribute.String(scopeNameLabel, "x"))) | ||||
| 				return nil | ||||
| 			}, | ||||
| 			expectGatherErrContains: "duplicate label", | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "histogram constructor error via duplicate label name", | ||||
| 			record: func(ctx context.Context, meter otelmetric.Meter) error { | ||||
| 				h, err := meter.Float64Histogram("hist_metric_dup") | ||||
| 				if err != nil { | ||||
| 					return err | ||||
| 				} | ||||
| 				h.Record(ctx, 1.23, otelmetric.WithAttributes(attribute.String(scopeNameLabel, "x"))) | ||||
| 				return nil | ||||
| 			}, | ||||
| 			expectGatherErrContains: "duplicate label", | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "bad gauge label name", | ||||
| 			record: func(ctx context.Context, meter otelmetric.Meter) error { | ||||
| 				g, err := meter.Float64Gauge("gauge_metric") | ||||
| 				if err != nil { | ||||
| 					return err | ||||
| 				} | ||||
| 				g.Record(ctx, 1, otelmetric.WithAttributes(attribute.Key("$%^&").String("B"))) | ||||
| 				return nil | ||||
| 			}, | ||||
| 			expectGatherErrContains: `normalization for label name "$%^&" resulted in invalid name "_"`, | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			name: "bad histogram label name", | ||||
| 			record: func(ctx context.Context, meter otelmetric.Meter) error { | ||||
| 				h, err := meter.Float64Histogram("hist_metric") | ||||
| 				if err != nil { | ||||
| 					return err | ||||
| 				} | ||||
| 				h.Record(ctx, 1.23, otelmetric.WithAttributes(attribute.Key("$%^&").String("B"))) | ||||
| 				return nil | ||||
| 			}, | ||||
| 			expectGatherErrContains: `normalization for label name "$%^&" resulted in invalid name "_"`, | ||||
| 			checkMetricFamilies: func(t testing.TB, mfs []*dto.MetricFamily) { | ||||
| 				require.NotEmpty(t, mfs) | ||||
| 				other := 0 | ||||
| 				seenTarget := false | ||||
| 				for _, mf := range mfs { | ||||
| 					if mf.GetName() == "target_info" { | ||||
| 						seenTarget = true | ||||
| 						continue | ||||
| 					} | ||||
| 					other++ | ||||
| 				} | ||||
| 				require.True(t, seenTarget) | ||||
| 				require.Equal(t, 0, other) | ||||
| 			}, | ||||
| 		}, | ||||
| 	} | ||||
| @@ -1881,49 +2260,70 @@ func TestEscapingErrorHandling(t *testing.T) { | ||||
| 			}) | ||||
| 			ctx = trace.ContextWithSpanContext(ctx, sc) | ||||
|  | ||||
| 			exporter, err := New( | ||||
| 			opts := []Option{ | ||||
| 				WithRegisterer(registry), | ||||
| 				WithTranslationStrategy(otlptranslator.UnderscoreEscapingWithSuffixes), | ||||
| 				WithNamespace(tc.namespace), | ||||
| 				WithResourceAsConstantLabels(attribute.NewDenyKeysFilter()), | ||||
| 			) | ||||
| 			} | ||||
| 			if tc.producer != nil { | ||||
| 				opts = append(opts, WithProducer(tc.producer)) | ||||
| 			} | ||||
| 			exporter, err := New(opts...) | ||||
| 			if tc.expectNewErr != "" { | ||||
| 				require.ErrorContains(t, err, tc.expectNewErr) | ||||
| 				return | ||||
| 			} | ||||
| 			require.NoError(t, err) | ||||
|  | ||||
| 			res, err := resource.New(ctx, | ||||
| 				resource.WithAttributes(semconv.ServiceName("prometheus_test")), | ||||
| 				resource.WithAttributes(semconv.TelemetrySDKVersion("latest")), | ||||
| 				resource.WithAttributes(tc.customResourceAttrs...), | ||||
| 			) | ||||
| 			require.NoError(t, err) | ||||
| 			provider := metric.NewMeterProvider( | ||||
| 				metric.WithReader(exporter), | ||||
| 				metric.WithResource(res), | ||||
| 			) | ||||
|  | ||||
| 			fooCounter, err := provider.Meter( | ||||
| 				"meterfoo", | ||||
| 				otelmetric.WithInstrumentationVersion("v0.1.0"), | ||||
| 				otelmetric.WithInstrumentationAttributes(tc.customScopeAttrs...), | ||||
| 			). | ||||
| 				Int64Counter( | ||||
| 					tc.counterName, | ||||
| 					otelmetric.WithUnit("s"), | ||||
| 					otelmetric.WithDescription(fmt.Sprintf(`meter %q counter`, tc.counterName))) | ||||
| 			if tc.expectMetricErr != "" { | ||||
| 				require.ErrorContains(t, err, tc.expectMetricErr) | ||||
| 			if !tc.skipInstrument { | ||||
| 				res, err := resource.New(ctx, | ||||
| 					resource.WithAttributes(semconv.ServiceName("prometheus_test")), | ||||
| 					resource.WithAttributes(semconv.TelemetrySDKVersion("latest")), | ||||
| 					resource.WithAttributes(tc.customResourceAttrs...), | ||||
| 				) | ||||
| 				require.NoError(t, err) | ||||
| 				provider := metric.NewMeterProvider( | ||||
| 					metric.WithReader(exporter), | ||||
| 					metric.WithResource(res), | ||||
| 				) | ||||
| 				meter := provider.Meter( | ||||
| 					"meterfoo", | ||||
| 					otelmetric.WithInstrumentationVersion("v0.1.0"), | ||||
| 					otelmetric.WithInstrumentationAttributes(tc.customScopeAttrs...), | ||||
| 				) | ||||
| 				if tc.record != nil { | ||||
| 					err := tc.record(ctx, meter) | ||||
| 					require.NoError(t, err) | ||||
| 				} else { | ||||
| 					fooCounter, err := meter.Int64Counter( | ||||
| 						tc.counterName, | ||||
| 						otelmetric.WithUnit("s"), | ||||
| 						otelmetric.WithDescription(fmt.Sprintf(`meter %q counter`, tc.counterName))) | ||||
| 					if tc.expectMetricErr != "" { | ||||
| 						require.ErrorContains(t, err, tc.expectMetricErr) | ||||
| 						return | ||||
| 					} | ||||
| 					require.NoError(t, err) | ||||
| 					var addOpts []otelmetric.AddOption | ||||
| 					if tc.labelName != "" { | ||||
| 						addOpts = append(addOpts, otelmetric.WithAttributes(attribute.String(tc.labelName, "foo"))) | ||||
| 					} | ||||
| 					fooCounter.Add(ctx, 100, addOpts...) | ||||
| 				} | ||||
| 			} else { | ||||
| 				// When skipping instruments, still register the reader so Collect will run. | ||||
| 				_ = metric.NewMeterProvider(metric.WithReader(exporter)) | ||||
| 			} | ||||
| 			got, err := registry.Gather() | ||||
| 			if tc.expectGatherErrContains != "" { | ||||
| 				require.Error(t, err) | ||||
| 				require.Contains(t, err.Error(), tc.expectGatherErrContains) | ||||
| 				return | ||||
| 			} | ||||
| 			require.NoError(t, err) | ||||
| 			var opts []otelmetric.AddOption | ||||
| 			if tc.labelName != "" { | ||||
| 				opts = append(opts, otelmetric.WithAttributes(attribute.String(tc.labelName, "foo"))) | ||||
| 			if tc.expectGatherErrIs != nil { | ||||
| 				require.ErrorIs(t, err, tc.expectGatherErrIs) | ||||
| 				return | ||||
| 			} | ||||
| 			fooCounter.Add(ctx, 100, opts...) | ||||
| 			got, err := registry.Gather() | ||||
| 			require.NoError(t, err) | ||||
| 			if tc.checkMetricFamilies != nil { | ||||
| 				tc.checkMetricFamilies(t, got) | ||||
|   | ||||
| @@ -13,6 +13,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= | ||||
| github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= | ||||
| github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= | ||||
| github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= | ||||
| github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= | ||||
| github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= | ||||
| github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= | ||||
| github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= | ||||
| github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= | ||||
|   | ||||
		Reference in New Issue
	
	Block a user