sdk/metric: Retain advisory attributes when View has no AttributeFilter

Fixes #8857.

According to the OpenTelemetry Metrics SDK specification, if attribute_keys
is not provided on a View, the SDK should use the 'Attributes' advisory parameter
configured on the instrument. Previously, when a View matched an instrument
without setting an AttributeFilter, the stream's AttributeFilter remained nil,
causing the pipeline to drop the instrument's advisory default attributes.

Centralize AttributeFilter defaulting inside cachedAggregator alongside
aggregation and exemplar reservoir provider defaulting.

Author: dashpole

CHANGELOG.md

@@ -25,6 +25,7 @@ The next release will require at least [Go 1.26].
 
 ### Fixed
 
+- Retain instrument advisory attributes (`metric/x.WithDefaultAttributes`) when a matching View does not specify an attribute filter in `go.opentelemetry.io/otel/sdk/metric`.
 - Accept quoted finite double values in OTLP/HTTP JSON requests in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#8831)
 - Export dropped attribute counts in OTLP log records from `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#8829)
 - Name span events created from OpenTracing logs after the `event` log field, falling back to `log`, instead of always using an empty name in `go.opentelemetry.io/otel/bridge/opentracing`. (#8648)

sdk/metric/meter_test.go

@@ -3000,9 +3000,37 @@ func TestMeterDefaultAttributes(t *testing.T) {
 			metricdatatest.AssertEqual(t, want, got, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars())
 		})
 
-		t.Run(tt.name+"_ViewOverride", func(t *testing.T) {
+		t.Run(tt.name+"_ViewWithoutFilter", func(t *testing.T) {
 			rdr := NewManualReader()
-			view := NewView(Instrument{Name: "*"}, Stream{}) // Match all instruments, override filter
+			view := NewView(
+				Instrument{Name: "*"},
+				Stream{Description: "updated"},
+			) // Match all instruments, no attribute filter
+			m := NewMeterProvider(WithReader(rdr), WithView(view)).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,
+				Description: "updated",
+				Data:        tt.wantData(alice),
+			}
+			metricdatatest.AssertEqual(t, want, got, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars())
+		})
+
+		t.Run(tt.name+"_ViewWithFilterOverride", func(t *testing.T) {
+			rdr := NewManualReader()
+			view := NewView(Instrument{Name: "*"}, Stream{
+				AttributeFilter: attribute.NewAllowKeysFilter(k1, k2),
+			}) // Match all instruments, override filter
 			m := NewMeterProvider(WithReader(rdr), WithView(view)).Meter("test")
 			tt.record(t, m)
 

sdk/metric/pipeline.go

@@ -255,7 +255,7 @@ func (i *inserter[N]) Instrument(
 			continue
 		}
 		matched = true
-		in, id, e := i.cachedAggregator(inst.Scope, inst.Kind, stream, readerAggregation)
+		in, id, e := i.cachedAggregator(inst.Scope, inst.Kind, stream, allowedKeys, readerAggregation)
 		if e != nil {
 			err = errors.Join(err, e)
 		}
@@ -284,13 +284,7 @@ func (i *inserter[N]) Instrument(
 		Description: inst.Description,
 		Unit:        inst.Unit,
 	}
-	// allowedKeys == nil indicates that the WithDefaultAttributes option was not passed,
-	// and all keys are allowed. An empty (non-nil) slice indicates that the option was passed
-	// with an empty set of keys, and no keys are allowed.
-	if allowedKeys != nil {
-		stream.AttributeFilter = attribute.NewAllowKeysFilter(allowedKeys...)
-	}
-	in, _, e := i.cachedAggregator(inst.Scope, inst.Kind, stream, readerAggregation)
+	in, _, e := i.cachedAggregator(inst.Scope, inst.Kind, stream, allowedKeys, readerAggregation)
 	if e != nil {
 		if err == nil {
 			err = errCreatingAggregators
@@ -364,6 +358,7 @@ func (i *inserter[N]) cachedAggregator(
 	scope instrumentation.Scope,
 	kind InstrumentKind,
 	stream Stream,
+	allowedKeys []attribute.Key,
 	readerAggregation Aggregation,
 ) (meas aggregate.Measure[N], aggID uint64, err error) {
 	switch stream.Aggregation.(type) {
@@ -378,6 +373,14 @@ func (i *inserter[N]) cachedAggregator(
 	if stream.ExemplarReservoirProviderSelector == nil {
 		stream.ExemplarReservoirProviderSelector = DefaultExemplarReservoirProviderSelector
 	}
+	// If attribute_keys is not provided on the view (AttributeFilter == nil),
+	// use the advisory Attributes configured on the instrument (allowedKeys).
+	// allowedKeys == nil indicates that the WithDefaultAttributes option was not passed,
+	// and all keys are allowed. An empty (non-nil) slice indicates that the option was passed
+	// with an empty set of keys, and no keys are allowed.
+	if stream.AttributeFilter == nil && allowedKeys != nil {
+		stream.AttributeFilter = attribute.NewAllowKeysFilter(allowedKeys...)
+	}
 
 	if err := isAggregatorCompatible(kind, stream.Aggregation); err != nil {
 		return nil, 0, fmt.Errorf(

sdk/metric/pipeline_test.go

@@ -380,7 +380,7 @@ func TestInserterCachedAggregatorNameConflict(t *testing.T) {
 	i := newInserter[int64](pipe, &vc)
 
 	readerAggregation := i.readerDefaultAggregation(kind)
-	_, origID, err := i.cachedAggregator(scope, kind, stream, readerAggregation)
+	_, origID, err := i.cachedAggregator(scope, kind, stream, nil, readerAggregation)
 	require.NoError(t, err)
 
 	require.Len(t, pipe.aggregations, 1)
@@ -390,7 +390,7 @@ func TestInserterCachedAggregatorNameConflict(t *testing.T) {
 	require.Equal(t, name, iSync[0].name)
 
 	stream.Name = "RequestCount"
-	_, id, err := i.cachedAggregator(scope, kind, stream, readerAggregation)
+	_, id, err := i.cachedAggregator(scope, kind, stream, nil, readerAggregation)
 	require.NoError(t, err)
 	assert.Equal(t, origID, id, "multiple aggregators for equivalent name")