Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

trask
Copy link
Member

@trask trask commented Dec 16, 2024

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 types ComplexAttribute / ComplexAttributeBuilder / ComplexAttributeKey which are just copies of Attributes / 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.

*/
@SuppressWarnings("rawtypes")
@Immutable
public interface ComplexAttribute {
Copy link
Contributor

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.

Copy link
Member Author

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) {
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants