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

Experimental support for Log AnyValue body #5880

Merged
merged 7 commits into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 4 additions & 1 deletion buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ tasks {
// https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM
"-Xlint:-processing",
// We suppress the "options" warning because it prevents compilation on modern JDKs
"-Xlint:-options"
"-Xlint:-options",

// Fail build on any warning
"-Werror",
),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import io.opentelemetry.proto.common.v1.AnyValue;
import io.opentelemetry.proto.common.v1.InstrumentationScope;
import io.opentelemetry.proto.common.v1.KeyValue;
import io.opentelemetry.proto.common.v1.KeyValueList;
import io.opentelemetry.proto.logs.v1.LogRecord;
import io.opentelemetry.proto.logs.v1.ResourceLogs;
import io.opentelemetry.proto.logs.v1.ScopeLogs;
Expand Down Expand Up @@ -154,19 +153,7 @@ void toProtoLogRecord_MinimalFields() {
assertThat(logRecord.getSeverityText()).isBlank();
assertThat(logRecord.getSeverityNumber().getNumber())
.isEqualTo(Severity.UNDEFINED_SEVERITY_NUMBER.getSeverityNumber());
assertThat(logRecord.getBody())
.isEqualTo(
AnyValue.newBuilder()
.setKvlistValue(
KeyValueList.newBuilder()
.addValues(
KeyValue.newBuilder()
.setKey("key")
.setValue(AnyValue.newBuilder().setStringValue("foo").build())
.build())
.build())
.setStringValue("")
.build());
assertThat(logRecord.getBody()).isEqualTo(AnyValue.newBuilder().setStringValue("").build());
assertThat(logRecord.getAttributesList()).isEmpty();
assertThat(logRecord.getDroppedAttributesCount()).isZero();
assertThat(logRecord.getTimeUnixNano()).isEqualTo(12345);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

package io.opentelemetry.extension.incubator.logs;

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

Expand Down Expand Up @@ -52,7 +53,7 @@ static AnyValue<Double> of(double value) {
}

/** Returns an {@link AnyValue} for the {@code byte[]} value. */
static AnyValue<byte[]> of(byte[] value) {
static AnyValue<ByteBuffer> of(byte[] value) {
return AnyValueBytes.create(value);
}

Expand Down Expand Up @@ -89,7 +90,8 @@ static AnyValue<List<KeyAnyValue>> of(Map<String, AnyValue<?>> value) {
* <li>{@link AnyValueType#DOUBLE} returns {@code double}
* <li>{@link AnyValueType#ARRAY} returns {@link List} of {@link AnyValue}
* <li>{@link AnyValueType#KEY_VALUE_LIST} returns {@link List} of {@link KeyAnyValue}
* <li>{@link AnyValueType#BYTES} returns {@code byte[]}
* <li>{@link AnyValueType#BYTES} returns read only {@link ByteBuffer}. See {@link
* ByteBuffer#asReadOnlyBuffer()}.
* </ul>
*/
T getValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,19 @@
package io.opentelemetry.extension.incubator.logs;

import io.opentelemetry.api.internal.OtelEncodingUtils;
import java.nio.ByteBuffer;
import java.util.Arrays;
import java.util.Objects;

final class AnyValueBytes implements AnyValue<byte[]> {
final class AnyValueBytes implements AnyValue<ByteBuffer> {

private final byte[] value;
private final byte[] raw;

private AnyValueBytes(byte[] value) {
this.value = value;
this.raw = value;
}

static AnyValue<byte[]> create(byte[] value) {
static AnyValue<ByteBuffer> create(byte[] value) {
Objects.requireNonNull(value, "value must not be null");
return new AnyValueBytes(Arrays.copyOf(value, value.length));
}
Expand All @@ -28,16 +29,16 @@
}

@Override
public byte[] getValue() {
return value;
public ByteBuffer getValue() {
return ByteBuffer.wrap(raw).asReadOnlyBuffer();
Copy link
Contributor

Choose a reason for hiding this comment

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

Stoked!

}

@Override
public String asString() {
// TODO: base64 would be better, but isn't available in android and java. Can we vendor in a
// base64 implementation?
char[] arr = new char[value.length * 2];
OtelEncodingUtils.bytesToBase16(value, arr, value.length);
char[] arr = new char[raw.length * 2];
OtelEncodingUtils.bytesToBase16(raw, arr, raw.length);
return new String(arr);
}

Expand All @@ -49,15 +50,13 @@
@Override
public boolean equals(Object o) {
if (this == o) {
return true;

Check warning on line 53 in extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/logs/AnyValueBytes.java

View check run for this annotation

Codecov / codecov/patch

extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/logs/AnyValueBytes.java#L53

Added line #L53 was not covered by tests
}
return (o instanceof AnyValue)
&& ((AnyValue<?>) o).getType() == AnyValueType.BYTES
&& Arrays.equals(this.value, (byte[]) ((AnyValue<?>) o).getValue());
return (o instanceof AnyValueBytes) && Arrays.equals(this.raw, ((AnyValueBytes) o).raw);
}

@Override
public int hashCode() {
return Arrays.hashCode(value);
return Arrays.hashCode(raw);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
@Override
public String asString() {
return value.stream()
.map(entry -> entry.getKey() + "=" + entry.getAnyValue().asString())
.map(item -> item.getKey() + "=" + item.getAnyValue().asString())
.collect(joining(", ", "[", "]"));
}

Expand All @@ -63,7 +63,7 @@
@Override
public boolean equals(Object o) {
if (this == o) {
return true;

Check warning on line 66 in extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/logs/KeyAnyValueList.java

View check run for this annotation

Codecov / codecov/patch

extensions/incubator/src/main/java/io/opentelemetry/extension/incubator/logs/KeyAnyValueList.java#L66

Added line #L66 was not covered by tests
}
return (o instanceof AnyValue) && Objects.equals(this.value, ((AnyValue<?>) o).getValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import static org.junit.jupiter.params.provider.Arguments.arguments;

import io.opentelemetry.api.internal.OtelEncodingUtils;
import java.nio.ByteBuffer;
import java.nio.ReadOnlyBufferException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -73,7 +75,12 @@ void anyValue_OfByteArray() {
.satisfies(
anyValue -> {
assertThat(anyValue.getType()).isEqualTo(AnyValueType.BYTES);
assertThat(anyValue.getValue()).isEqualTo(new byte[] {'a', 'b'});
ByteBuffer value = anyValue.getValue();
// AnyValueBytes returns read only view of ByteBuffer
assertThatThrownBy(value::array).isInstanceOf(ReadOnlyBufferException.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

byte[] bytes = new byte[value.remaining()];
value.get(bytes);
assertThat(bytes).isEqualTo(new byte[] {'a', 'b'});
assertThat(anyValue).hasSameHashCodeAs(AnyValue.of(new byte[] {'a', 'b'}));
});
}
Expand Down
Loading