-
Notifications
You must be signed in to change notification settings - Fork 780
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
[shared] Tag transformer improvements #5455
[shared] Tag transformer improvements #5455
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5455 +/- ##
==========================================
+ Coverage 83.38% 85.49% +2.10%
==========================================
Files 297 289 -8
Lines 12531 12509 -22
==========================================
+ Hits 10449 10694 +245
+ Misses 2082 1815 -267
Flags with carried forward coverage won't be shown. Click here to find out more.
|
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpTagTransformer.cs
Outdated
Show resolved
Hide resolved
Update... Ran into some strange behaviors with On .NET/Core, this test always passes: Assert.False(((Array)new nint[0]) is long[]);
Assert.False(((Array)new nint[0]) is int[]);
Assert.False(((Array)new nuint[0]) is long[]);
Assert.False(((Array)new nuint[0]) is int[]); On .NET Framework, the test varies based on processor architecture: // On x64
Assert.True(((Array)new nint[0]) is long[]);
Assert.False(((Array)new nint[0]) is int[]);
Assert.True(((Array)new nuint[0]) is long[]);
Assert.False(((Array)new nuint[0]) is int[]);
// On x86
Assert.False(((Array)new nint[0]) is long[]);
Assert.True(((Array)new nint[0]) is int[]);
Assert.False(((Array)new nuint[0]) is long[]);
Assert.True(((Array)new nuint[0]) is int[]); This test is consistent across .NET Framework and .NET/Core: Assert.False(((object)(nint)1) is long);
Assert.False(((object)(nint)1) is int);
Assert.False(((object)(nuint)1) is long);
Assert.False(((object)(nuint)1) is int); Everything worked before sort of accidentally but this change started causing exceptions on .NET Framework. @utpilla and I chatted offline and we decided to revert that new behavior just to make sure nothing breaks for 1.8.0 release: 331eb1a |
Changes
Removed the static coupling used to attach event source logging to the tag transformer class
Added nullable annotations
Removed TagAndValueTransformer. This was only used by OtlpExporter and added a whole bunch of unnecessary indirection
Merge requirement checklist
CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)