Use standard ArrayList size rather than max number of links for initial span links allocation

[johnbley]

Most spans that have links don't grow to contain the maximum number of links [Citation needed]. So, when faulting in a links list to hold them, there's no need to assume we'll need memory to hold the maximum.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.88%. Comparing base (36bc703) to head (90713a0).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6252      +/-   ##
============================================
+ Coverage     90.86%   90.88%   +0.02%     
- Complexity     6030     6033       +3     
============================================
  Files           651      651              
  Lines         17728    17728              
  Branches       1777     1777              
============================================
+ Hits          16108    16112       +4     
+ Misses         1105     1103       -2     
+ Partials        515      513       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[breedx-splk]

I'm curious to see this benchmarked against an implementation that uses LinkedList, especially in cases for:

• no links (min)
• 11 links
• 128 links (max)

[johnbley]

• no links (min)

For no links, this is faulted in, so any data structure's null is equally performant. I'm not sure I'd care to guess about the numerical distribution of any particular telemetry structure, much less tune to it. I'm pretty certain, however, that most spans don't reach the max number of links.

[jkwatson]

I'd like to see before/after benchmarks for a variety of cases before changing this.

[trask]

this seems the right thing to do for me even without benchmarks

[jack-berg]

this seems the right thing to do for me even without benchmarks

I agree

[breedx-splk]

I see where @trask and @jack-berg are coming from, and I don't want my ask to be a blocker. This is still probably a net improvement. I'm just a little hung up on the idea that a certain use case (messaging?) could generate lots of spans with ~11 links, which then requires one allocation for the first link and then another for the grow at 10. 🤷🏻

[johnbley]

generate lots of spans with ~11 links

Fair question. The maximal case I can think of is a lambda triggered by an sqs message, with xray links turned on, and somehow the http-level invocation also carries b3, jaeger, and opentracing headers configured to be attached as links, which doesn't get anywhere near 11 links. I grepped for addLinks and SpanLinksBuilder usage in opentelemetry-java-instrumentation and didn't see anything that operated in a loop or in batch.

[laurit]

I also think this is a reasonable change. As for using a linked list, here is what Josh Block has to say about it https://twitter.com/joshbloch/status/583813919019573248?lang=en

[jkwatson]

seems fine. a few bytes saved per span seems like an ok incremental improvement.