Resource Attributes allow Primitive Arrays #1852
Conversation
| @@ -93,8 +93,7 @@ public Resource Merge(Resource other) | |||
| string sanitizedKey; | |||
| if (attribute.Key == null) | |||
| { | |||
| OpenTelemetrySdkEventSource.Log.InvalidArgument("Create resource", "attribute key", "Attribute key should be non-null string."); | |||
| sanitizedKey = string.Empty; | |||
| throw new System.ArgumentException("Resource's attributes contains a null key"); | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
The message might be misleading since it is talking about "attributes on the resource" but the method is only focusing on one attribute.
One possible way is to put something like nameof(attribute) has a null key here.
There was a problem hiding this comment.
I might be misunderstanding but each attribute is just a key and value, so a null key makes it impossible to do that right? I could print the Value instead since Key is null, but it may also be null and would then require checking and formatting
There was a problem hiding this comment.
Found out C# prevents you from instantiating any dictionary where the key is null, it'll throw its own ArgumentNullException. Might be the reason we originally didn't check for key == null, and I'm happy to just remove this change if that makes sense.
Codecov Report
@@ Coverage Diff @@
## main #1852 +/- ##
==========================================
+ Coverage 83.77% 84.02% +0.24%
==========================================
Files 187 187
Lines 5967 6004 +37
==========================================
+ Hits 4999 5045 +46
+ Misses 968 959 -9
|
|
Doesn't have to be done on this PR, but we also need to make sure exporters support this. For example, this... ...only supports the primitives. It needs to do something like what But can't call that exactly (I don't think) because Process doesn't have the PooledList. A dash of refactoring first. |
Fixes #1652
Changes
Previously would throw exceptions upon string[], bool[], long[], double[]. Accepts now.
Also, throws exception on null KEYs as well, not just null VALUEs
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes