Add support for array attributes
henrinormak opened this issue · 6 comments
Is your feature request related to a problem? Please describe.
OpenTelemetry Spans support attributes with array values, these attributes are currently not exported to CloudTrace. This is not notified of in any way (at least not that I can identify), which leads to application developers suspecting faults in their use of the library.
Describe the solution you'd like
Code that transforms Spans should support array types from the list of allowed types. This might require changes in CloudTrace APIs, which I can also file a feature request for, presumably here?
Describe alternatives you've considered
There are workarounds, like concatenating values into a singular string, but that's hacky, especially as OpenTelemetry Spans do allow for array types of attributes.
Thanks for raising this. The OTel spec actually has this section:
For protocols that do not natively support non-string values, non-string values SHOULD be represented as JSON-encoded strings. For example, the expression
int64(100)
will be encoded as100
,float64(1.5)
will be encoded as1.5
, and an empty array of any type will be encoded as[]
.
You're welcome to open a feature request for array types, but it probably won't be a quick process. Even though it's hacky, I think the JSON approach would be the best here.
@dashpole any thoughts on if we should do this? We also currently attributes of slice type in our Go exporter as well:
I can definitely open a PR for the JSON approach, my only question would be whether that would look decent on CloudTrace UI or not? I assume, filtering etc. won't work properly? Would it still make sense to open a feature request on the CloudTrace product as an entirety and in the meanwhile at least implement the stringification on the exporter level?
EDIT: Apparently I already opened the feature request before I opened this issue, linking it here for posterity - https://issuetracker.google.com/issues/254598673
my only question would be whether that would look decent on CloudTrace UI or not? I assume, filtering etc. won't work properly?
If we add array attributes as json strings, it probably wouldn't look great. Also, if string array attributes are ever supported, switching from json strings to the string array would technically be breaking. If we supported string arrays as json, we could put it behind a config option so that we could add support in the future (for users that don't enable the option).
@henrinormak would that work for you?
Sounds reasonable, I'm not too familiar with the package, but I can try and add this configurability to my PR linked above.
EDIT: Amended my PR with a config flag passed to the exporter.