Improve attribute handling

[rubvs]

Fix elastic/apm-server#13703, with some refactoring of the conversion between OTLP attributes and APMEvent labels.

Context

From my understanding looking at https://opentelemetry.io/docs/specs/otel/common/#attribute, the element values of an array attribute has to:

1. Only consist of primitive values which includes; string, int64, bool, and float64
2. All elements in this array has to be the same (homogeneous).

It can be deduced that nested arrays are not allowed, since if this would be the case, the spec would have to also allow pointers, or a struct type to be included as a primitive. Effectively, creating an array reference. An array as value would not suffice, since its not atomic in principle. Rather, a collection, therefore breaking the homogeneous invariant. So I highly doubt that Attribute array value are allowed to be nested.

From a practical point of view, the concept of an attribute is not hierarchical. So nesting doesn't really makes sense. Spans and traces can be nested. Attributes cannot.

Problem

I've come to believe the reason for the bug in elastic/apm-server#13703 is more fundamental to the way we convert between OTLP attributes and APMEvent labels, than simply dropping heterogeneous elements.

The func ifaceAttributeValue acts as an interface between conversion of a pcommon.Value and a Go primitive. However, this is done by using the interface construct in Go as an abstract buffer for type conversion.

This technique, though valid, in practice tends to force a developer's hand to introduce templating, as we see in the pSliceToType function. Which is a slippery slope into metaprogramming hell. This is a deadly trap in C++, and is why I'm personally not a big proponent of templates in Go.

The bigger problem with introducing interface as a buffer, is that at some point, we have to do the actual conversion from any (aka interface) to the real, concrete Go primitives. To my limited knowledge, this is what I understand the purpose of setLabel to be; a producer of Go primitives when the exact translation is not made explicit. Therefore, setLabel is mostly called in the default section or the switch statements in translation methods, like translateResourceMetadata, etc.

The problem with this is multifold:

1. Testing: It becomes very hard to test. You're left with converting between primitives multiple times to ensure you handle all the conversion edge cases.
2. Information Loss: Fundamentally, the reason for this difficult testing is due to information loss by introducing interface. For example, the concept of a bool in lost after ifaceAttributeValue is called.
3. Stack: ifaceAttributeValue is called recursively to ensure that all array elements are properly converted. This recursive call is needed if arrays can be nested, which I don't think they can. So you are left with unnecessary stack calls.
4. Complexity: All of this adds unnecessary code; extra testing, interface, templating, more functions than needed, and difficulty debugging.

Solution

Do the conversion between pcommon.Value and Go primitives directly (by only calling setLabel). Do not use interface or templating. Ignore a recursive solution. We are not working with a Tree or Graph data structure. Simply, a linear homogeneous array.

[carsonip]

Can you also specify in PR description that this will fix elastic/apm-server#13703 ?

[carsonip]

The general direction looks great. Thanks for refactoring it. The code is much cleaner and easier to understand now.

[1pkg]

lgtm

[carsonip]

lgtm, thanks for refactoring. Modifying the existing tests to cover the new behavior is great.