Allow O(1) get operations for SpanBuilder::attributes [breaking]

[LukeMathWalker]

Closes #794

It changes the type of SpanBuilder::attributes from Vec<KeyValue> to OrderMap. OrderMap is a thin wrapper around index_map::IndexMap that only exposes the methods that preserve the original insertion order, which is what we care about later in the pipeline when enforcing the maximum number of attributes per span.

[codecov]

Codecov Report

Merging #799 (e39fdd4) into main (7534891) will increase coverage by 0.0%.
The diff coverage is 65.2%.

@@          Coverage Diff          @@
##            main    #799   +/-   ##
=====================================
  Coverage   69.7%   69.8%           
=====================================
  Files        109     110    +1     
  Lines       9045    9060   +15     
=====================================
+ Hits        6313    6324   +11     
- Misses      2732    2736    +4     

Impacted Files	Coverage Δ
opentelemetry-api/src/trace/mod.rs	72.0% <ø> (ø)
opentelemetry-api/src/trace/tracer.rs	61.3% <50.0%> (ø)
opentelemetry-sdk/src/trace/tracer.rs	90.6% <60.0%> (-0.1%)	⬇️
opentelemetry-api/src/trace/order_map.rs	64.2% <64.2%> (ø)
opentelemetry-sdk/src/trace/sampler.rs	97.4% <100.0%> (+<0.1%)	⬆️
opentelemetry-sdk/src/metrics/controllers/push.rs	83.3% <0.0%> (+3.3%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7534891...e39fdd4. Read the comment docs.

[TommyCpp]

👍 Nice work. Just one request

[LukeMathWalker]

I've addressed it in an additional commit 👍🏻
That code example now compiles without having to be touched at all.

[TommyCpp]

LGTM. Thanks a lot. This is a great improvement 🎉

[jtescher]

@LukeMathWalker @TommyCpp Uping MSRV to 1.55 will likely break compatibility with tracing's 1.49 MSRV for tracing-opentelemtry. What's the motivation for optimizing attribute get operations?

[TommyCpp]

@LukeMathWalker @TommyCpp Uping MSRV to 1.55 will likely break compatibility with tracing's 1.49 MSRV for tracing-opentelemtry. What's the motivation for optimizing attribute get operations?

#794 has the full discussion but on the high level, lib writers wants to manipulate the attributes attached on spans and current implementation only supports do it by iterate through the attributes array, which is O(N)

On top of that we need to preserve the relative order the insertion so that the when number of the attributes exceed the limits we can trim the last attributes.

Combine the requirements we somehow want a OrderMap. indexmap provided that function. It's not a must have as we can just implement the same function on our own.

We can try to pin the version of indexmap to an older version to avoid a MSRV bump. I think tower depends on indexmap 1.0?

There are other places in the PR uses features stabilized after 1.49 but we can probably remove them.

[jtescher]

#794 has the full discussion but on the high level, lib writers wants to manipulate the attributes attached on spans and current implementation only supports do it by iterate through the attributes array, which is O(N)

I see. The spec has some similar issues open, could be worth documenting the relationship between OrderMap and EvictedHashMap, or consider merging the concepts and exposing in API crate if necessary.

Also worth considering the configuration options for other structs besides Span that currently take a Vec<KeyValue> (e.g. Event, Link) as there is now some inconsistency in how attributes are passed.