Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 73 additions & 0 deletions api/all/src/main/java/io/opentelemetry/api/common/Value.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
package io.opentelemetry.api.common;

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -84,6 +86,77 @@ static Value<List<KeyValue>> of(Map<String, Value<?>> value) {
return KeyValueList.createFromMap(value);
}

/**
* Convert an object of primitives, Lists, and Maps (nested in any manner) to {@link Value}.
*
* <p>Specifically, the following types are supported. If the {@code object} (or any nested data
* structures) contains an unsupported type, an {@link IllegalArgumentException} is thrown.
*
* <ul>
* <li>{@link Long}
* <li>{@link Integer}
* <li>{@link Float}
* <li>{@link Double}
* <li>{@link Boolean}
* <li>{@code byte[]}
* <li>{@code List<?>}, where each list entry is a supported type
* <li>{@code Map<String, ?>}, where each value is a supported type
* </ul>
*
* @param object the object to convert
* @return the equivalent {@link Value}
* @throws IllegalArgumentException if not able to convert the object to {@link Value}
Copy link
Member Author

Choose a reason for hiding this comment

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

Might want to throw a checked exception to more forcibly signal to callers that they need to handle this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Decided against this:

  • This method is analagous to jackson ObjectMapper#convertValue, which only throws a runtime exception. This API is quite popular and I've used it myself without issue for years, suggesting that a runtime exception is sufficient.
  • I'm not sure what checked exception would be most appropriate to throw. Probably some subclass of IOException, and we'd probably want to introduce our own dedicated exception. In contrast, IllegalArgumentException pretty perfectly describes the error and seems appropriate to throw.

*/
@SuppressWarnings("ThrowsUncheckedException")
static Value<?> convert(Object object) throws IllegalArgumentException {
Copy link
Member

Choose a reason for hiding this comment

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

do we think we would use the Object-ness? or would we just use convert(Map) and convert(List)?

e.g. in the structured log attribute mapping case, would we do instanceof and use regular attributes where possible, and only use Value attributes when it's a Map or List? or would we just convert(Object) all of them?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good point!

Given #7814 and the perf penalty associated with using Value to needless represent primitive / array of primitive attributes, I think we could steer people away from doing this by only publicly exposing Map and List variants. I.e.

public static Value<?> convert(Map<String, Object>) {}
public static Value<?> convert(List<Object>) {}
private static Value<?> convert(Object) {}

if (object instanceof Integer) {
return Value.of((Integer) object);
}
if (object instanceof Long) {
return Value.of((Long) object);
}
if (object instanceof Float) {
return Value.of((Float) object);
}
if (object instanceof Double) {
return Value.of((Double) object);
}
if (object instanceof Boolean) {
return Value.of((Boolean) object);
}
if (object instanceof String) {
return Value.of((String) object);
}
if (object instanceof byte[]) {
return Value.of((byte[]) object);
}
if (object instanceof List) {
List<?> list = (List<?>) object;
List<Value<?>> valueList = new ArrayList<>(list.size());
for (Object entry : list) {
valueList.add(Value.convert(entry));
}
return Value.of(valueList);
}
if (object instanceof Map) {
Map<?, ?> map = (Map<?, ?>) object;
Map<String, Value<?>> valueMap = new HashMap<>(map.size());
map.forEach(
(key, value) -> {
if (!(key instanceof String)) {
throw new IllegalArgumentException(
"Cannot convert map with key type "
+ key.getClass().getSimpleName()
+ " to value");
}
valueMap.put((String) key, Value.convert(value));
});
return Value.of(valueMap);
}
throw new IllegalArgumentException(
"Cannot convert object of type " + object.getClass().getSimpleName() + " to value");
}

/** Returns the type of this {@link Value}. Useful for building switch statements. */
ValueType getType();

Expand Down
75 changes: 75 additions & 0 deletions api/all/src/test/java/io/opentelemetry/api/logs/ValueTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.params.provider.Arguments.arguments;

