Update addEvent API to attach events to span in Context (Azure#22332)

samvaity · web-flow · commit 16c2d5acca00 · 2021-06-23T15:26:59.000-07:00
diff --git a/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md b/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md
@@ -1,7 +1,8 @@
 # Release History
 
 ## 1.0.0-beta.12 (Unreleased)
-
+### Key Bugs Fixed
+- Fixed `addEvent` API to use the span provided in the context rather than `Span.current()`.
 
 ## 1.0.0-beta.11 (2021-06-07)
 
diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracer.java b/sdk/core/azure-core-tracing-opentelemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracer.java
@@ -127,15 +127,15 @@ public void end(int responseCode, Throwable throwable, Context context) {
     public void setAttribute(String key, String value, Context context) {
         Objects.requireNonNull(context, "'context' cannot be null");
         if (CoreUtils.isNullOrEmpty(value)) {
-            logger.warning("Failed to set span attribute since value is null or empty.");
+            logger.verbose("Failed to set span attribute since value is null or empty.");
             return;
         }
 
         final Span span = getOrDefault(context, PARENT_SPAN_KEY, null, Span.class);
         if (span != null) {
             span.setAttribute(key, value);
         } else {
-            logger.warning("Failed to find span to add attribute.");
+            logger.verbose("Failed to find span to add attribute.");
         }
     }
 
@@ -154,7 +154,7 @@ public Context setSpanName(String spanName, Context context) {
     public void end(String statusMessage, Throwable throwable, Context context) {
         Span span = getOrDefault(context, PARENT_SPAN_KEY, null, Span.class);
         if (span == null) {
-            logger.warning("Failed to find span to end it.");
+            logger.verbose("Failed to find span to end it.");
             return;
         }
 
@@ -169,13 +169,13 @@ public void end(String statusMessage, Throwable throwable, Context context) {
     public void addLink(Context context) {
         final SpanBuilder spanBuilder = getOrDefault(context, SPAN_BUILDER_KEY, null, SpanBuilder.class);
         if (spanBuilder == null) {
-            logger.warning("Failed to find spanBuilder to link it.");
+            logger.verbose("Failed to find spanBuilder to link it.");
             return;
         }
 
         final SpanContext spanContext = getOrDefault(context, SPAN_CONTEXT_KEY, null, SpanContext.class);
         if (spanContext == null) {
-            logger.warning("Failed to find span context to link it.");
+            logger.verbose("Failed to find span context to link it.");
             return;
         }
         spanBuilder.addLink(spanContext);
@@ -198,12 +198,22 @@ public Context getSharedSpanBuilder(String spanName, Context context) {
      * {@inheritDoc}
      */
     @Override
+    @SuppressWarnings("deprecation")
     public void addEvent(String eventName, Map<String, Object> traceEventAttributes, OffsetDateTime timestamp) {
+        addEvent(eventName, traceEventAttributes, timestamp, new Context(PARENT_SPAN_KEY, Span.current()));
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public void addEvent(String eventName, Map<String, Object> traceEventAttributes, OffsetDateTime timestamp,
+                         Context context) {
         Objects.requireNonNull(eventName, "'eventName' cannot be null.");
 
-        Span currentSpan = Span.current();
+        Span currentSpan = getOrDefault(context, PARENT_SPAN_KEY, null, Span.class);
         if (currentSpan == null) {
-            logger.info("Failed to find a starting span to associate the {} with.", eventName);
+            logger.verbose("Failed to find a starting span to associate the {} with.", eventName);
             return;
         }
 
diff --git a/sdk/core/azure-core-tracing-opentelemetry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracerTest.java b/sdk/core/azure-core-tracing-opentelemetry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracerTest.java
@@ -293,7 +293,7 @@ public void startSpanOverloadNullPointerException() {
     @Test
     public void addLinkTest() {
         // Arrange
-        SpanBuilder span = tracer.spanBuilder("parent-span");
+        SpanBuilder span = tracer.spanBuilder(PARENT_SPAN_KEY);
         Span toLinkSpan = tracer.spanBuilder("new test span").startSpan();
 
         Context spanContext = new Context(
@@ -317,7 +317,7 @@ public void addLinkTest() {
     @Test
     public void addLinkNoSpanContextTest() {
         // Arrange
-        SpanBuilder span = tracer.spanBuilder("parent-span");
+        SpanBuilder span = tracer.spanBuilder(PARENT_SPAN_KEY);
 
         // Act
         openTelemetryTracer.addLink(new Context(SPAN_BUILDER_KEY, span));
@@ -331,7 +331,7 @@ public void addLinkNoSpanContextTest() {
     @Test
     public void addLinkNoSpanToLinkTest() {
         // Arrange
-        SpanBuilder span = tracer.spanBuilder("parent-span");
+        SpanBuilder span = tracer.spanBuilder(PARENT_SPAN_KEY);
 
         // Act
         openTelemetryTracer.addLink(Context.NONE);
@@ -489,10 +489,11 @@ public void addEventWithNonNullEventName() {
         final String eventName = "event-0";
 
         // Act
-        openTelemetryTracer.addEvent(eventName, null, null);
+        openTelemetryTracer.addEvent(eventName, null, null, tracingContext);
 
         // Assert
         final ReadableSpan recordEventsSpan = (ReadableSpan) tracingContext.getData(PARENT_SPAN_KEY).get();
+        assertEquals(PARENT_SPAN_KEY, recordEventsSpan.getName());
         List<EventData> eventData = recordEventsSpan.toSpanData().getEvents();
         assertNotNull(eventData);
         assertEquals(1, eventData.size());
@@ -513,10 +514,11 @@ public void addEventWithAttributes() {
             }};
 
         // Act
-        openTelemetryTracer.addEvent(eventName, input, null);
+        openTelemetryTracer.addEvent(eventName, input, null, tracingContext);
 
         // Assert
         final ReadableSpan recordEventsSpan = (ReadableSpan) tracingContext.getData(PARENT_SPAN_KEY).get();
+        assertEquals(PARENT_SPAN_KEY, recordEventsSpan.getName());
         List<EventData> eventData = recordEventsSpan.toSpanData().getEvents();
         assertNotNull(eventData);
         assertEquals(1, eventData.size());
@@ -541,10 +543,11 @@ public void addEventWithTimeSpecification() {
         OffsetDateTime eventTime = OffsetDateTime.parse("2021-01-01T18:35:24.00Z");
 
         // Act
-        openTelemetryTracer.addEvent(eventName, null, eventTime);
+        openTelemetryTracer.addEvent(eventName, null, eventTime, tracingContext);
 
         // Assert
         final ReadableSpan recordEventsSpan = (ReadableSpan) tracingContext.getData(PARENT_SPAN_KEY).get();
+        assertEquals(PARENT_SPAN_KEY, recordEventsSpan.getName());
         List<EventData> eventData = recordEventsSpan.toSpanData().getEvents();
         assertNotNull(eventData);
         assertEquals(1, eventData.size());
@@ -560,7 +563,7 @@ public void addEventAfterSpanEnd() {
 
         // Act
         parentSpan.end();
-        openTelemetryTracer.addEvent(eventName, null, null);
+        openTelemetryTracer.addEvent(eventName, null, null, tracingContext);
 
         // Assert
         final ReadableSpan recordEventsSpan = (ReadableSpan) tracingContext.getData(PARENT_SPAN_KEY).get();
@@ -570,6 +573,44 @@ public void addEventAfterSpanEnd() {
         assertEquals(0, eventData.size());
     }
 
+    @Test
+    public void addEventWithChildSpan() {
+        // Arrange
+        tracingContext = openTelemetryTracer.start("child-span-1", tracingContext);
+        final String eventName = "event-0";
+        OffsetDateTime eventTime = OffsetDateTime.parse("2021-01-01T18:35:24.00Z");
+
+        // Act
+        openTelemetryTracer.addEvent(eventName, null, eventTime, tracingContext);
+
+        // Assert
+        final ReadableSpan recordEventsSpan = (ReadableSpan) tracingContext.getData(PARENT_SPAN_KEY).get();
+        assertEquals("child-span-1", recordEventsSpan.getName());
+        List<EventData> eventData = recordEventsSpan.toSpanData().getEvents();
+        assertNotNull(eventData);
+        assertEquals(1, eventData.size());
+        assertEquals(eventName, eventData.get(0).getName());
+        assertEquals(eventTime,
+            OffsetDateTime.ofInstant(Instant.ofEpochMilli(eventData.get(0).getEpochNanos() / 1000000), ZoneOffset.UTC));
+    }
+
+    @Test
+    public void addEventWithoutEventSpanContext() {
+        // Arrange
+        final String eventName = "event-0";
+        OffsetDateTime eventTime = OffsetDateTime.parse("2021-01-01T18:35:24.00Z");
+
+        // Act
+        openTelemetryTracer.addEvent(eventName, null, eventTime);
+
+        // Assert
+        final ReadableSpan recordEventsSpan = (ReadableSpan) tracingContext.getData(PARENT_SPAN_KEY).get();
+        assertEquals(PARENT_SPAN_KEY, recordEventsSpan.getName());
+        List<EventData> eventData = recordEventsSpan.toSpanData().getEvents();
+        assertNotNull(eventData);
+        assertEquals(1, eventData.size());
+    }
+
     private static void assertSpanWithExplicitParent(Context updatedContext, String parentSpanId) {
         assertNotNull(updatedContext.getData(PARENT_SPAN_KEY).get());
 
diff --git a/sdk/core/azure-core/src/main/java/com/azure/core/util/tracing/Tracer.java b/sdk/core/azure-core/src/main/java/com/azure/core/util/tracing/Tracer.java
@@ -248,7 +248,28 @@ default Context getSharedSpanBuilder(String spanName, Context context) {
      * @param attributes the additional attributes to be set for the event.
      * @param timestamp The instant, in UTC, at which the event will be associated to the span.
      * @throws NullPointerException if {@code eventName} is {@code null}.
+     * @deprecated Use {@link #addEvent(String, Map, OffsetDateTime, Context)}
      */
+    @Deprecated
     default void addEvent(String name, Map<String, Object> attributes, OffsetDateTime timestamp) {
     }
+
+    /**
+     * Adds an event to the span present in the {@code Context} with the provided {@code timestamp}
+     * and {@code attributes}.
+     * <p>This API does not provide any normalization if provided timestamps are out of range of the current
+     * span timeline</p>
+     * <p>Supported attribute values include String, double, boolean, long, String [], double [], long [].
+     * Any other Object value type and null values will be silently ignored.</p>
+     *
+     * @param name the name of the event.
+     * @param attributes the additional attributes to be set for the event.
+     * @param timestamp The instant, in UTC, at which the event will be associated to the span.
+     * @param context the call metadata containing information of the span to which the event should be associated with.
+     * @throws NullPointerException if {@code eventName} is {@code null}.
+     */
+    default void addEvent(String name, Map<String, Object> attributes, OffsetDateTime timestamp,
+                          Context context) {
+
+    }
 }
