Describe the bug
When we start a span, we should make a copy of the attributes from the SpanBuilder, but instead we just use a reference to the Builder's collection. This means you can continue to add attributes via the Builder.
Steps to reproduce
Span.Builder spanBuilder = tracerSdk.spanBuilder(SPAN_NAME);
spanBuilder.setAttribute("attr1", "value1");
// this should make a copy of the attributes from the builder
RecordEventsReadableSpan span = (RecordEventsReadableSpan) spanBuilder.startSpan();
Map<String, AttributeValue> startingAttributes = span.toSpanData().getAttributes();
// add another link to the *builder*
spanBuilder.setAttribute("attr2", "value2");
// this number is correct
assertThat(startingAttributes.size()).isEqualTo(1);
// but, we actually added an attribute to the underlying collection, via the builder.
assertThat(span.toSpanData().getAttributes().size()).isEqualTo(1);
I'd be up for indeed making a copy of the attributes. Wondering what @bogdandrutu thinks about this (as it would mean more allocations ;) )
I discovered this while starting to work on getting rid of the SpanData allocations, so maybe it'll all balance out. :)
We can avoid an allocation by moving the initial list to the Span instead of copying it. When the Span is created we initialize the span attributes with the list (data structure for attributes) created in the builder, and we make the list in the builder null.
Most helpful comment
We can avoid an allocation by moving the initial list to the Span instead of copying it. When the Span is created we initialize the span attributes with the list (data structure for attributes) created in the builder, and we make the list in the builder null.