Opentelemetry-java: You can add attributes to a started span via the builder that created it

Created on 12 May 2020  路  3Comments  路  Source: open-telemetry/opentelemetry-java

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);
Bug

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.

All 3 comments

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

z1c0 picture z1c0  路  3Comments

arminru picture arminru  路  5Comments

thisthat picture thisthat  路  5Comments

Oberon00 picture Oberon00  路  3Comments

jkwatson picture jkwatson  路  3Comments