Skip to content

Prototype: Complex attributes (Option C) #7813

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.

Sign up for GitHub

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

Closed
trask wants to merge 6 commits into from
Closed

Prototype: Complex attributes (Option C) #7813

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a8085a5
Prototype: Complex attributes (Option C - minimal)
trask Nov 5, 2025
d9ed983
Remove String, Value<?> overload
trask Nov 6, 2025
fa87c47
Add javadoc explaining coersion
trask Nov 6, 2025
5cfe4ab
more javadoc
trask Nov 7, 2025
42d4b8a
more javadoc
trask Nov 7, 2025
99668f0
more
trask Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/all/src/main/java/io/opentelemetry/api/common/AttributeKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,9 @@ static AttributeKey<List<Long>> longArrayKey(String key) {
static AttributeKey<List<Double>> doubleArrayKey(String key) {
return InternalAttributeKeyImpl.create(key, AttributeType.DOUBLE_ARRAY);
}

/** Returns a new AttributeKey for generic {@link Value} valued attributes. */
static AttributeKey<Value<?>> valueKey(String key) {
return InternalAttributeKeyImpl.create(key, AttributeType.VALUE);
}
}
3 changes: 2 additions & 1 deletion api/all/src/main/java/io/opentelemetry/api/common/AttributeType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ public enum AttributeType {
STRING_ARRAY,
BOOLEAN_ARRAY,
LONG_ARRAY,
DOUBLE_ARRAY
DOUBLE_ARRAY,
VALUE
}
16 changes: 16 additions & 0 deletions api/all/src/main/java/io/opentelemetry/api/common/AttributesBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import static io.opentelemetry.api.common.AttributeKey.longKey;
import static io.opentelemetry.api.common.AttributeKey.stringArrayKey;
import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static io.opentelemetry.api.common.AttributeKey.valueKey;

import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -164,6 +165,21 @@ default AttributesBuilder put(String key, boolean... value) {
return put(booleanArrayKey(key), toList(value));
}

/**
* Puts a generic ({@link Value}) attribute into this.
*
* <p>Note: It is strongly recommended to use {@link #put(AttributeKey, Object)}, and pre-allocate
* your keys, if possible.
*
* @return this Builder
*/
default AttributesBuilder put(String key, Value<?> value) {
Copy link
Member Author

@trask trask Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, should these be identical?

  • put("abc", Value.of("xyz"))
  • put("abc", "xyz")

they should be identical over OTLP

but should they be exposed identically to in-memory processors?

specifically, is the following behavior ok for put("abc", Value.of("xyz"))

  • Attributes.get(stringKey("abc")) returns null
  • Attributes.forEach() and Attributes.asMap() both represent the key value pair as an attribute key with AttributeType VALUE and a value with type Value

I think this is ok, since you're explicitly opting in to an AttributeKey with AttributeType VALUE when you're setting the attribute

Oh wait, maybe this is a good reason not to have this overload put(String, Value<?>) and require calling put(AttributeKey, Value<?>).

Copy link
Member

@jack-berg jack-berg Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but should they be exposed identically to in-memory processors?

I think yes since if they are not identical, then every in-memory processors has extra work to do if they want to perform the simple task of accessing a scalar value.

Oh wait, maybe this is a good reason not to have this overload put(String, Value) and require calling put(AttributeKey, Value)

This isn't a good enough defense because a caller could just call put(AttriubteKey.valueKey("abc"), Value.of("xyz") and achieve the same affect.

We could have the default implementation of put(String, Value) and put(AttributeKey, Value):

  • Check if the value is a primitive or array of primitives
  • If so, call the equivalent API (e.g. put(String, String) for string)
  • Else the value is complex (map, array of maps, etc) and can be left as a Value

Copy link
Contributor

@breedx-splk breedx-splk Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's my hot take that any time one finds oneself re/inventing a type system in a language to try and work around the language's type system shortcomings, you're in for a bad time. 😬

But yeah, if we go this route, the a value of type String must be necessarily different from a Value of type String. So yeah, ValueString (which isA Value<String>) does not have semantic equivalence to a String string.

I'm sure this will trip up someone, or at least cause them to cuss about this, but it doesn't seem like a deal breaker to me.

...or as was already mentioned, requires other code to do more work to check.

Copy link
Member Author

@trask trask Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Check if the value is a primitive or array of primitives
  • If so, call the equivalent API (e.g. put(String, String) for string)

I drafted javadoc to help us evaluate this option: fa87c47

if (value == null) {
return this;
}
return put(valueKey(key), value);
}

/**
* Puts all the provided attributes into this Builder.
*
Expand Down
Loading