From 6e3d3448fc8d87b1430e441497fbbc9cd4ad4da2 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Thu, 11 Jun 2026 14:33:54 -0400 Subject: [PATCH] Add WithUnsafeAttributes to support no-copy dynamic metric API calls (#8251) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split from https://github.com/open-telemetry/opentelemetry-go/pull/8179 Part of https://github.com/open-telemetry/opentelemetry-go/issues/7743 This just defines the new WithUnsafeAttributes option, but converts it to a set, and leaves the optimization of that path for a follow-up. Benchmarks ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/metric cpu: Intel(R) Xeon(R) CPU @ 2.20GHz │ out.txt │ │ sec/op │ EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithAttributeSet-24 65.18n ± 25% EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithAttributes-24 61.96n ± 19% EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithUnsafeAttributes-24 199.9n ± 22% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithAttributeSet-24 231.6n ± 9% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithAttributes-24 288.0n ± 14% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithUnsafeAttributes-24 271.9n ± 6% EndToEndCounterAdd/NoFilter/Attributes/1/Naive/WithAttributes-24 352.7n ± 14% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithAttributeSet-24 68.73n ± 22% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithAttributes-24 61.04n ± 22% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithUnsafeAttributes-24 754.0n ± 25% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithAttributeSet-24 637.3n ± 21% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithAttributes-24 908.2n ± 28% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithUnsafeAttributes-24 901.7n ± 14% EndToEndCounterAdd/NoFilter/Attributes/5/Naive/WithAttributes-24 1.062µ ± 15% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithAttributeSet-24 53.82n ± 28% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithAttributes-24 65.82n ± 5% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithUnsafeAttributes-24 1.717µ ± 14% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithAttributeSet-24 1.183µ ± 13% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithAttributes-24 1.803µ ± 20% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithUnsafeAttributes-24 2.050µ ± 15% EndToEndCounterAdd/NoFilter/Attributes/10/Naive/WithAttributes-24 2.082µ ± 33% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithAttributeSet-24 204.1n ± 14% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithAttributes-24 214.5n ± 28% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithUnsafeAttributes-24 433.9n ± 18% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithAttributeSet-24 412.6n ± 10% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithAttributes-24 464.1n ± 18% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithUnsafeAttributes-24 471.4n ± 13% EndToEndCounterAdd/Filtered/Attributes/1/Naive/WithAttributes-24 558.5n ± 19% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithAttributeSet-24 772.4n ± 14% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithAttributes-24 748.7n ± 9% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithUnsafeAttributes-24 1.453µ ± 17% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithAttributeSet-24 1.501µ ± 19% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithAttributes-24 1.489µ ± 9% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithUnsafeAttributes-24 1.608µ ± 24% EndToEndCounterAdd/Filtered/Attributes/5/Naive/WithAttributes-24 1.671µ ± 14% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithAttributeSet-24 1.324µ ± 18% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithAttributes-24 1.463µ ± 17% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithUnsafeAttributes-24 2.833µ ± 33% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithAttributeSet-24 2.635µ ± 9% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithAttributes-24 3.125µ ± 14% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithUnsafeAttributes-24 2.685µ ± 24% EndToEndCounterAdd/Filtered/Attributes/10/Naive/WithAttributes-24 3.404µ ± 8% geomean 610.5n │ out.txt │ │ B/op │ EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithAttributeSet-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithAttributes-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithUnsafeAttributes-24 128.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithAttributeSet-24 88.00 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithAttributes-24 152.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithUnsafeAttributes-24 128.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Naive/WithAttributes-24 232.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithAttributeSet-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithAttributes-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithUnsafeAttributes-24 640.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithAttributeSet-24 344.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithAttributes-24 664.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithUnsafeAttributes-24 640.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Naive/WithAttributes-24 1000.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithAttributeSet-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithAttributes-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithUnsafeAttributes-24 1.375Ki ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithAttributeSet-24 729.0 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithAttributes-24 1.399Ki ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithUnsafeAttributes-24 1.376Ki ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Naive/WithAttributes-24 2.102Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithAttributeSet-24 64.00 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithAttributes-24 64.00 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithUnsafeAttributes-24 192.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithAttributeSet-24 152.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithAttributes-24 216.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithUnsafeAttributes-24 192.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Naive/WithAttributes-24 296.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithAttributeSet-24 576.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithAttributes-24 576.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithUnsafeAttributes-24 1.188Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithAttributeSet-24 921.0 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithAttributes-24 1.212Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithUnsafeAttributes-24 1.188Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Naive/WithAttributes-24 1.539Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithAttributeSet-24 1.312Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithAttributes-24 1.312Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithUnsafeAttributes-24 2.688Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithAttributeSet-24 2.025Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithAttributes-24 2.713Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithUnsafeAttributes-24 2.689Ki ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Naive/WithAttributes-24 3.414Ki ± 0% geomean ¹ ¹ summaries must be >0 to compute geomean │ out.txt │ │ allocs/op │ EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithAttributeSet-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithAttributes-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Precomputed/WithUnsafeAttributes-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithAttributeSet-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithAttributes-24 3.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Dynamic/WithUnsafeAttributes-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/1/Naive/WithAttributes-24 5.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithAttributeSet-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithAttributes-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Precomputed/WithUnsafeAttributes-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithAttributeSet-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithAttributes-24 3.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Dynamic/WithUnsafeAttributes-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/5/Naive/WithAttributes-24 5.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithAttributeSet-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithAttributes-24 0.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Precomputed/WithUnsafeAttributes-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithAttributeSet-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithAttributes-24 3.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Dynamic/WithUnsafeAttributes-24 2.000 ± 0% EndToEndCounterAdd/NoFilter/Attributes/10/Naive/WithAttributes-24 5.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithAttributeSet-24 1.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithAttributes-24 1.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Precomputed/WithUnsafeAttributes-24 3.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithAttributeSet-24 3.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithAttributes-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Dynamic/WithUnsafeAttributes-24 3.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/1/Naive/WithAttributes-24 6.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithAttributeSet-24 2.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithAttributes-24 2.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Precomputed/WithUnsafeAttributes-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithAttributeSet-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithAttributes-24 5.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Dynamic/WithUnsafeAttributes-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/5/Naive/WithAttributes-24 7.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithAttributeSet-24 2.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithAttributes-24 2.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Precomputed/WithUnsafeAttributes-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithAttributeSet-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithAttributes-24 5.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Dynamic/WithUnsafeAttributes-24 4.000 ± 0% EndToEndCounterAdd/Filtered/Attributes/10/Naive/WithAttributes-24 7.000 ± 0% geomean ¹ ¹ summaries must be >0 to compute geomean ``` --- CHANGELOG.md | 4 + metric/x/options.go | 33 +++ metric/x/options_test.go | 52 ++++ sdk/metric/benchmark_test.go | 39 +++ sdk/metric/instrument.go | 50 +++- sdk/metric/instrument_test.go | 440 ++++++++++++++++++++++++++++++++++ sdk/metric/meter.go | 14 +- 7 files changed, 624 insertions(+), 8 deletions(-) create mode 100644 metric/x/options_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 339d985e4..27f224729 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 `WithUnsafeAttributes` to `go.opentelemetry.io/otel/metric/x` as an experimental no-copy attribute option intended for future performance work. This is a work in progress. (#8251) + ### Fixed - Avoid preallocating scope attributes when they are disabled in `go.opentelemetry.io/otel/exporters/prometheus` . (#8404) diff --git a/metric/x/options.go b/metric/x/options.go index 7be062046..577f48106 100644 --- a/metric/x/options.go +++ b/metric/x/options.go @@ -28,6 +28,39 @@ func WithDefaultAttributes(keys ...attribute.Key) metric.InstrumentOption { return defaultAttributesOption{keys: keys} } +type unsafeAttributesOption struct { + metric.MeasurementOption + kvs []attribute.KeyValue +} + +// Experimental prevents the API from panicking when the option is used. +func (*unsafeAttributesOption) Experimental() {} + +// RawAttributes returns the raw key-values associated with the option. +func (o *unsafeAttributesOption) RawAttributes() []attribute.KeyValue { + return o.kvs +} + +// Settable implements the x.Settable interface. +// As with all Settable implementations, Set must not be called concurrently +// with the usage of the option in an instrument call (e.g., counter.Add) +// or by other goroutines. +func (o *unsafeAttributesOption) Set(kvs []attribute.KeyValue) { + o.kvs = kvs +} + +// WithUnsafeAttributes returns a metric.MeasurementOption that stores the raw attributes +// and associates them with a measurement without making a copy. +// The caller must not modify the attributes slice after passing it to this function. +// This is a work-in-progress, and does not yet have better performance than metric.WithAttributeSet. +// +// Attributes passed via WithUnsafeAttributes are merged with attributes from standard options +// (e.g., metric.WithAttributes). If duplicate keys exist, attributes from WithUnsafeAttributes +// take precedence and will override standard attributes, regardless of the order they are passed. +func WithUnsafeAttributes(kvs ...attribute.KeyValue) metric.MeasurementOption { + return &unsafeAttributesOption{kvs: kvs} +} + // Settable is an optional interface that Options can implement // to allow reuse without additional allocations. // diff --git a/metric/x/options_test.go b/metric/x/options_test.go new file mode 100644 index 000000000..0734b4c2e --- /dev/null +++ b/metric/x/options_test.go @@ -0,0 +1,52 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package x + +import ( + "testing" + + "go.opentelemetry.io/otel/attribute" +) + +func TestWithUnsafeAttributes(t *testing.T) { + kvs := []attribute.KeyValue{attribute.String("A", "B")} + opt := WithUnsafeAttributes(kvs...) + + unsafeOpt, ok := opt.(*unsafeAttributesOption) + if !ok { + t.Fatalf("expected *unsafeAttributesOption") + } + + attrs := unsafeOpt.RawAttributes() + if len(attrs) != 1 { + t.Errorf("expected 1 attribute, got %d", len(attrs)) + } + if attrs[0].Key != "A" || attrs[0].Value.AsString() != "B" { + t.Errorf("expected attribute A='B', got %v", attrs[0]) + } +} + +func TestUnsafeAttributesOption_Set(t *testing.T) { + opt := WithUnsafeAttributes() + + kvs := []attribute.KeyValue{attribute.String("C", "D")} + + settable, ok := opt.(Settable[[]attribute.KeyValue]) + if !ok { + t.Fatalf("expected Settable[[]attribute.KeyValue]") + } + settable.Set(kvs) + + unsafeOpt, ok := opt.(*unsafeAttributesOption) + if !ok { + t.Fatalf("expected *unsafeAttributesOption") + } + attrs := unsafeOpt.RawAttributes() + if len(attrs) != 1 { + t.Errorf("expected 1 attribute, got %d", len(attrs)) + } + if attrs[0].Key != "C" || attrs[0].Value.AsString() != "D" { + t.Errorf("expected attribute C='D', got %v", attrs[0]) + } +} diff --git a/sdk/metric/benchmark_test.go b/sdk/metric/benchmark_test.go index 7bc984a8f..f54956471 100644 --- a/sdk/metric/benchmark_test.go +++ b/sdk/metric/benchmark_test.go @@ -624,6 +624,16 @@ func BenchmarkEndToEndCounterAdd(b *testing.B) { } }) }) + b.Run("Precomputed/WithUnsafeAttributes", func(b *testing.B) { + counter := testCounter(b, mp.provider()) + precomputedOpts := []metric.AddOption{x.WithUnsafeAttributes(attributes(attrsLen)...)} + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + counter.Add(ctx, 1, precomputedOpts...) + } + }) + }) // This case shows the performance of our API + SDK when // following our contributor guidance for recording // varying attributes by passing attribute.Set: @@ -716,6 +726,35 @@ func BenchmarkEndToEndCounterAdd(b *testing.B) { } }) }) + b.Run("Dynamic/WithUnsafeAttributes", func(b *testing.B) { + counter := testCounter(b, mp.provider()) + b.ReportAllocs() + b.RunParallel(func(pb *testing.PB) { + opt := x.WithUnsafeAttributes() + for pb.Next() { + func() { + attrsSlice := attrPool.Get().(*[]attribute.KeyValue) + defer func() { + *attrsSlice = (*attrsSlice)[:0] // Reset. + attrPool.Put(attrsSlice) + }() + *attrsSlice = appendAttributes(*attrsSlice, attrsLen) + addOpt := addOptPool.Get().(*[]metric.AddOption) + defer func() { + *addOpt = (*addOpt)[:0] + addOptPool.Put(addOpt) + }() + if settable, ok := opt.(x.Settable[[]attribute.KeyValue]); ok { + settable.Set(*attrsSlice) + } else { + opt = x.WithUnsafeAttributes(*attrsSlice...) + } + *addOpt = append(*addOpt, opt) + counter.Add(ctx, 1, *addOpt...) + }() + } + }) + }) // This case shows the performance of our API + SDK when // users use it in the "obvious" way, without explicitly // trying to optimize for performance. diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index dfb4964d9..284e5a947 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -180,6 +180,44 @@ func (i instID) normalize() instID { return i } +type rawAttributesOption interface { + RawAttributes() []attribute.KeyValue + Experimental() +} + +func extractRawKVs[T any](opts []T) []attribute.KeyValue { + var rawKVs []attribute.KeyValue + var count int + for _, opt := range opts { + if r, ok := any(opt).(rawAttributesOption); ok { + count++ + if count == 1 { + rawKVs = r.RawAttributes() + } else { + if count == 2 { + // Create a new slice to avoid modifying the original slice from the first option. + rawKVs = append([]attribute.KeyValue(nil), rawKVs...) + } + rawKVs = append(rawKVs, r.RawAttributes()...) + } + } + } + return rawKVs +} + +func resolveAttributes(configAttrs attribute.Set, rawKVs []attribute.KeyValue) attribute.Set { + if len(rawKVs) == 0 { + return configAttrs + } + merged := make([]attribute.KeyValue, 0, configAttrs.Len()+len(rawKVs)) + merged = append(merged, configAttrs.ToSlice()...) + // rawKVs are appended after configAttrs, meaning they will override any duplicate keys in configAttrs. + // This behavior is documented in WithUnsafeAttributes. + merged = append(merged, rawKVs...) + // TODO(#7743): Defer computing the full attribute.NewSet. + return attribute.NewSet(merged...) +} + type int64Inst struct { measures []aggregate.Measure[int64] @@ -198,12 +236,14 @@ var ( func (i *int64Inst) Add(ctx context.Context, val int64, opts ...metric.AddOption) { c := metric.NewAddConfig(opts) - i.aggregate(ctx, val, c.Attributes()) + rawKVs := extractRawKVs(opts) + i.aggregate(ctx, val, resolveAttributes(c.Attributes(), rawKVs)) } func (i *int64Inst) Record(ctx context.Context, val int64, opts ...metric.RecordOption) { c := metric.NewRecordConfig(opts) - i.aggregate(ctx, val, c.Attributes()) + rawKVs := extractRawKVs(opts) + i.aggregate(ctx, val, resolveAttributes(c.Attributes(), rawKVs)) } func (i *int64Inst) Enabled(context.Context) bool { @@ -238,12 +278,14 @@ var ( func (i *float64Inst) Add(ctx context.Context, val float64, opts ...metric.AddOption) { c := metric.NewAddConfig(opts) - i.aggregate(ctx, val, c.Attributes()) + rawKVs := extractRawKVs(opts) + i.aggregate(ctx, val, resolveAttributes(c.Attributes(), rawKVs)) } func (i *float64Inst) Record(ctx context.Context, val float64, opts ...metric.RecordOption) { c := metric.NewRecordConfig(opts) - i.aggregate(ctx, val, c.Attributes()) + rawKVs := extractRawKVs(opts) + i.aggregate(ctx, val, resolveAttributes(c.Attributes(), rawKVs)) } func (i *float64Inst) Enabled(context.Context) bool { diff --git a/sdk/metric/instrument_test.go b/sdk/metric/instrument_test.go index 52d39b753..b6150b1dc 100644 --- a/sdk/metric/instrument_test.go +++ b/sdk/metric/instrument_test.go @@ -4,11 +4,17 @@ package metric import ( + "context" "testing" + "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/x" "go.opentelemetry.io/otel/sdk/metric/internal/aggregate" "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" ) func BenchmarkInstrument(b *testing.B) { @@ -74,3 +80,437 @@ func BenchmarkInstrument(b *testing.B) { } }) } + +func TestExtractRawKVs(t *testing.T) { + k1 := attribute.String("k1", "v1") + k2 := attribute.String("k2", "v2") + k3 := attribute.String("k3", "v3") + + tests := []struct { + name string + opts []metric.AddOption + want []attribute.KeyValue + }{ + { + name: "Empty", + opts: nil, + want: nil, + }, + { + name: "NoRawAttributes", + opts: []metric.AddOption{metric.WithAttributes(k1)}, + want: nil, + }, + { + name: "OneRawAttributes", + opts: []metric.AddOption{x.WithUnsafeAttributes(k1, k2)}, + want: []attribute.KeyValue{k1, k2}, + }, + { + name: "MultipleRawAttributes", + opts: []metric.AddOption{ + x.WithUnsafeAttributes(k1), + x.WithUnsafeAttributes(k2, k3), + }, + want: []attribute.KeyValue{k1, k2, k3}, + }, + { + name: "Mixed", + opts: []metric.AddOption{ + x.WithUnsafeAttributes(k1), + metric.WithAttributes(k2), + x.WithUnsafeAttributes(k3), + }, + want: []attribute.KeyValue{k1, k3}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractRawKVs(tt.opts) + require.Equal(t, tt.want, got) + }) + } +} + +func TestExtractRawKVs_NoModifyOriginalSlice(t *testing.T) { + k1 := attribute.String("k1", "v1") + k2 := attribute.String("k2", "v2") + + s1 := []attribute.KeyValue{k1} + // Give it some extra capacity to ensure append would overwrite if it reused the backing array. + s1 = append(s1, attribute.KeyValue{})[:1] + + opt1 := x.WithUnsafeAttributes(s1...) + opt2 := x.WithUnsafeAttributes(k2) + + opts := []metric.AddOption{opt1, opt2} + + got := extractRawKVs(opts) + require.Equal(t, []attribute.KeyValue{k1, k2}, got) + + // Check that s1 was not modified. + require.Equal(t, []attribute.KeyValue{k1}, s1[:1]) + fullS1 := s1[:cap(s1)] + if len(fullS1) > 1 { + require.NotEqual(t, k2, fullS1[1], "Original slice was modified by extractRawKVs") + } +} + +func TestResolveAttributes(t *testing.T) { + k1 := attribute.String("k1", "v1") + k2 := attribute.String("k2", "v2") + k3 := attribute.String("k3", "v3") + k1Alt := attribute.String("k1", "v1_alt") + + tests := []struct { + name string + configAttrs attribute.Set + rawKVs []attribute.KeyValue + want attribute.Set + }{ + { + name: "Empty", + configAttrs: *attribute.EmptySet(), + rawKVs: nil, + want: *attribute.EmptySet(), + }, + { + name: "OnlyConfig", + configAttrs: attribute.NewSet(k1, k2), + rawKVs: nil, + want: attribute.NewSet(k1, k2), + }, + { + name: "OnlyRaw", + configAttrs: *attribute.EmptySet(), + rawKVs: []attribute.KeyValue{k1, k2}, + want: attribute.NewSet(k1, k2), + }, + { + name: "MergeNoOverlap", + configAttrs: attribute.NewSet(k1), + rawKVs: []attribute.KeyValue{k2, k3}, + want: attribute.NewSet(k1, k2, k3), + }, + { + name: "MergeWithOverlap_RawOverrides", + configAttrs: attribute.NewSet(k1, k2), + rawKVs: []attribute.KeyValue{k1Alt, k3}, + want: attribute.NewSet(k1Alt, k2, k3), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveAttributes(tt.configAttrs, tt.rawKVs) + require.Equal(t, tt.want, got) + }) + } +} + +func TestMeterWithUnsafeAttributes(t *testing.T) { + k1 := attribute.Key("k1") + k2 := attribute.Key("k2") + + combined := attribute.NewSet(k1.String("alice"), k2.String("bob")) + + testCases := []struct { + name string + instName string + record func(t *testing.T, m metric.Meter) + wantData func(attrs attribute.Set) metricdata.Aggregation + }{ + { + name: "Int64Counter", + instName: "sint", + record: func(t *testing.T, m metric.Meter) { + ctr, err := m.Int64Counter("sint") + require.NoError(t, err) + ctr.Add(t.Context(), 3, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 3}}, + } + }, + }, + { + name: "Float64Counter", + instName: "sfloat", + record: func(t *testing.T, m metric.Meter) { + ctr, err := m.Float64Counter("sfloat") + require.NoError(t, err) + ctr.Add(t.Context(), 3.5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[float64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[float64]{{Attributes: attrs, Value: 3.5}}, + } + }, + }, + { + name: "Int64ObservableCounter", + instName: "aint", + record: func(t *testing.T, m metric.Meter) { + _, err := m.Int64ObservableCounter( + "aint", + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(4, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + return nil + }), + ) + require.NoError(t, err) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 4}}, + } + }, + }, + { + name: "Float64ObservableCounter", + instName: "afloat", + record: func(t *testing.T, m metric.Meter) { + _, err := m.Float64ObservableCounter( + "afloat", + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(4.5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + return nil + }), + ) + require.NoError(t, err) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[float64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: true, + DataPoints: []metricdata.DataPoint[float64]{{Attributes: attrs, Value: 4.5}}, + } + }, + }, + { + name: "Int64UpDownCounter", + instName: "sudint", + record: func(t *testing.T, m metric.Meter) { + ctr, err := m.Int64UpDownCounter("sudint") + require.NoError(t, err) + ctr.Add(t.Context(), -3, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: false, + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: -3}}, + } + }, + }, + { + name: "Float64UpDownCounter", + instName: "sudfloat", + record: func(t *testing.T, m metric.Meter) { + ctr, err := m.Float64UpDownCounter("sudfloat") + require.NoError(t, err) + ctr.Add(t.Context(), -3.5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[float64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: false, + DataPoints: []metricdata.DataPoint[float64]{{Attributes: attrs, Value: -3.5}}, + } + }, + }, + { + name: "Int64Histogram", + instName: "shist", + record: func(t *testing.T, m metric.Meter) { + hist, err := m.Int64Histogram("shist") + require.NoError(t, err) + hist.Record(t.Context(), 5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Histogram[int64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[int64]{ + { + Attributes: attrs, + Count: 1, + Sum: 5, + Bounds: []float64{ + 0, + 5, + 10, + 25, + 50, + 75, + 100, + 250, + 500, + 750, + 1000, + 2500, + 5000, + 7500, + 10000, + }, + BucketCounts: []uint64{0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + Min: metricdata.NewExtrema[int64](5), + Max: metricdata.NewExtrema[int64](5), + }, + }, + } + }, + }, + { + name: "Float64Histogram", + instName: "sfhist", + record: func(t *testing.T, m metric.Meter) { + hist, err := m.Float64Histogram("sfhist") + require.NoError(t, err) + hist.Record(t.Context(), 5.5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Histogram[float64]{ + Temporality: metricdata.CumulativeTemporality, + DataPoints: []metricdata.HistogramDataPoint[float64]{ + { + Attributes: attrs, + Count: 1, + Sum: 5.5, + Bounds: []float64{ + 0, + 5, + 10, + 25, + 50, + 75, + 100, + 250, + 500, + 750, + 1000, + 2500, + 5000, + 7500, + 10000, + }, + BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + Min: metricdata.NewExtrema[float64](5.5), + Max: metricdata.NewExtrema[float64](5.5), + }, + }, + } + }, + }, + { + name: "Int64ObservableUpDownCounter", + instName: "audint", + record: func(t *testing.T, m metric.Meter) { + _, err := m.Int64ObservableUpDownCounter( + "audint", + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(-4, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + return nil + }), + ) + require.NoError(t, err) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[int64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: false, + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: -4}}, + } + }, + }, + { + name: "Float64ObservableUpDownCounter", + instName: "audfloat", + record: func(t *testing.T, m metric.Meter) { + _, err := m.Float64ObservableUpDownCounter( + "audfloat", + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(-4.5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + return nil + }), + ) + require.NoError(t, err) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Sum[float64]{ + Temporality: metricdata.CumulativeTemporality, + IsMonotonic: false, + DataPoints: []metricdata.DataPoint[float64]{{Attributes: attrs, Value: -4.5}}, + } + }, + }, + { + name: "Int64ObservableGauge", + instName: "agint", + record: func(t *testing.T, m metric.Meter) { + _, err := m.Int64ObservableGauge( + "agint", + metric.WithInt64Callback(func(_ context.Context, o metric.Int64Observer) error { + o.Observe(10, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + return nil + }), + ) + require.NoError(t, err) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Gauge[int64]{ + DataPoints: []metricdata.DataPoint[int64]{{Attributes: attrs, Value: 10}}, + } + }, + }, + { + name: "Float64ObservableGauge", + instName: "agfloat", + record: func(t *testing.T, m metric.Meter) { + _, err := m.Float64ObservableGauge( + "agfloat", + metric.WithFloat64Callback(func(_ context.Context, o metric.Float64Observer) error { + o.Observe(10.5, x.WithUnsafeAttributes(k1.String("alice"), k2.String("bob"))) + return nil + }), + ) + require.NoError(t, err) + }, + wantData: func(attrs attribute.Set) metricdata.Aggregation { + return metricdata.Gauge[float64]{ + DataPoints: []metricdata.DataPoint[float64]{{Attributes: attrs, Value: 10.5}}, + } + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + rdr := NewManualReader() + m := NewMeterProvider(WithReader(rdr)).Meter("test") + tt.record(t, m) + + rm := metricdata.ResourceMetrics{} + err := rdr.Collect(t.Context(), &rm) + require.NoError(t, err) + + require.Len(t, rm.ScopeMetrics, 1) + sm := rm.ScopeMetrics[0] + require.Len(t, sm.Metrics, 1) + got := sm.Metrics[0] + + want := metricdata.Metrics{ + Name: tt.instName, + Data: tt.wantData(combined), + } + metricdatatest.AssertEqual(t, want, got, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars()) + }) + } +} diff --git a/sdk/metric/meter.go b/sdk/metric/meter.go index e9a2d5957..0ab283cc0 100644 --- a/sdk/metric/meter.go +++ b/sdk/metric/meter.go @@ -593,11 +593,13 @@ func (r observer) ObserveFloat64(o metric.Float64Observable, v float64, opts ... return } c := metric.NewObserveConfig(opts) + rawKVs := extractRawKVs(opts) + set := resolveAttributes(c.Attributes(), rawKVs) // Access to r.pipe.float64Measure is already guarded by a lock in pipeline.produce. // TODO (#5946): Refactor pipeline and observable measures. measures := r.pipe.float64Measures[oImpl.observableID] for _, m := range measures { - m(context.Background(), v, c.Attributes()) + m(context.Background(), v, set) } } @@ -624,11 +626,13 @@ func (r observer) ObserveInt64(o metric.Int64Observable, v int64, opts ...metric return } c := metric.NewObserveConfig(opts) + rawKVs := extractRawKVs(opts) + set := resolveAttributes(c.Attributes(), rawKVs) // Access to r.pipe.int64Measures is already guarded b a lock in pipeline.produce. // TODO (#5946): Refactor pipeline and observable measures. measures := r.pipe.int64Measures[oImpl.observableID] for _, m := range measures { - m(context.Background(), v, c.Attributes()) + m(context.Background(), v, set) } } @@ -793,7 +797,8 @@ type int64Observer struct { func (o int64Observer) Observe(val int64, opts ...metric.ObserveOption) { c := metric.NewObserveConfig(opts) - o.observe(val, c.Attributes()) + rawKVs := extractRawKVs(opts) + o.observe(val, resolveAttributes(c.Attributes(), rawKVs)) } type float64Observer struct { @@ -803,7 +808,8 @@ type float64Observer struct { func (o float64Observer) Observe(val float64, opts ...metric.ObserveOption) { c := metric.NewObserveConfig(opts) - o.observe(val, c.Attributes()) + rawKVs := extractRawKVs(opts) + o.observe(val, resolveAttributes(c.Attributes(), rawKVs)) } func defaultAttributes[T any](opts []T) []attribute.Key {