import com.google.common.collect.ImmutableMap;
import io.opentelemetry.api.common.KeyValue;
import io.opentelemetry.api.common.Value;
import io.opentelemetry.api.common.ValueType;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.nio.ByteBuffer;
import java.nio.ReadOnlyBufferException;
import java.nio.charset.StandardCharsets;
Expand All @@ -20,7 +23,9 @@
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Stream;
import org.assertj.core.data.Offset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -212,4 +217,74 @@ void valueByteAsString() {
byte[] decodedBytes = Base64.getDecoder().decode(base64Encoded);
assertThat(new String(decodedBytes, StandardCharsets.UTF_8)).isEqualTo(str);
}

@ParameterizedTest
@MethodSource("convertArgs")
void convert(Object object, Value<?> expected) {
Value<?> converted = Value.convert(object);
if (Objects.requireNonNull(expected.getType()) == ValueType.DOUBLE) {
assertThat(converted.getType()).isEqualTo(ValueType.DOUBLE);
assertThat((Double) converted.getValue())
.isEqualTo((Double) expected.getValue(), Offset.offset(.001));
} else {
assertThat(converted).isEqualTo(expected);
}
}

@SuppressWarnings("cast")
private static Stream<Arguments> convertArgs() {
return Stream.of(
Arguments.of((int) 1, Value.of(1)),
Arguments.of(1L, Value.of(1L)),
Arguments.of(Long.valueOf(1L), Value.of(1L)),
Arguments.of((float) 1.1, Value.of(1.1)),
Arguments.of(1.1D, Value.of(1.1)),
Arguments.of(Double.valueOf(1.1D), Value.of(1.1)),
Arguments.of(true, Value.of(true)),
Arguments.of(Boolean.valueOf(true), Value.of(true)),
Arguments.of("value", Value.of("value")),
Arguments.of(
"value".getBytes(StandardCharsets.UTF_8),
Value.of("value".getBytes(StandardCharsets.UTF_8))),
Arguments.of(
Arrays.asList("value1", "value2"),
Value.of(Arrays.asList(Value.of("value1"), Value.of("value2")))),
Arguments.of(
Arrays.asList("value", true),
Value.of(Arrays.asList(Value.of("value"), Value.of(true)))),
Arguments.of(
ImmutableMap.builder().put("key1", "value").put("key2", true).build(),
Value.of(
ImmutableMap.<String, Value<?>>builder()
.put("key1", Value.of("value"))
.put("key2", Value.of(true))
.build())),
Arguments.of(
ImmutableMap.builder()
.put("key1", "value")
.put("key2", true)
.put("key3", Collections.singletonMap("key4", "value"))
.build(),
Value.of(
ImmutableMap.<String, Value<?>>builder()
.put("key1", Value.of("value"))
.put("key2", Value.of(true))
.put("key3", Value.of(Collections.singletonMap("key4", Value.of("value"))))
.build())));
}

@ParameterizedTest
@MethodSource("convertUnsupportedArgs")
void convertUnsupported(Object object) {
assertThatThrownBy(() -> Value.convert(object)).isInstanceOf(IllegalArgumentException.class);
}

private static Stream<Arguments> convertUnsupportedArgs() {
return Stream.of(
Arguments.of(new Object()),
Arguments.of(new BigInteger("1")),
Arguments.of(new BigDecimal("1.1")),
Arguments.of(Collections.singletonList(new Object())),
Arguments.of(Collections.singletonMap(new Object(), "value")));
}
}
5 changes: 5 additions & 0 deletions docs/apidiffs/current_vs_latest/opentelemetry-api.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
Comparing source compatibility of opentelemetry-api-1.57.0-SNAPSHOT.jar against opentelemetry-api-1.56.0.jar
*** MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.api.common.Value (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
GENERIC TEMPLATES: === T:java.lang.Object
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.api.common.Value<?> convert(java.lang.Object)
+++ NEW EXCEPTION: java.lang.IllegalArgumentException
*** MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.api.GlobalOpenTelemetry (not serializable)
=== CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+++ NEW METHOD: PUBLIC(+) STATIC(+) io.opentelemetry.api.OpenTelemetry getOrNoop()
Expand Down