Remove emit overload, align setTimestamp with existing conventions

open-telemetry · Nov 9, 2023 · 21585cf · 21585cf
1 parent 27e94a8
commit 21585cf
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 120 deletions.
diff --git a/api/events/src/main/java/io/opentelemetry/api/events/DefaultEventEmitter.java b/api/events/src/main/java/io/opentelemetry/api/events/DefaultEventEmitter.java
@@ -6,6 +6,8 @@
 package io.opentelemetry.api.events;
 
 import io.opentelemetry.api.common.Attributes;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
 
 class DefaultEventEmitter implements EventEmitter {
 
@@ -17,9 +19,6 @@ static EventEmitter getInstance() {
     return INSTANCE;
   }
 
-  @Override
-  public void emit(long epochNanos, String eventName, Attributes attributes) {}
-
   @Override
   public void emit(String eventName, Attributes attributes) {}
 
@@ -33,7 +32,12 @@ private static class NoOpEventBuilder implements EventBuilder {
     public static final EventBuilder INSTANCE = new NoOpEventBuilder();
 
     @Override
-    public EventBuilder setTimestamp(long epochNanos) {
+    public EventBuilder setTimestamp(long timestamp, TimeUnit unit) {
+      return this;
+    }
+
+    @Override
+    public EventBuilder setTimestamp(Instant instant) {
       return this;
     }
 

diff --git a/api/events/src/main/java/io/opentelemetry/api/events/EventBuilder.java b/api/events/src/main/java/io/opentelemetry/api/events/EventBuilder.java
@@ -5,11 +5,27 @@
 
 package io.opentelemetry.api.events;
 
-/** The EventBuilder is used to emit() events. */
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
+
+/** The EventBuilder is used to {@link #emit()} events. */
 public interface EventBuilder {
 
-  /** Sets the timestamp for the event. */
-  EventBuilder setTimestamp(long epochNanos);
+  /**
+   * Set the epoch {@code timestamp} for the event, 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.
+   *
+   * <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);
 
   /** Emit an event. */
   void emit();

diff --git a/api/events/src/main/java/io/opentelemetry/api/events/EventEmitter.java b/api/events/src/main/java/io/opentelemetry/api/events/EventEmitter.java
@@ -35,22 +35,18 @@ public interface EventEmitter {
   /**
    * Emit an event.
    *
-   * @param epochNanos The time at which the event happened, in epoch nanoseconds.
    * @param eventName the event name, which acts as a classifier for events. Within a particular
    *     event domain, event name defines a particular class or type of event.
    * @param attributes attributes associated with the event
    */
-  @SuppressWarnings("InconsistentOverloads")
-  void emit(long epochNanos, String eventName, Attributes attributes);
+  void emit(String eventName, Attributes attributes);
 
   /**
-   * Emit an event.
+   * Return a {@link EventBuilder} to emit an event.
    *
    * @param eventName the event name, which acts as a classifier for events. Within a particular
    *     event domain, event name defines a particular class or type of event.
    * @param attributes attributes associated with the event
    */
-  void emit(String eventName, Attributes attributes);
-
   EventBuilder builder(String eventName, Attributes attributes);
 }
diff --git a/api/events/src/test/java/io/opentelemetry/api/events/DefaultEventEmitterTest.java b/api/events/src/test/java/io/opentelemetry/api/events/DefaultEventEmitterTest.java
@@ -8,6 +8,8 @@
 import static org.assertj.core.api.Assertions.assertThatCode;
 
 import io.opentelemetry.api.common.Attributes;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.Test;
 
 class DefaultEventEmitterTest {
@@ -23,35 +25,17 @@ void emit() {
         .doesNotThrowAnyException();
   }
 
-  @Test
-  void emitWithTimestamp() {
-    EventEmitter emitter = DefaultEventEmitter.getInstance();
-    Attributes attributes = Attributes.builder().put("key1", "value1").build();
-    assertThatCode(() -> emitter.emit(System.nanoTime(), "event-name", attributes))
-        .doesNotThrowAnyException();
-  }
-
   @Test
   void builder() {
     Attributes attributes = Attributes.builder().put("key1", "value1").build();
     EventEmitter emitter = DefaultEventEmitter.getInstance();
-    assertThatCode(() -> emitter.builder("myEvent", attributes).setTimestamp(123456L).emit())
-        .doesNotThrowAnyException();
-  }
-
-  @Test
-  void builderWithName() {
-    Attributes attributes = Attributes.builder().put("key1", "value1").build();
-    EventEmitter emitter = DefaultEventEmitter.getInstance();
-    assertThatCode(() -> emitter.builder("myEvent", attributes).setTimestamp(123456L).emit())
-        .doesNotThrowAnyException();
-  }
-
-  @Test
-  void builderWithNameAndAttrs() {
-    Attributes attributes = Attributes.builder().put("key1", "value1").build();
-    EventEmitter emitter = DefaultEventEmitter.getInstance();
-    assertThatCode(() -> emitter.builder("myEvent", attributes).setTimestamp(123456L).emit())
+    assertThatCode(
+            () ->
+                emitter
+                    .builder("myEvent", attributes)
+                    .setTimestamp(123456L, TimeUnit.NANOSECONDS)
+                    .setTimestamp(Instant.now())
+                    .emit())
         .doesNotThrowAnyException();
   }
 }
diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventBuilder.java
@@ -5,51 +5,37 @@
 
 package io.opentelemetry.sdk.logs.internal;
 
-import static io.opentelemetry.sdk.logs.internal.SdkEventEmitterProvider.EVENT_DOMAIN;
-import static io.opentelemetry.sdk.logs.internal.SdkEventEmitterProvider.EVENT_NAME;
-
-import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.events.EventBuilder;
-import io.opentelemetry.api.logs.Logger;
-import io.opentelemetry.sdk.common.Clock;
+import io.opentelemetry.api.logs.LogRecordBuilder;
+import java.time.Instant;
 import java.util.concurrent.TimeUnit;
 
 class SdkEventBuilder implements EventBuilder {
-  private final Clock clock;
-  private final Logger delegateLogger;
+  private final LogRecordBuilder logRecordBuilder;
   private final String eventDomain;
   private final String eventName;
-  private final Attributes attributes;
-  private long epochNanos = Long.MIN_VALUE;
 
-  SdkEventBuilder(
-      Clock clock,
-      Logger delegateLogger,
-      String eventDomain,
-      String eventName,
-      Attributes attributes) {
-    this.clock = clock;
-    this.delegateLogger = delegateLogger;
+  SdkEventBuilder(LogRecordBuilder logRecordBuilder, String eventDomain, String eventName) {
+    this.logRecordBuilder = logRecordBuilder;
     this.eventDomain = eventDomain;
     this.eventName = eventName;
-    this.attributes = attributes;
   }
 
   @Override
-  public EventBuilder setTimestamp(long epochNanos) {
-    this.epochNanos = epochNanos;
+  public EventBuilder setTimestamp(long timestamp, TimeUnit unit) {
+    this.logRecordBuilder.setTimestamp(timestamp, unit);
+    return this;
+  }
+
+  @Override
+  public EventBuilder setTimestamp(Instant instant) {
+    this.logRecordBuilder.setTimestamp(instant);
     return this;
   }
 
   @Override
   public void emit() {
-    long timestamp = epochNanos == Long.MIN_VALUE ? clock.now() : epochNanos;
-    delegateLogger
-        .logRecordBuilder()
-        .setTimestamp(timestamp, TimeUnit.NANOSECONDS)
-        .setAllAttributes(attributes)
-        .setAttribute(EVENT_DOMAIN, eventDomain)
-        .setAttribute(EVENT_NAME, eventName)
-        .emit();
+    SdkEventEmitterProvider.addEventNameAndDomain(logRecordBuilder, eventDomain, eventName);
+    logRecordBuilder.emit();
   }
 }
diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventEmitterProvider.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkEventEmitterProvider.java
@@ -11,10 +11,12 @@
 import io.opentelemetry.api.events.EventEmitter;
 import io.opentelemetry.api.events.EventEmitterBuilder;
 import io.opentelemetry.api.events.EventEmitterProvider;
+import io.opentelemetry.api.logs.LogRecordBuilder;
 import io.opentelemetry.api.logs.Logger;
 import io.opentelemetry.api.logs.LoggerBuilder;
 import io.opentelemetry.api.logs.LoggerProvider;
 import io.opentelemetry.sdk.common.Clock;
+import java.util.concurrent.TimeUnit;
 
 /**
  * SDK implementation for {@link EventEmitterProvider}.
@@ -113,19 +115,29 @@ private SdkEventEmitter(Clock clock, Logger delegateLogger, String eventDomain)
 
     @Override
     public EventBuilder builder(String eventName, Attributes attributes) {
-      return new SdkEventBuilder(clock, delegateLogger, eventDomain, eventName, attributes);
+      return new SdkEventBuilder(
+          delegateLogger
+              .logRecordBuilder()
+              .setTimestamp(clock.now(), TimeUnit.NANOSECONDS)
+              .setAllAttributes(attributes),
+          eventDomain,
+          eventName);
     }
 
     @Override
     public void emit(String eventName, Attributes attributes) {
-      emit(clock.now(), eventName, attributes);
+      LogRecordBuilder logRecordBuilder =
+          delegateLogger
+              .logRecordBuilder()
+              .setTimestamp(clock.now(), TimeUnit.NANOSECONDS)
+              .setAllAttributes(attributes);
+      addEventNameAndDomain(logRecordBuilder, eventDomain, eventName);
+      logRecordBuilder.emit();
     }
+  }
 
-    @Override
-    public void emit(long epochNanos, String eventName, Attributes attributes) {
-      new SdkEventBuilder(clock, delegateLogger, eventDomain, eventName, attributes)
-          .setTimestamp(epochNanos)
-          .emit();
-    }
+  static void addEventNameAndDomain(
+      LogRecordBuilder logRecordBuilder, String eventDomain, String eventName) {
+    logRecordBuilder.setAttribute(EVENT_DOMAIN, eventDomain).setAttribute(EVENT_NAME, eventName);
   }
 }
diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/internal/SdkEventBuilderTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/internal/SdkEventBuilderTest.java
@@ -12,10 +12,9 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.logs.LogRecordBuilder;
-import io.opentelemetry.api.logs.Logger;
-import io.opentelemetry.sdk.common.Clock;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.Test;
 
 class SdkEventBuilderTest {
@@ -24,21 +23,20 @@ class SdkEventBuilderTest {
   void emit() {
     String eventDomain = "mydomain";
     String eventName = "banana";
-    Attributes attributes = Attributes.of(stringKey("foo"), "bar");
 
-    Logger logger = mock(Logger.class);
     LogRecordBuilder logRecordBuilder = mock(LogRecordBuilder.class);
-    when(logger.logRecordBuilder()).thenReturn(logRecordBuilder);
     when(logRecordBuilder.setTimestamp(anyLong(), any())).thenReturn(logRecordBuilder);
     when(logRecordBuilder.setAttribute(any(), any())).thenReturn(logRecordBuilder);
-    when(logRecordBuilder.setAllAttributes(any())).thenReturn(logRecordBuilder);
 
-    new SdkEventBuilder(Clock.getDefault(), logger, eventDomain, eventName, attributes)
-        .setTimestamp(123456L)
+    Instant instant = Instant.now();
+    new SdkEventBuilder(logRecordBuilder, eventDomain, eventName)
+        .setTimestamp(123456L, TimeUnit.NANOSECONDS)
+        .setTimestamp(instant)
         .emit();
-    verify(logRecordBuilder).setAllAttributes(attributes);
     verify(logRecordBuilder).setAttribute(stringKey("event.domain"), eventDomain);
     verify(logRecordBuilder).setAttribute(stringKey("event.name"), eventName);
+    verify(logRecordBuilder).setTimestamp(123456L, TimeUnit.NANOSECONDS);
+    verify(logRecordBuilder).setTimestamp(instant);
     verify(logRecordBuilder).emit();
   }
 }
diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/internal/SdkEventEmitterProviderTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/internal/SdkEventEmitterProviderTest.java
@@ -95,48 +95,14 @@ void emit_NoDomain() {
                 .build());
   }
 
-  @Test
-  void emit_withTimestamp() {
-    long yesterday = System.nanoTime() - TimeUnit.DAYS.toNanos(1);
-    Attributes attributes = Attributes.of(stringKey("foo"), "bar");
-
-    EventEmitter emitter = eventEmitterProvider.eventEmitterBuilder("test-scope").build();
-
-    emitter.emit(yesterday, "testing", attributes);
-
-    verifySeen(yesterday, attributes);
-  }
-
-  @Test
-  void builderWithNameAndAttributes() {
-    long yesterday = System.nanoTime() - TimeUnit.DAYS.toNanos(1);
-    Attributes attributes = Attributes.of(stringKey("foo"), "bar");
-
-    EventEmitter emitter = eventEmitterProvider.eventEmitterBuilder("test-scope").build();
-
-    emitter.builder("testing", attributes).setTimestamp(yesterday).emit();
-    verifySeen(yesterday, attributes);
-  }
-
-  @Test
-  void builderWithName() {
-    long yesterday = System.nanoTime() - TimeUnit.DAYS.toNanos(1);
-    Attributes attributes = Attributes.of(stringKey("foo"), "bar");
-
-    EventEmitter emitter = eventEmitterProvider.eventEmitterBuilder("test-scope").build();
-
-    emitter.builder("testing", attributes).setTimestamp(yesterday).emit();
-    verifySeen(yesterday, attributes);
-  }
-
   @Test
   void builder() {
     long yesterday = System.nanoTime() - TimeUnit.DAYS.toNanos(1);
     Attributes attributes = Attributes.of(stringKey("foo"), "bar");
 
     EventEmitter emitter = eventEmitterProvider.eventEmitterBuilder("test-scope").build();
 
-    emitter.builder("testing", attributes).setTimestamp(yesterday).emit();
+    emitter.builder("testing", attributes).setTimestamp(yesterday, TimeUnit.NANOSECONDS).emit();
     verifySeen(yesterday, attributes);
   }