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.

Sign up for GitHub

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

Update addEvent API to attach events to span in Context #22332

Merged
merged 4 commits into from
Jun 23, 2021
Merged

Update addEvent API to attach events to span in Context #22332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
778fdbe
add addevent overload
samvaity Jun 16, 2021
d053f57
add defaulting behavior for addEvent overload
samvaity Jun 23, 2021
3a229a2
update to deprecated
samvaity Jun 23, 2021
c77772b
add deprecation suppression
samvaity Jun 23, 2021
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
3 changes: 2 additions & 1 deletion sdk/core/azure-core-tracing-opentelemetry/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
24 changes: 17 additions & 7 deletions ...opentelemetry/src/main/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}

Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -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;
}

Expand Down
55 changes: 48 additions & 7 deletions ...telemetry/src/test/java/com/azure/core/tracing/opentelemetry/OpenTelemetryTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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();
Expand All @@ -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());

Expand Down
21 changes: 21 additions & 0 deletions sdk/core/azure-core/src/main/java/com/azure/core/util/tracing/Tracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

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

Are we not deprecating this as @trask suggested? I agree that this interface design is centered around passing the Context to each method. So, this method should be deprecated and still use the default behavior to use the current span if someone calls it.

}

/**
* 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) {

}
}