diff --git a/otel/otel.go b/otel/otel.go index 76a9de5b..499c56ab 100644 --- a/otel/otel.go +++ b/otel/otel.go @@ -171,18 +171,7 @@ func BuildTracerProvider(opts *Opts) (*sdktrace.TracerProvider, func()) { func InitGlobalMeterProvider(opts *Opts) (func(), error) { logExporterEndpoint("metrics") exp, err := otlpmetrichttp.New(context.Background(), - otlpmetrichttp.WithTemporalitySelector(func(kind sdkmetric.InstrumentKind) metricdata.Temporality { - switch kind { - case - sdkmetric.InstrumentKindCounter, - sdkmetric.InstrumentKindUpDownCounter, - sdkmetric.InstrumentKindObservableCounter, - sdkmetric.InstrumentKindObservableUpDownCounter: - return metricdata.DeltaTemporality - default: - return metricdata.CumulativeTemporality - } - }), + otlpmetrichttp.WithTemporalitySelector(deltaTemporality), ) if err != nil { return nil, err @@ -202,3 +191,24 @@ func InitGlobalMeterProvider(opts *Opts) (func(), error) { } }, nil } + +// deltaTemporality exports every instrument kind with delta temporality, +// including histograms. +// +// Delta is what lets the ops collector aggregate an attribute away. Stripping +// a label from a cumulative stream merges independent monotonic series whose +// resets are interleaved, which corrupts rate() silently rather than failing; +// delta datapoints just sum. The ops collector relies on this to drop +// route.id/instance.id/host.name from proxy.session.goodput, whose ~10.8k +// distinct host.name values were driving the histogram to ~55M series (see +// getlantern/engineering#3831). +// +// Temporality is chosen per instrument KIND at the exporter, so this covers +// every histogram this binary emits, not just goodput. The only other one with +// readers is proxy_http_ping_request_duration_seconds, read as a p90 over +// .bucket by the host_metrics and track_performance_by_volume dashboards; a +// bucket quantile is computed over per-bucket rates and reads correctly on +// either temporality. +func deltaTemporality(sdkmetric.InstrumentKind) metricdata.Temporality { + return metricdata.DeltaTemporality +} diff --git a/otel/otel_test.go b/otel/otel_test.go new file mode 100644 index 00000000..336cc66c --- /dev/null +++ b/otel/otel_test.go @@ -0,0 +1,36 @@ +package otel + +import ( + "testing" + + "github.com/stretchr/testify/assert" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +// TestDeltaTemporalityCoversHistograms guards the exporter's temporality +// selection. +// +// InstrumentKindHistogram is the one that matters: it was cumulative while +// counters were already delta, and it is the kind proxy.session.goodput uses. +// Delta is required for the ops collector to strip +// route.id/instance.id/host.name from that metric — aggregating an identifier +// away merges the matching series, and merging cumulative streams interleaves +// their resets, corrupting rate() with no error. lantern-cloud also reads the +// paired .count stream with timeAggregation=sum, which is only correct for +// delta (increase vs sum on the wrong temporality is a ~500x error). +func TestDeltaTemporalityCoversHistograms(t *testing.T) { + kinds := []sdkmetric.InstrumentKind{ + sdkmetric.InstrumentKindCounter, + sdkmetric.InstrumentKindUpDownCounter, + sdkmetric.InstrumentKindHistogram, + sdkmetric.InstrumentKindGauge, + sdkmetric.InstrumentKindObservableCounter, + sdkmetric.InstrumentKindObservableUpDownCounter, + sdkmetric.InstrumentKindObservableGauge, + } + for _, k := range kinds { + assert.Equal(t, metricdata.DeltaTemporality, deltaTemporality(k), + "instrument kind %v must export delta", k) + } +}