Add support for []interface{} as ArrayValue if all values are primitive types

[dmarkhas]

Currently, only arrays of specific primitive types are supported as attribute array values ([]string, []int, etc.), however it's not uncommon to deal with []interface{}, where the runtime type of every element is in fact a primitive type.
In such a case, there's no reason to discard the array, and it can be treated like other primitive arrays.

[linux-foundation-easycla]

• ❌ The commit (8036f77 ,760ad3f8ecdc32208986444458708075d87da2b4) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

[Aneurysm9]

Allowing []interface{} where all elements are the same primitive type could be a good addition, but I don't think this change quite gets there. It only assures that all elements are a primitive type. The spec requires that arrays be homogenous:

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.

One thing I'd worry about allowing []interface{} is that the behavior of uses of that capability may be unpredictable. It can be known when you write span.SetAttributes(attributes.Array("foo", []int{1,2,3})) that it'll work. Doing the same with []interface{} may fail unpredictably if the values are taken from user input or some other source of unpredictable types.

[dmarkhas]

Thanks for the comment @Aneurysm9. I missed that part of the spec, but it's not too much of an effort to restrict it to match the spec.

The problem IMO with the current implementation is that doing span.SetAttributes(attributes.Array("foo", []int{1,2,3})) will indeed "work" but so would doing span.SetAttributes(attributes.Array("foo", []interface{}{1,2,3})) in the sense that it compiles and no error is ever reported. It's just that the 2nd call is silently dropped. One has to read the godoc of attributes.Array to understand its limits.

Would stringifying all non-homogenous arrays (as the spec suggests) be a sufficiently safe solution, in your opinion?

[Aneurysm9]

Thanks for the comment @Aneurysm9. I missed that part of the spec, but it's not too much of an effort to restrict it to match the spec.

The problem IMO with the current implementation is that doing span.SetAttributes(attributes.Array("foo", []int{1,2,3})) will indeed "work" but so would doing span.SetAttributes(attributes.Array("foo", []interface{}{1,2,3})) in the sense that it compiles and no error is ever reported. It's just that the 2nd call is silently dropped. One has to read the godoc of attributes.Array to understand its limits.

I think I can be convinced that allowing homogenous []interface{} is fine, but I'd like to hear from other contributors on the topic. I wonder whether combining that with a warning when heterogenous []interface{} are presented would be a good experience.

Would stringifying all non-homogenous arrays (as the spec suggests) be a sufficiently safe solution, in your opinion?

I probably shouldn't have included the second sentence in that quote, it's speaking to something distinct. That sentence is intended to provide a means to convey array attributes over protocols that do not support arrays. It does not provide for serializing non-homogenous array attributes. The caller is free to do so if they want, though.

[dmarkhas]

Thanks.
What I meant is that a homogeneous []interface{} meets the spec and can be supported, while a non-homogeneous []interface{} can be made to match the spec by stringifying all of its values, thus making it effectively homogeneous.

[MrAlias]

What I meant is that a homogeneous []interface{} meets the spec and can be supported, while a non-homogeneous []interface{} can be made to match the spec by stringifying all of its values, thus making it effectively homogeneous.

The conversion to string is a topic that needs to be resolved at the specification level. It has been a topic of discussion there in the past. The exact format of strigification needs to be explicit and well defined to ensure compatibility between OTel implementations. However, prior discussions could not reach agreement about what that format should be or even if it is possible for all languages.

Personally, I do not like the idea of allowing []interface{} slice types and I do not think this change should be accepted. That said, I think this shows an underlying issue with our implementation. There is a real user experience issue being presented here: an attribute is added that is syntactically valid, it is dropped, and the user has no knowledge of this omission without diving into implementation. This needs to be resolved. By just adding another special type to passed as an interface{} type will only prolong the underlying issue and will not ultimately resolve the issue. It will crop back up whenever another user passes another type.

Given generics are not yet a part of the Go language, I think we need to resolve this issue by explicitly breaking out the allowed array types into their own values. I think the best place for this discussion is in an issue of it's own. I'll open one to track this.

[MrAlias]

#2158

[MrAlias]

Superseded by #2162