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

Refactor Event API to reflect spec changes #6318

Merged
merged 2 commits into from
Mar 29, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
package io.opentelemetry.api.incubator.events;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.incubator.logs.AnyValue;
import io.opentelemetry.api.logs.Severity;
import io.opentelemetry.context.Context;
import java.time.Instant;
import java.util.concurrent.TimeUnit;

Expand All @@ -20,17 +23,19 @@ static EventLogger getInstance() {
}

@Override
public void emit(String eventName, Attributes attributes) {}
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 it's fine for now, but removing this requires the user wanting to use the "empty" event (where there is no payload, which spec allows) to do eventLogger.builder("foo").build() which is fine, but could have improved ergonomics in the future. 👍🏻

I also respect that we're sensitive to API surface area. 🙃


@Override
public EventBuilder builder(String eventName, Attributes attributes) {
public EventBuilder builder(String eventName) {
return NoOpEventBuilder.INSTANCE;
}

private static class NoOpEventBuilder implements EventBuilder {

public static final EventBuilder INSTANCE = new NoOpEventBuilder();

@Override
public EventBuilder put(String key, AnyValue<?> value) {
return this;
}

@Override
public EventBuilder setTimestamp(long timestamp, TimeUnit unit) {
return this;
Expand All @@ -41,6 +46,21 @@ public EventBuilder setTimestamp(Instant instant) {
return this;
}

@Override
public EventBuilder setContext(Context context) {
return this;
}

@Override
public EventBuilder setSeverity(Severity severity) {
return this;
}

@Override
public EventBuilder setAttributes(Attributes attributes) {
return this;
}

@Override
public void emit() {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,107 @@

package io.opentelemetry.api.incubator.events;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.incubator.logs.AnyValue;
import io.opentelemetry.api.logs.Severity;
import io.opentelemetry.context.Context;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;

/** The EventBuilder is used to {@link #emit()} events. */
public interface EventBuilder {

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, String value) {
return put(key, AnyValue.of(value));
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, long value) {
return put(key, AnyValue.of(value));
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, double value) {
return put(key, AnyValue.of(value));
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, boolean value) {
return put(key, AnyValue.of(value));
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, String... value) {
List<AnyValue<?>> values = new ArrayList<>(value.length);
for (String val : value) {
values.add(AnyValue.of(val));
}
return put(key, AnyValue.of(values));
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, long... value) {
List<AnyValue<?>> values = new ArrayList<>(value.length);
for (long val : value) {
values.add(AnyValue.of(val));
}
return put(key, AnyValue.of(values));
breedx-splk marked this conversation as resolved.
Show resolved Hide resolved
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, double... value) {
List<AnyValue<?>> values = new ArrayList<>(value.length);
for (double val : value) {
values.add(AnyValue.of(val));
}
return put(key, AnyValue.of(values));
}

/** Put the given {@code key} and {@code value} in the payload. */
default EventBuilder put(String key, boolean... value) {
List<AnyValue<?>> values = new ArrayList<>(value.length);
for (boolean val : value) {
values.add(AnyValue.of(val));
}
return put(key, AnyValue.of(values));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The test coverage on these default methods comes from the SDK. Do we care about having direct coverage from the API itself?


/** Put the given {@code key} and {@code value} in the payload. */
EventBuilder put(String key, AnyValue<?> value);

/**
* Set the epoch {@code timestamp} for the event, using the timestamp and unit.
* Set the epoch {@code timestamp}, using the timestamp and unit.
*
* <p>The {@code timestamp} is the time at which the event occurred. If unset, it will be set to
* the current time when {@link #emit()} is called.
*/
EventBuilder setTimestamp(long timestamp, TimeUnit unit);

/**
* Set the epoch {@code timestamp} for the event, using the instant.
* Set the epoch {@code timestamp}, using the instant.
*
* <p>The {@code timestamp} is the time at which the event occurred. If unset, it will be set to
* the current time when {@link #emit()} is called.
*/
EventBuilder setTimestamp(Instant instant);

/** Set the context. */
EventBuilder setContext(Context context);

/** Set the severity. */
EventBuilder setSeverity(Severity severity);

/**
* Set the attributes.
*
* <p>Event {@link io.opentelemetry.api.common.Attributes} provide additional details about the
* Event which are not part of the well-defined {@link AnyValue} {@code payload}.
*/
EventBuilder setAttributes(Attributes attributes);

/** Emit an event. */
void emit();
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

package io.opentelemetry.api.incubator.events;

import io.opentelemetry.api.common.Attributes;
import javax.annotation.concurrent.ThreadSafe;

/**
Expand All @@ -20,10 +19,10 @@
* .build();
*
* void doWork() {
* eventLogger.emit("my-namespace.my-event", Attributes.builder()
* eventLogger.builder("my-namespace.my-event")
* .put("key1", "value1")
* .put("key2", "value2")
* .build())
* .emit();
* // do work
* }
* }
Expand All @@ -32,18 +31,6 @@
@ThreadSafe
public interface EventLogger {

/**
* Emit an event.
*
* @param eventName the event name, which identifies the class or type of event. Event with the
* same name are structurally similar to one another. Event names are subject to the same
* naming rules as attribute names. Notably, they are namespaced to avoid collisions. See <a
* href="https://opentelemetry.io/docs/specs/semconv/general/events/">event.name semantic
* conventions</a> for more details.
* @param attributes attributes associated with the event
*/
void emit(String eventName, Attributes attributes);
breedx-splk marked this conversation as resolved.
Show resolved Hide resolved

/**
* Return a {@link EventBuilder} to emit an event.
*
Expand All @@ -52,7 +39,6 @@ public interface EventLogger {
* naming rules as attribute names. Notably, they are namespaced to avoid collisions. See <a
* href="https://opentelemetry.io/docs/specs/semconv/general/events/">event.name semantic
* conventions</a> for more details.
* @param attributes attributes associated with the event
*/
EventBuilder builder(String eventName, Attributes attributes);
EventBuilder builder(String eventName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ static AnyValue<List<AnyValue<?>>> of(AnyValue<?>... value) {
return AnyValueArray.create(value);
}

/** Returns an {@link AnyValue} for the list of {@link AnyValue} values. */
static AnyValue<List<AnyValue<?>>> of(List<AnyValue<?>> value) {
return AnyValueArray.create(value);
}

/**
* Returns an {@link AnyValue} for the array of {@link KeyAnyValue} values. {@link
* KeyAnyValue#getKey()} values should not repeat - duplicates may be dropped.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ static AnyValue<List<AnyValue<?>>> create(AnyValue<?>... value) {
return new AnyValueArray(Collections.unmodifiableList(list));
}

static AnyValue<List<AnyValue<?>>> create(List<AnyValue<?>> value) {
return new AnyValueArray(Collections.unmodifiableList(value));
}

@Override
public AnyValueType getType() {
return AnyValueType.ARRAY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;

import io.opentelemetry.api.common.Attributes;
import org.junit.jupiter.api.Test;

class DefaultEventLoggerProviderTest {
Expand All @@ -33,7 +32,8 @@ void noopEventLoggerProvider_doesNotThrow() {
provider
.eventLoggerBuilder("scope-name")
.build()
.emit("event-name", Attributes.empty()))
.builder("namespace.event-name")
.emit())
.doesNotThrowAnyException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,27 @@
import static org.assertj.core.api.Assertions.assertThatCode;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.logs.Severity;
import io.opentelemetry.context.Context;
import java.time.Instant;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;

class DefaultEventLoggerTest {

@Test
void emit() {
assertThatCode(() -> DefaultEventLogger.getInstance().emit("event-name", Attributes.empty()))
.doesNotThrowAnyException();
assertThatCode(
() ->
DefaultEventLogger.getInstance()
.emit(
"event-domain.event-name",
Attributes.builder().put("key1", "value1").build()))
.doesNotThrowAnyException();
}

@Test
void builder() {
Attributes attributes = Attributes.builder().put("key1", "value1").build();
EventLogger eventLogger = DefaultEventLogger.getInstance();
assertThatCode(
() ->
eventLogger
.builder("com.example.MyEvent", attributes)
.builder("namespace.myEvent")
.put("key", "value")
.setTimestamp(123456L, TimeUnit.NANOSECONDS)
.setTimestamp(Instant.now())
.setContext(Context.current())
.setSeverity(Severity.DEBUG)
.setAttributes(Attributes.empty())
.emit())
.doesNotThrowAnyException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ private static void testLogRecordExporter(LogRecordExporter logRecordExporter) {
.setSeverityText("DEBUG")
.setContext(Context.current())
.emit();
eventLogger.emit("event-name", Attributes.builder().put("key", "value").build());
eventLogger.builder("namespace.event-name").put("key", "value").emit();
}

// Closing triggers flush of processor
Expand Down Expand Up @@ -708,17 +708,18 @@ private static void testLogRecordExporter(LogRecordExporter logRecordExporter) {

// LogRecord via EventLogger.emit(String, Attributes)
io.opentelemetry.proto.logs.v1.LogRecord protoLog2 = ilLogs.getLogRecords(1);
assertThat(protoLog2.getBody().getStringValue()).isEmpty();
assertThat(protoLog2.getAttributesList())
assertThat(protoLog2.getBody().getKvlistValue().getValuesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.name")
.setValue(AnyValue.newBuilder().setStringValue("event-name").build())
.build(),
KeyValue.newBuilder()
.setKey("key")
.setValue(AnyValue.newBuilder().setStringValue("value").build())
.build());
assertThat(protoLog2.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.name")
.setValue(AnyValue.newBuilder().setStringValue("namespace.event-name").build())
.build());
assertThat(protoLog2.getSeverityText()).isEmpty();
assertThat(TraceId.fromBytes(protoLog2.getTraceId().toByteArray()))
.isEqualTo(spanContext.getTraceId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import io.opentelemetry.proto.collector.trace.v1.TraceServiceGrpc;
import io.opentelemetry.proto.common.v1.AnyValue;
import io.opentelemetry.proto.common.v1.KeyValue;
import io.opentelemetry.proto.logs.v1.SeverityNumber;
import io.opentelemetry.proto.metrics.v1.Metric;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import java.util.ArrayList;
Expand Down Expand Up @@ -207,7 +208,8 @@ void configures() throws Exception {
logger.logRecordBuilder().setBody("info log message").setSeverity(Severity.INFO).emit();

EventLogger eventLogger = GlobalEventLoggerProvider.get().eventLoggerBuilder("test").build();
eventLogger.emit("test-name", Attributes.builder().put("cow", "moo").build());
eventLogger.builder("namespace.test-name").put("cow", "moo").emit();
;

openTelemetrySdk.getSdkTracerProvider().forceFlush().join(10, TimeUnit.SECONDS);
openTelemetrySdk.getSdkLoggerProvider().forceFlush().join(10, TimeUnit.SECONDS);
Expand Down Expand Up @@ -329,17 +331,23 @@ void configures() throws Exception {
assertThat(logRecord.getSeverityNumberValue())
.isEqualTo(Severity.INFO.getSeverityNumber());
},
logRecord ->
assertThat(logRecord.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.name")
.setValue(AnyValue.newBuilder().setStringValue("test-name").build())
.build(),
KeyValue.newBuilder()
.setKey("cow")
.setValue(AnyValue.newBuilder().setStringValue("moo").build())
.build()));
logRecord -> {
assertThat(logRecord.getBody().getKvlistValue().getValuesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("cow")
.setValue(AnyValue.newBuilder().setStringValue("moo").build())
.build());
assertThat(logRecord.getSeverityNumber())
.isEqualTo(SeverityNumber.SEVERITY_NUMBER_INFO);
assertThat(logRecord.getAttributesList())
.containsExactlyInAnyOrder(
KeyValue.newBuilder()
.setKey("event.name")
.setValue(
AnyValue.newBuilder().setStringValue("namespace.test-name").build())
.build());
});
}

private static List<KeyValue> getFirstDataPointLabels(Metric metric) {
Expand Down
Loading
Loading