From 778fdbe8d7af1196f82d33b83547642f3d94a7d1 Mon Sep 17 00:00:00 2001 From: samvaity Date: Wed, 16 Jun 2021 15:52:17 -0700 Subject: [PATCH 1/4] add addevent overload --- .../opentelemetry/OpenTelemetryTracer.java | 11 +++- .../OpenTelemetryTracerTest.java | 55 ++++++++++++++++--- .../com/azure/core/util/tracing/Tracer.java | 18 ++++++ 3 files changed, 76 insertions(+), 8 deletions(-) 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 index 77e1a0480cfce..be2a5c98563c4 100644 --- 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 @@ -199,9 +199,18 @@ public Context getSharedSpanBuilder(String spanName, Context context) { */ @Override public void addEvent(String eventName, Map traceEventAttributes, OffsetDateTime timestamp) { + addEvent(eventName, traceEventAttributes, timestamp, Context.NONE); + } + + /** + * {@inheritDoc} + */ + @Override + public void addEvent(String eventName, Map 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); 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 index 79a82c4dc34d6..507aa9c36b2d8 100644 --- 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 = 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 = 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 = 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 = 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 addEventWithoutContext() { + // 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 = recordEventsSpan.toSpanData().getEvents(); + assertNotNull(eventData); + assertEquals(0, 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 index 3102391b23d9e..12765de8fc4ec 100644 --- 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 @@ -251,4 +251,22 @@ default Context getSharedSpanBuilder(String spanName, Context context) { */ default void addEvent(String name, Map attributes, OffsetDateTime timestamp) { } + + /** + * Adds an event to the current span with the provided {@code timestamp} and {@code attributes}. + *

This API does not provide any normalization if provided timestamps are out of range of the current + * span timeline

+ *

Supported attribute values include String, double, boolean, long, String [], double [], long []. + * Any other Object value type and null values will be silently ignored.

+ * + * @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 attributes, OffsetDateTime timestamp, + Context context) { + + } } From d053f5725a24d3b36d79ecfdf6df18bebe3d9aaa Mon Sep 17 00:00:00 2001 From: samvaity Date: Tue, 22 Jun 2021 17:30:54 -0700 Subject: [PATCH 2/4] add defaulting behavior for addEvent overload --- .../azure/core/tracing/opentelemetry/OpenTelemetryTracer.java | 2 +- .../core/tracing/opentelemetry/OpenTelemetryTracerTest.java | 4 ++-- .../src/main/java/com/azure/core/util/tracing/Tracer.java | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) 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 index be2a5c98563c4..7e36340d5d530 100644 --- 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 @@ -199,7 +199,7 @@ public Context getSharedSpanBuilder(String spanName, Context context) { */ @Override public void addEvent(String eventName, Map traceEventAttributes, OffsetDateTime timestamp) { - addEvent(eventName, traceEventAttributes, timestamp, Context.NONE); + addEvent(eventName, traceEventAttributes, timestamp, new Context(PARENT_SPAN_KEY, Span.current())); } /** 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 index 507aa9c36b2d8..473519bc5112b 100644 --- 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 @@ -595,7 +595,7 @@ public void addEventWithChildSpan() { } @Test - public void addEventWithoutContext() { + public void addEventWithoutEventSpanContext() { // Arrange final String eventName = "event-0"; OffsetDateTime eventTime = OffsetDateTime.parse("2021-01-01T18:35:24.00Z"); @@ -608,7 +608,7 @@ public void addEventWithoutContext() { assertEquals(PARENT_SPAN_KEY, recordEventsSpan.getName()); List eventData = recordEventsSpan.toSpanData().getEvents(); assertNotNull(eventData); - assertEquals(0, eventData.size()); + assertEquals(1, eventData.size()); } private static void assertSpanWithExplicitParent(Context updatedContext, String parentSpanId) { 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 index 12765de8fc4ec..0f6fd55599d9d 100644 --- 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 @@ -253,7 +253,8 @@ default void addEvent(String name, Map attributes, OffsetDateTim } /** - * Adds an event to the current span with the provided {@code timestamp} and {@code attributes}. + * Adds an event to the span present in the {@code Context} with the provided {@code timestamp} + * and {@code attributes}. *

This API does not provide any normalization if provided timestamps are out of range of the current * span timeline

*

Supported attribute values include String, double, boolean, long, String [], double [], long []. From 3a229a2b0e31a80d65aefcf9efa0e95f896648d0 Mon Sep 17 00:00:00 2001 From: samvaity Date: Wed, 23 Jun 2021 11:48:44 -0700 Subject: [PATCH 3/4] update to deprecated --- .../src/main/java/com/azure/core/util/tracing/Tracer.java | 2 ++ 1 file changed, 2 insertions(+) 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 index 0f6fd55599d9d..376f0f78e49f7 100644 --- 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,9 @@ 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 attributes, OffsetDateTime timestamp) { } From c77772ba8504451d20d4558b43c1eab9536847fe Mon Sep 17 00:00:00 2001 From: samvaity Date: Wed, 23 Jun 2021 14:33:44 -0700 Subject: [PATCH 4/4] add deprecation suppression --- .../azure-core-tracing-opentelemetry/CHANGELOG.md | 3 ++- .../tracing/opentelemetry/OpenTelemetryTracer.java | 13 +++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md b/sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md index 873f7228e6957..8e414a571494c 100644 --- 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 index 7e36340d5d530..e7c66787ee572 100644 --- 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,7 +127,7 @@ 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; } @@ -135,7 +135,7 @@ public void setAttribute(String key, String value, Context context) { 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,6 +198,7 @@ public Context getSharedSpanBuilder(String spanName, Context context) { * {@inheritDoc} */ @Override + @SuppressWarnings("deprecation") public void addEvent(String eventName, Map traceEventAttributes, OffsetDateTime timestamp) { addEvent(eventName, traceEventAttributes, timestamp, new Context(PARENT_SPAN_KEY, Span.current())); } @@ -212,7 +213,7 @@ public void addEvent(String eventName, Map traceEventAttributes, 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; }