Span attribute drop logic does not comply with the specification.

[MrAlias]

Description

Currently the trace SDK drops attributes based on a least-recently-used algorithm: the most recently unique attributes added are kept and the oldest not updated attributes are dropped when capacity is reached. However, the specification states:

for each unique attributes key, addition of which would result in exceeding the limit, SDK MUST discard that key/value pair

Meaning if a user adds an attribute after capacity is reached that attribute (if it has a unique key) should be dropped. This is not the behavior currently implemented.

Environment

• opentelemetry-go version: v1.3.0

Steps To Reproduce

package trace // import "go.opentelemetry.io/otel/sdk/trace"

import (
	"context"
	"testing"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
	"go.opentelemetry.io/otel/attribute"
)

func TestSpanAttributeCapacityDropOrder(t *testing.T) {
	// The tracing specification states:
	//
	//   For each unique attribute key, addition of which would result in
	//   exceeding the limit, SDK MUST discard that key/value pair
	//
	// Therefore, adding attributes after the capacity is reached should
	// result in those attributes being dropped.

	const (
		instName = "TestSpanAttributeCapacityDropOrder"
		spanName = "test span"
	)

	te := NewTestExporter()
	tp := NewTracerProvider(
		WithSyncer(te),
		WithSpanLimits(SpanLimits{AttributeCountLimit: 1}),
	)
	attrs := []attribute.KeyValue{
		attribute.String("key1", "value1"),
		attribute.String("key2", "value2"),
	}

	_, span := tp.Tracer(instName).Start(context.Background(), spanName)
	span.SetAttributes(attrs[0])
	// These should be dropped based on limits.
	span.SetAttributes(attrs[1])
	span.SetAttributes(attrs...)
	span.End()

	got, ok := te.GetSpan(spanName)
	require.Truef(t, ok, "span %s not exported", spanName)
	assert.Equal(t, attrs[:1], got.attributes)
}

Expected behavior

The included test should pass and the first attributes a user passes should be preserved.

[jmacd]

I think this behavior snuck in from the OpenCensus-Go library. I'd be glad to see it go! 😁

[jmacd]

See also open-telemetry/opentelemetry-specification#2248

[MrAlias]

See also open-telemetry/opentelemetry-specification#2248

Right. We are compliant with this currently (1), but it is important to consider this when resolving this.