-
Notifications
You must be signed in to change notification settings - Fork 916
Complex attributes incubating implementation #7814
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
base: main
Are you sure you want to change the base?
Changes from all commits
5e889a8
4a2c048
3b8e35f
04937c0
73466c8
1958768
e4308cf
5f342e2
8c40374
7d49b5e
d3f9dbe
08e6e0c
c7824e1
26a3413
d83ff5d
10d6e02
198faa2
4386c8c
575535d
f1ffad8
6480d0e
22e857d
b91d3bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,9 +8,13 @@ | |
| import io.opentelemetry.api.common.AttributeKey; | ||
| import io.opentelemetry.api.common.Attributes; | ||
| import io.opentelemetry.api.common.AttributesBuilder; | ||
| import io.opentelemetry.api.common.Value; | ||
| import io.opentelemetry.api.common.ValueType; | ||
| import io.opentelemetry.api.internal.ImmutableKeyValuePairs; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.Comparator; | ||
| import java.util.List; | ||
| import javax.annotation.Nullable; | ||
| import javax.annotation.concurrent.Immutable; | ||
|
|
||
|
|
@@ -51,9 +55,121 @@ public ExtendedAttributesBuilder toBuilder() { | |
| @Override | ||
| @Nullable | ||
| public <T> T get(ExtendedAttributeKey<T> key) { | ||
| if (key == null) { | ||
| return null; | ||
| } | ||
| if (key.getType() == ExtendedAttributeType.VALUE) { | ||
| return (T) getAsValue(key.getKey()); | ||
| } | ||
| // Check if we're looking for an array type but have a VALUE with empty array | ||
| if (isArrayType(key.getType())) { | ||
| T value = (T) super.get(key); | ||
| if (value == null) { | ||
| // Check if there's a VALUE with the same key that contains an empty array | ||
| Value<?> valueAttr = getValueAttribute(key.getKey()); | ||
| if (valueAttr != null && isEmptyArray(valueAttr)) { | ||
| return (T) Collections.emptyList(); | ||
| } | ||
| } | ||
| return value; | ||
| } | ||
| return (T) super.get(key); | ||
| } | ||
|
|
||
| private static boolean isArrayType(ExtendedAttributeType type) { | ||
| return type == ExtendedAttributeType.STRING_ARRAY | ||
| || type == ExtendedAttributeType.LONG_ARRAY | ||
| || type == ExtendedAttributeType.DOUBLE_ARRAY | ||
| || type == ExtendedAttributeType.BOOLEAN_ARRAY; | ||
| } | ||
|
|
||
| @Nullable | ||
| private Value<?> getValueAttribute(String keyName) { | ||
| List<Object> data = data(); | ||
| for (int i = 0; i < data.size(); i += 2) { | ||
| ExtendedAttributeKey<?> currentKey = (ExtendedAttributeKey<?>) data.get(i); | ||
| if (currentKey.getKey().equals(keyName) | ||
| && currentKey.getType() == ExtendedAttributeType.VALUE) { | ||
| return (Value<?>) data.get(i + 1); | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private static boolean isEmptyArray(Value<?> value) { | ||
| if (value.getType() != ValueType.ARRAY) { | ||
| return false; | ||
| } | ||
| @SuppressWarnings("unchecked") | ||
| List<Value<?>> arrayValues = (List<Value<?>>) value.getValue(); | ||
| return arrayValues.isEmpty(); | ||
| } | ||
|
|
||
| @Nullable | ||
| private Value<?> getAsValue(String keyName) { | ||
| // Find any attribute with the same key name and convert it to Value | ||
| List<Object> data = data(); | ||
| for (int i = 0; i < data.size(); i += 2) { | ||
| ExtendedAttributeKey<?> currentKey = (ExtendedAttributeKey<?>) data.get(i); | ||
| if (currentKey.getKey().equals(keyName)) { | ||
| Object value = data.get(i + 1); | ||
| return asValue(currentKey.getType(), value); | ||
| } | ||
| } | ||
| return null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this implementation benefit from #7076? |
||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| @Nullable | ||
| private static Value<?> asValue(ExtendedAttributeType type, Object value) { | ||
| switch (type) { | ||
| case STRING: | ||
| return Value.of((String) value); | ||
| case LONG: | ||
| return Value.of((Long) value); | ||
| case DOUBLE: | ||
| return Value.of((Double) value); | ||
| case BOOLEAN: | ||
| return Value.of((Boolean) value); | ||
| case STRING_ARRAY: | ||
| List<String> stringList = (List<String>) value; | ||
| Value<?>[] stringValues = new Value<?>[stringList.size()]; | ||
| for (int i = 0; i < stringList.size(); i++) { | ||
| stringValues[i] = Value.of(stringList.get(i)); | ||
| } | ||
| return Value.of(stringValues); | ||
| case LONG_ARRAY: | ||
| List<Long> longList = (List<Long>) value; | ||
| Value<?>[] longValues = new Value<?>[longList.size()]; | ||
| for (int i = 0; i < longList.size(); i++) { | ||
| longValues[i] = Value.of(longList.get(i)); | ||
| } | ||
| return Value.of(longValues); | ||
| case DOUBLE_ARRAY: | ||
| List<Double> doubleList = (List<Double>) value; | ||
| Value<?>[] doubleValues = new Value<?>[doubleList.size()]; | ||
| for (int i = 0; i < doubleList.size(); i++) { | ||
| doubleValues[i] = Value.of(doubleList.get(i)); | ||
| } | ||
| return Value.of(doubleValues); | ||
| case BOOLEAN_ARRAY: | ||
| List<Boolean> booleanList = (List<Boolean>) value; | ||
| Value<?>[] booleanValues = new Value<?>[booleanList.size()]; | ||
| for (int i = 0; i < booleanList.size(); i++) { | ||
| booleanValues[i] = Value.of(booleanList.get(i)); | ||
| } | ||
| return Value.of(booleanValues); | ||
| case VALUE: | ||
| // Already a Value | ||
| return (Value<?>) value; | ||
| case EXTENDED_ATTRIBUTES: | ||
| // Cannot convert EXTENDED_ATTRIBUTES to Value | ||
| return null; | ||
| } | ||
| // Should not reach here | ||
| return null; | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| @Override | ||
| public Attributes asAttributes() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,17 @@ | |
|
|
||
| package io.opentelemetry.api.incubator.common; | ||
|
|
||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.booleanArrayKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.booleanKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.doubleArrayKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.doubleKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.longArrayKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.longKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.stringArrayKey; | ||
| import static io.opentelemetry.api.incubator.common.ExtendedAttributeKey.stringKey; | ||
|
|
||
| import io.opentelemetry.api.common.Value; | ||
| import io.opentelemetry.api.common.ValueType; | ||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
|
|
@@ -36,11 +47,118 @@ public <T> ExtendedAttributesBuilder put(ExtendedAttributeKey<T> key, T value) { | |
| if (key == null || key.getKey().isEmpty() || value == null) { | ||
| return this; | ||
| } | ||
| if (key.getType() == ExtendedAttributeType.VALUE && value instanceof Value) { | ||
| putValue(key, (Value<?>) value); | ||
| return this; | ||
| } | ||
| data.add(key); | ||
| data.add(value); | ||
| return this; | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private void putValue(ExtendedAttributeKey<?> key, Value<?> valueObj) { | ||
| // Convert VALUE type to narrower type when possible | ||
| String keyName = key.getKey(); | ||
| switch (valueObj.getType()) { | ||
| case STRING: | ||
| put(stringKey(keyName), ((Value<String>) valueObj).getValue()); | ||
| return; | ||
| case LONG: | ||
| put(longKey(keyName), ((Value<Long>) valueObj).getValue()); | ||
| return; | ||
| case DOUBLE: | ||
| put(doubleKey(keyName), ((Value<Double>) valueObj).getValue()); | ||
| return; | ||
| case BOOLEAN: | ||
| put(booleanKey(keyName), ((Value<Boolean>) valueObj).getValue()); | ||
| return; | ||
| case ARRAY: | ||
| List<Value<?>> arrayValues = (List<Value<?>>) valueObj.getValue(); | ||
| ExtendedAttributeType attributeType = attributeType(arrayValues); | ||
| switch (attributeType) { | ||
| case STRING_ARRAY: | ||
| List<String> strings = new ArrayList<>(arrayValues.size()); | ||
| for (Value<?> v : arrayValues) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're interested in premature performance optimization, there's probably a way to to merge this with I'd say only pursue if it ends up making the code more concise with the perf improvement.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would require optimistically creating the array as we loop through and potentially throwing it away? |
||
| strings.add((String) v.getValue()); | ||
| } | ||
| put(stringArrayKey(keyName), strings); | ||
| return; | ||
| case LONG_ARRAY: | ||
| List<Long> longs = new ArrayList<>(arrayValues.size()); | ||
| for (Value<?> v : arrayValues) { | ||
| longs.add((Long) v.getValue()); | ||
| } | ||
| put(longArrayKey(keyName), longs); | ||
| return; | ||
| case DOUBLE_ARRAY: | ||
| List<Double> doubles = new ArrayList<>(arrayValues.size()); | ||
| for (Value<?> v : arrayValues) { | ||
| doubles.add((Double) v.getValue()); | ||
| } | ||
| put(doubleArrayKey(keyName), doubles); | ||
| return; | ||
| case BOOLEAN_ARRAY: | ||
| List<Boolean> booleans = new ArrayList<>(arrayValues.size()); | ||
| for (Value<?> v : arrayValues) { | ||
| booleans.add((Boolean) v.getValue()); | ||
| } | ||
| put(booleanArrayKey(keyName), booleans); | ||
| return; | ||
| case VALUE: | ||
| // Not coercible (empty, non-homogeneous, or unsupported element type) | ||
| data.add(key); | ||
| data.add(valueObj); | ||
| return; | ||
| case EXTENDED_ATTRIBUTES: | ||
| // Not coercible | ||
| data.add(key); | ||
| data.add(valueObj); | ||
| return; | ||
| default: | ||
| throw new IllegalArgumentException("Unexpected array attribute type: " + attributeType); | ||
| } | ||
| case KEY_VALUE_LIST: | ||
| case BYTES: | ||
| // Keep as VALUE type | ||
| data.add(key); | ||
| data.add(valueObj); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns the ExtendedAttributeType for a homogeneous array (STRING_ARRAY, LONG_ARRAY, | ||
| * DOUBLE_ARRAY, or BOOLEAN_ARRAY), or VALUE if the array is empty, non-homogeneous, or contains | ||
| * unsupported element types. | ||
| */ | ||
| private static ExtendedAttributeType attributeType(List<Value<?>> arrayValues) { | ||
| if (arrayValues.isEmpty()) { | ||
| return ExtendedAttributeType.VALUE; | ||
| } | ||
| ValueType elementType = arrayValues.get(0).getType(); | ||
| for (Value<?> v : arrayValues) { | ||
| if (v.getType() != elementType) { | ||
| return ExtendedAttributeType.VALUE; | ||
| } | ||
| } | ||
| switch (elementType) { | ||
| case STRING: | ||
| return ExtendedAttributeType.STRING_ARRAY; | ||
| case LONG: | ||
| return ExtendedAttributeType.LONG_ARRAY; | ||
| case DOUBLE: | ||
| return ExtendedAttributeType.DOUBLE_ARRAY; | ||
| case BOOLEAN: | ||
| return ExtendedAttributeType.BOOLEAN_ARRAY; | ||
| case ARRAY: | ||
| case KEY_VALUE_LIST: | ||
| case BYTES: | ||
| return ExtendedAttributeType.VALUE; | ||
| } | ||
| throw new IllegalArgumentException("Unsupported element type: " + elementType); | ||
| } | ||
|
|
||
| @Override | ||
| public ExtendedAttributesBuilder removeIf(Predicate<ExtendedAttributeKey<?>> predicate) { | ||
| if (predicate == null) { | ||
|
|
||
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 don't understand this bit. Is there special handling of empty somewhere?
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.
this is for
tested here: