Resource attributes fix

[Austin-Tan]

This is my first time working on a change that would require a rework of a unit test. Please let me know if I have done that incorrectly.

Fixes #1606 .

Changes

Resource Attributes fixed to allow the values of key-value pairs to be of any object type, matching the behavior of Activity's SetTag method.

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Design discussion issue #
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #1647 (28aa176) into master (e24dccb) will increase coverage by 0.29%.
The diff coverage is 87.35%.

@@            Coverage Diff             @@
##           master    #1647      +/-   ##
==========================================
+ Coverage   81.90%   82.19%   +0.29%     
==========================================
  Files         245      250       +5     
  Lines        6637     6763     +126     
==========================================
+ Hits         5436     5559     +123     
- Misses       1201     1204       +3     

Impacted Files	Coverage Δ
...xporter.Console/ConsoleExporterHelperExtensions.cs	0.00% <0.00%> (ø)
...porter.Console/ConsoleExporterLoggingExtensions.cs	0.00% <0.00%> (ø)
...rter.InMemory/InMemoryExporterLoggingExtensions.cs	0.00% <0.00%> (ø)
....Exporter.Jaeger/JaegerExporterHelperExtensions.cs	15.38% <0.00%> (ø)
....Exporter.Zipkin/ZipkinExporterHelperExtensions.cs	15.38% <0.00%> (ø)
src/OpenTelemetry/BatchExportProcessor.cs	93.05% <ø> (ø)
src/OpenTelemetry/BatchLogRecordExportProcessor.cs	0.00% <0.00%> (ø)
...lemetry/Internal/SelfDiagnosticsConfigRefresher.cs	37.20% <0.00%> (ø)
src/OpenTelemetry/SimpleExportProcessor.cs	70.00% <ø> (ø)
...nTelemetryProtocol/OtlpExporterHelperExtensions.cs	92.30% <50.00%> (ø)
... and 20 more

[CodeBlanch]

I think the existing behavior is actually correct based on the spec:

Attributes are a list of zero or more key-value pairs. An Attribute MUST have the following properties:

• The attribute key, which MUST be a non-null and non-empty string.
• The attribute value, which is either:
  • A primitive type: string, boolean, double precision floating point (IEEE 754-1985) or signed 64 bit integer.
  • An array of primitive type values. The array MUST be homogeneous, i.e. it MUST NOT contain values of different types. For protocols that do not natively support array values such values SHOULD be represented as JSON strings.

We should allow int, maybe by converting to long, but everything? Not sure about that.

[cijothomas]

I think the existing behavior is actually correct based on the spec:

Attributes are a list of zero or more key-value pairs. An Attribute MUST have the following properties:

• The attribute key, which MUST be a non-null and non-empty string.

• The attribute value, which is either:

  • A primitive type: string, boolean, double precision floating point (IEEE 754-1985) or signed 64 bit integer.
  • An array of primitive type values. The array MUST be homogeneous, i.e. it MUST NOT contain values of different types. For protocols that do not natively support array values such values SHOULD be represented as JSON strings.

We should allow int, maybe by converting to long, but everything? Not sure about that.

Yes. We only need to do the conversion from int to long, float to double, and nothing more.

[Austin-Tan]

Thanks @CodeBlanch , I hadn't seen that part of the spec before, was just going off the Attributes section in Resource, which only mentioned that the value needed to be retrieved as a string. Will fix this tomorrow.

[Austin-Tan]

Restored unit test, functionality of the Resource Attributes should be identical to before, except using the System.Convert library to first convert any Ints to Longs, and any Floats to Doubles.

Noted that conversions Floats -> Doubles will have weird precision errors, but we allow customers to assume this risk if they want to use Floats? Highlighted below:

[Austin-Tan]

I think the existing behavior is actually correct based on the spec:

Attributes are a list of zero or more key-value pairs. An Attribute MUST have the following properties:

• The attribute key, which MUST be a non-null and non-empty string.

• The attribute value, which is either:

  • A primitive type: string, boolean, double precision floating point (IEEE 754-1985) or signed 64 bit integer.
  • An array of primitive type values. The array MUST be homogeneous, i.e. it MUST NOT contain values of different types. For protocols that do not natively support array values such values SHOULD be represented as JSON strings.

We should allow int, maybe by converting to long, but everything? Not sure about that.

Yes. We only need to do the conversion from int to long, float to double, and nothing more.

We are currently not supporting the last part of the spec, about arrays of primitive type values, correct @CodeBlanch @cijothomas ?

[CodeBlanch]

We are currently not supporting the last part of the spec, about arrays of primitive type values, correct?

That's correct. I don't think there's a particular reason for that, though, maybe we should tackle in a follow-up PR?