[Prototype] How to support complex attributes in logs/events? (Option C)#6960
[Prototype] How to support complex attributes in logs/events? (Option C)#6960trask wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
| */ | ||
| @SuppressWarnings("rawtypes") | ||
| @Immutable | ||
| public interface ComplexAttribute { |
There was a problem hiding this comment.
I think that since this is so similar to Attributes that this should be plural: ComplexAttributes.
There was a problem hiding this comment.
it seems more like a single complex attribute value to me, e.g.
Logger.setAttribute("xyz", complexAttribute)
as it's not really a collection of complex attributes, it's a collection of attributes (both standard and complex)
I could see the interface named something like NestedAttributes though...
| * @param value the value for this attribute. | ||
| * @return this. | ||
| */ | ||
| default <T> LogRecordBuilder setComplexAttribute(AttributeKey<T> key, ComplexAttribute value) { |
There was a problem hiding this comment.
What about adding a method:
LogRecordBuilder setAttribute(String key, Value value);
Now that we have a specification definition for standard attribute which perfectly aligns with our Attribute interface, I'd like to see if we can keep this alignment.
What do we loose if we provide log API setters for adding complex attributes, without any changes to Attributes / AttributeKey, AttributeBuilder?
|
Closing in favor of #6983 |
Another half-baked option.
This is basically Option A, but instead of reusing the
Attributesclass as the property bag for nested attributes, it introduces new typesComplexAttribute/ComplexAttributeBuilder/ComplexAttributeKeywhich are just copies ofAttributes/AttributesBuilder/AttributesKey.The disadvantage of this option is obviously the copy-pasting.
The advantage of this option is that it is a bit more explicit for users (and so probably a bit less confusing than reusing the top-level Attributes bag to represent complex attributes).
Note: I'm using the term "complex attributes" to mean map-valued attributes. I will see if this (or some other) term can be agreed on in specification / semconv.
Another naming option could be:
MapAttributeMapAttributeBuilderMapAttributeKeyIf we ever need to support "any valued" attributes (i.e. heterogeneous arrays), then I think we could layer that on top of this using Option B.