-
Notifications
You must be signed in to change notification settings - Fork 772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OTLP Span arrays allow exporting null values + support for long arrays #1919
OTLP Span arrays allow exporting null values + support for long arrays #1919
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1919 +/- ##
==========================================
+ Coverage 83.69% 84.38% +0.68%
==========================================
Files 192 188 -4
Lines 6159 6117 -42
==========================================
+ Hits 5155 5162 +7
+ Misses 1004 955 -49
|
using var rootActivity = activitySource.StartActivity("root", ActivityKind.Client); | ||
|
||
var stringArr = new string[] { "test", null }; | ||
var longArr = new long?[] { 1, null }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens to this null element ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the tests for nullable primtive type arrays like long?[] and double?[]. I think discussion of them is outside the scope of this PR
test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterTest.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ActivityExtensions.cs
Outdated
Show resolved
Hide resolved
I think 41b3eef needs to be reverted. Copying my comment from #1939 (comment): Hi! The spec says that nulls in arrays are invalid and you should only allow them if the language does not make it possible to prevent them at compile-time. So explicitly using
only then does the spec also say:
But in C# this only applies to strings. |
Fixes #1843 .
CHANGELOG.md
updated for non-trivial changes