chore: ensure attributes are spec compliant

[dyladan]

Fixes #1340

• Attribute values may be number, string, boolean, or a homogeneous array of number, string, or boolean
• Attribute keys must not be empty strings
• Setting an attribute which is already set overwrites it
• Setting an attribute to null (or undefined) deletes it
• Null values in arrays are allowed and preserved because they convey meaning

[codecov]

Codecov Report

Merging #1488 into master will decrease coverage by 0.04%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master    #1488      +/-   ##
==========================================
- Coverage   93.16%   93.11%   -0.05%     
==========================================
  Files         152      155       +3     
  Lines        4580     4811     +231     
  Branches      931      971      +40     
==========================================
+ Hits         4267     4480     +213     
- Misses        313      331      +18     

Impacted Files	Coverage Δ
packages/opentelemetry-tracing/src/Span.ts	98.14% <81.81%> (-1.86%)	⬇️
...ckages/opentelemetry-core/src/common/attributes.ts	97.29% <97.29%> (ø)
...ackages/opentelemetry-shim-opentracing/src/shim.ts	87.70% <100.00%> (-0.30%)	⬇️
packages/opentelemetry-tracing/src/Tracer.ts	100.00% <100.00%> (ø)
.../opentelemetry-exporter-collector/src/transform.ts	86.81% <0.00%> (-9.90%)	⬇️
...emetry-core/src/platform/node/RandomIdGenerator.ts	87.50% <0.00%> (-6.25%)	⬇️
...ry-plugin-xml-http-request/src/enums/EventNames.ts	100.00% <0.00%> (ø)
...s/opentelemetry-plugin-xml-http-request/src/xhr.ts	97.23% <0.00%> (ø)

[obecny]

can you please improve the test coverage, the coverage for added file attributes.ts shows no tests

[Flarna]

Not sure if it is relevant but the attributes given to Tracer#startSpan() are passed to shouldSample before they get validated.

[obecny]

lgtm, I hope you can add tests to added code (attributes.ts ) - what codecov is showing before merging that.

[dyladan]

Not sure if it is relevant but the attributes given to Tracer#startSpan() are passed to shouldSample before they get validated.

Fixed

lgtm, I hope you can add tests to added code (attributes.ts ) - what codecov is showing before merging that.

Fixed

[Flarna]

Attributes directly passed to startTrace() are now checked twice but I assume the overhead is negligible compared to all the rest done by the tracer.

[dyladan]

Attributes directly passed to startTrace() are now checked twice but I assume the overhead is negligible compared to all the rest done by the tracer.

We could remove the check from the Span constructor, but then we would be trusting the input is validated by tracer and user hasn't done something like new Span().

Another option would be to remove the validation in the Tracer and make sure the samplers are resilient to untrusted input, but then we need to do that in every sampler. And users can plug in custom samplers that might assume data is valid.