-
Notifications
You must be signed in to change notification settings - Fork 852
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Prototype] How to support complex attributes in logs/events? (Option C) #6960
base: main
Are you sure you want to change the base?
Conversation
*/ | ||
@SuppressWarnings("rawtypes") | ||
@Immutable | ||
public interface ComplexAttribute { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that since this is so similar to Attributes
that this should be plural: ComplexAttributes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
?
Another half-baked option.
This is basically Option A, but instead of reusing the
Attributes
class as the property bag for nested attributes, it introduces new typesComplexAttribute
/ComplexAttributeBuilder
/ComplexAttributeKey
which 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:
MapAttribute
MapAttributeBuilder
MapAttributeKey
If 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.