Array values for span attributes

[thisthat]

This PR implements the specification introduced in open-telemetry/opentelemetry-specification#368 and resolves #801

[codecov-io]

Codecov Report

Merging #807 into master will increase coverage by 0.09%.
The diff coverage is 87.32%.

@@             Coverage Diff              @@
##             master     #807      +/-   ##
============================================
+ Coverage     85.55%   85.65%   +0.09%     
- Complexity     1066     1073       +7     
============================================
  Files           137      137              
  Lines          3932     3994      +62     
  Branches        348      354       +6     
============================================
+ Hits           3364     3421      +57     
- Misses          430      437       +7     
+ Partials        138      136       -2     

Impacted Files	Coverage Δ	Complexity Δ
...io/opentelemetry/exporters/otlp/CommonAdapter.java	87.50% <0.00%> (-3.81%)	6.00 <0.00> (ø)
...n/java/io/opentelemetry/common/AttributeValue.java	80.48% <84.90%> (+7.15%)	9.00 <4.00> (+4.00)
...ava/io/opentelemetry/exporters/jaeger/Adapter.java	96.70% <100.00%> (+0.50%)	20.00 <0.00> (+4.00)
...ntelemetry/sdk/trace/RecordEventsReadableSpan.java	85.40% <100.00%> (+0.15%)	47.00 <0.00> (+1.00)
...ava/io/opentelemetry/sdk/trace/SpanBuilderSdk.java	94.84% <100.00%> (+1.70%)	37.00 <0.00> (-2.00)	⬆️
...elemetry/sdk/trace/export/BatchSpansProcessor.java	93.23% <0.00%> (+1.50%)	9.00% <0.00%> (ø%)

---

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 83eb97e...84c0eb3. Read the comment docs.

[jkwatson]

looks like this needs a format

[carlosalberto]

Left a few comments. The important one for me is the one regarding null values in the arrays - it feels like a slight overkill, but something needed. Not sold on this one though. Thoughts?

[Oberon00]

@carlosalberto This should be dealt with in open-telemetry/opentelemetry-specification#431 (you gave it the 0.4 milestone before the array aspect came up)

[thisthat]

@open-telemetry/java-approvers PTAL again since open-telemetry/opentelemetry-specification#459 was merged! I will provide a followup PR to address the null and empty str cases of #948

[arminru]

Follow-up (#948):

• ignore null arrays instead of replacing them with empty ones (but remove previously set attribute, if any)
• store empty arrays since those are valid values
• tests for the changes above

[jkwatson]

Looks like this needs a rebase to pick up the java11 build.

[bogdandrutu]

Please rebase and add an issue to make sure we keep this in sync with specs.

[thisthat]

@carlosalberto done!

[jkwatson]

@thisthat In this PR, you call everything "array", but the return values are all Lists. With List<Boolean>, you'll require boxing all primitives into objects. did you consider actually implementing this with arrays, so that primitives don't need to be boxed (and so the name matches the type)?

[thisthat]

@jkwatson Yes, I considered that! I also would like to have pure arrays without all the boxing. Unfortunately, AutoValue does not support arrays (see here). An array-only implementation is possible but cumbersome, so I preferred implementing this with List<> and AutoValue. To guarantee immutability, we would need to duplicate the array internally so attributes are stored using a new pointer that is not visible by the OTel users.

[carlosalberto]

@bogdandrutu I remember you had a few doubts about this in the past, so please comment, or else we will merge soon ;) (you can always fill an issue later, though)

[bogdandrutu]

Let's merge this, will go with a flow and see where we will be in the future :)