Skip to content

Tracing updates #11116

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

Merged
samvaity merged 5 commits into from
May 18, 2020
Merged

Tracing updates #11116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9b688c6
update log level in tracing lib
samvaity May 12, 2020
21fd4bf
update list of tracers to single tracer
samvaity May 13, 2020
320d89a
update single tracer support amqp
samvaity May 13, 2020
1893a06
update tests
samvaity May 13, 2020
2fe7373
update static variable name
samvaity May 17, 2020
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
69 changes: 36 additions & 33 deletions ...core/azure-core-amqp/src/main/java/com/azure/core/amqp/implementation/TracerProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,31 @@
package com.azure.core.amqp.implementation;

import com.azure.core.amqp.exception.AmqpException;
import com.azure.core.util.tracing.ProcessKind;
import com.azure.core.util.Context;
import com.azure.core.util.logging.ClientLogger;
import com.azure.core.util.tracing.ProcessKind;
import com.azure.core.util.tracing.Tracer;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import reactor.core.publisher.Signal;

import java.util.Objects;

public class TracerProvider {
private final ClientLogger logger = new ClientLogger(TracerProvider.class);
private final List<Tracer> tracers = new ArrayList<>();
private Tracer tracer;

public TracerProvider(Iterable<Tracer> tracers) {
Objects.requireNonNull(tracers, "'tracers' cannot be null.");
tracers.forEach(e -> this.tracers.add(e));
if (tracers.iterator().hasNext()) {
tracer = tracers.iterator().next();
}
}

public boolean isEnabled() {
return tracers.size() > 0;
return tracer != null;
}

/**
* For each tracer plugged into the SDK a new tracing span is created.
* For a plugged tracer implementation a new tracing span is created.
*
* The {@code context} will be checked for containing information about a parent span. If a parent span is found the
* new span will be added as a child, otherwise the span will be created and added to the context and any downstream
Expand All @@ -37,15 +38,14 @@ public boolean isEnabled() {
* @return An updated context object.
*/
public Context startSpan(Context context, ProcessKind processKind) {
Context local = Objects.requireNonNull(context, "'context' cannot be null.");
if (tracer == null) {
return context;
}
Objects.requireNonNull(context, "'context' cannot be null.");
Objects.requireNonNull(processKind, "'processKind' cannot be null.");
String spanName = getSpanName(processKind);

for (Tracer tracer : tracers) {
local = tracer.start(spanName, local, processKind);
}

return local;
return tracer.start(spanName, context, processKind);
}

/**
Expand All @@ -56,6 +56,9 @@ public Context startSpan(Context context, ProcessKind processKind) {
* @param signal The signal indicates the status and contains the metadata we need to end the tracing span.
*/
public void endSpan(Context context, Signal<Void> signal) {
if (tracer == null) {
return;
}
Objects.requireNonNull(context, "'context' cannot be null.");
Objects.requireNonNull(signal, "'signal' cannot be null.");

Expand Down Expand Up @@ -84,48 +87,49 @@ public void endSpan(Context context, Signal<Void> signal) {
}

/**
* For each tracer plugged into the SDK a link is created between the parent tracing span and
* For a plugged tracer implementation a link is created between the parent tracing span and
* the current service call.
*
* @param context Additional metadata that is passed through the call stack.
*/
public void addSpanLinks(Context context) {
if (tracer == null) {
return;
}
Objects.requireNonNull(context, "'context' cannot be null.");
tracers.forEach(tracer -> tracer.addLink(context));
tracer.addLink(context);
}

/**
* For each tracer plugged into the SDK a new context is extracted from the event's diagnostic Id.
* For a plugged tracer implementation a new context is extracted from the event's diagnostic Id.
*
* @param diagnosticId Unique identifier of an external call from producer to the queue.
*/
public Context extractContext(String diagnosticId, Context context) {
Context local = Objects.requireNonNull(context, "'context' cannot be null.");
Objects.requireNonNull(diagnosticId, "'diagnosticId' cannot be null.");
for (Tracer tracer : tracers) {
local = tracer.extractContext(diagnosticId, local);
if (tracer == null) {
return context;
}
return local;
Objects.requireNonNull(context, "'context' cannot be null.");
Objects.requireNonNull(diagnosticId, "'diagnosticId' cannot be null.");
return tracer.extractContext(diagnosticId, context);
}

/**
* For each tracer plugged into the SDK a new context containing the span builder is returned.
* For a plugged tracer implementation a new context containing the span builder is returned.
*
* @param context Additional metadata containing the span name for creating the span builer.
* @param context Additional metadata containing the span name for creating the span builder.
*/
public Context getSharedSpanBuilder(Context context) {
Context local = Objects.requireNonNull(context, "'context' cannot be null.");
String spanName = getSpanName(ProcessKind.SEND);
for (Tracer tracer : tracers) {
local = tracer.getSharedSpanBuilder(spanName, local);
if (tracer == null) {
return context;
}
return local;
Objects.requireNonNull(context, "'context' cannot be null.");
String spanName = getSpanName(ProcessKind.SEND);
return tracer.getSharedSpanBuilder(spanName, context);
}

private void end(String statusMessage, Throwable throwable, Context context) {
for (Tracer tracer : tracers) {
tracer.end(statusMessage, throwable, context);
}
tracer.end(statusMessage, throwable, context);
}

private String getSpanName(ProcessKind processKind) {
Expand All @@ -144,7 +148,6 @@ private String getSpanName(ProcessKind processKind) {
logger.warning("Unknown processKind type: {}", processKind);
break;
}

return spanName;
}
}
41 changes: 4 additions & 37 deletions .../azure-core-amqp/src/test/java/com/azure/core/amqp/implementation/TracerProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,19 @@
import org.mockito.MockitoAnnotations;
import reactor.core.publisher.Signal;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Optional;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;

public class TracerProviderTest {
private static final String METHOD_NAME = "EventHubs.send";

@Mock
private Tracer tracer;
@Mock
private Tracer tracer2;

private List<Tracer> tracers;
private TracerProvider tracerProvider;
Expand All @@ -47,7 +38,7 @@ public class TracerProviderTest {
public void setup() {
MockitoAnnotations.initMocks(this);

tracers = Arrays.asList(tracer, tracer2);
tracers = Collections.singletonList(tracer);
tracerProvider = new TracerProvider(tracers);
}

Expand Down Expand Up @@ -82,34 +73,22 @@ public void startSpanReturnsUpdatedContext() {
// Arrange
final String parentKey = "parent-key";
final String parentValue = "parent-value";
final String childKey = "child-key";
final String childValue = "child-value";
final Context startingContext = Context.NONE;
when(tracer.start(METHOD_NAME, startingContext, ProcessKind.SEND)).thenAnswer(
invocation -> {
Context passed = invocation.getArgument(1, Context.class);
return passed.addData(parentKey, parentValue);
}
);
when(tracer2.start(eq(METHOD_NAME), any(), eq(ProcessKind.SEND))).thenAnswer(
invocation -> {
Context passed = invocation.getArgument(1, Context.class);
return passed.addData(childKey, childValue);
}
);

// Act
final Context updatedContext = tracerProvider.startSpan(startingContext, ProcessKind.SEND);

// Assert
// Want to ensure that the data added to the parent and child are available.
// Want to ensure that the data added to the parent are available.
final Optional<Object> parentData = updatedContext.getData(parentKey);
Assertions.assertTrue(parentData.isPresent());
Assertions.assertEquals(parentValue, parentData.get());

final Optional<Object> childData = updatedContext.getData(childKey);
Assertions.assertTrue(childData.isPresent());
Assertions.assertEquals(childValue, childData.get());
}

@Test
Expand Down Expand Up @@ -202,8 +181,6 @@ public void getSpanBuilderReturnsUpdatedContext() {
final String spanBuilderKey = "spanBuilder-key";
final String spanBuilderValue = "spanBuilder-value";

final String spanBuilderKey1 = "spanBuilder-key1";
final String spanBuilderValue1 = "spanBuilder-value1";
final Context startingContext = Context.NONE;

when(tracer.getSharedSpanBuilder(anyString(), any())).thenAnswer(
Expand All @@ -212,12 +189,6 @@ public void getSpanBuilderReturnsUpdatedContext() {
return passed.addData(spanBuilderKey, spanBuilderValue);
}
);
when(tracer2.getSharedSpanBuilder(anyString(), any())).thenAnswer(
invocation -> {
Context passed = invocation.getArgument(1, Context.class);
return passed.addData(spanBuilderKey1, spanBuilderValue1);
}
);

// Act
final Context updatedContext = tracerProvider.getSharedSpanBuilder(startingContext);
Expand All @@ -226,9 +197,5 @@ public void getSpanBuilderReturnsUpdatedContext() {
final Optional<Object> spanBuilderData = updatedContext.getData(spanBuilderKey);
Assertions.assertTrue(spanBuilderData.isPresent());
Assertions.assertEquals(spanBuilderValue, spanBuilderData.get());

final Optional<Object> spanBuilderData1 = updatedContext.getData(spanBuilderKey1);
Assertions.assertTrue(spanBuilderData1.isPresent());
Assertions.assertEquals(spanBuilderValue1, spanBuilderData1.get());
}
}
2 changes: 1 addition & 1 deletion ...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 @@ -311,7 +311,7 @@ private Builder getSpanBuilder(String spanName, Context context) {
private <T> T getOrDefault(Context context, String key, T defaultValue, Class<T> clazz) {
final Optional<Object> optional = context.getData(key);
final Object result = optional.filter(value -> clazz.isAssignableFrom(value.getClass())).orElseGet(() -> {
logger.warning("Could not extract key '{}' of type '{}' from context.", key, clazz);
logger.verbose("Could not extract key '{}' of type '{}' from context.", key, clazz);
return defaultValue;
});

Expand Down
46 changes: 23 additions & 23 deletions sdk/core/azure-core/src/main/java/com/azure/core/util/tracing/TracerProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@

import com.azure.core.util.Context;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Iterator;
import java.util.ServiceLoader;

/**
Expand All @@ -17,14 +15,14 @@
*/
public final class TracerProxy {

private static final List<Tracer> TRACERS;
private static Tracer tracer;

static {
ServiceLoader<Tracer> serviceLoader = ServiceLoader.load(Tracer.class);
List<Tracer> tracers = new ArrayList<>();
for (Tracer tracer : serviceLoader) {
tracers.add(tracer);
Iterator<?> iterator = serviceLoader.iterator();
if (iterator.hasNext()) {
tracer = serviceLoader.iterator().next();
}
TRACERS = Collections.unmodifiableList(tracers);
}

private TracerProxy() {
Expand All @@ -33,7 +31,7 @@ private TracerProxy() {

/**
* A new tracing span is created for each {@link Tracer tracer} plugged into the SDK.
*
* <p>
* The {@code context} will be checked for information about a parent span. If a parent span is found, the new span
* will be added as a child. Otherwise, the parent span will be created and added to the {@code context} and any
* downstream {@code start()} calls will use the created span as the parent.
Expand All @@ -44,35 +42,39 @@ private TracerProxy() {
* @return An updated {@link Context} object.
*/
public static Context start(String methodName, Context context) {
Context local = context;
for (Tracer tracer : TRACERS) {
local = tracer.start(methodName, local);
if (tracer == null) {
return context;
}

return local;
return tracer.start(methodName, context);
}

/**
* For each {@link Tracer tracer} plugged into the SDK, the key-value pair metadata is added to its current span. If
* For the plugged in {@link Tracer tracer}, the key-value pair metadata is added to its current span. If
* the {@code context} does not contain a span, then no metadata is added.
*
* @param key Name of the metadata.
* @param value Value of the metadata.
* @param context Additional metadata that is passed through the call stack.
*/
public static void setAttribute(String key, String value, Context context) {
TRACERS.forEach(tracer -> tracer.setAttribute(key, value, context));
if (tracer == null) {
return;
}
tracer.setAttribute(key, value, context);
}

/**
* For each {@link Tracer tracer} plugged into the SDK, its current tracing span is marked as completed.
* For the plugged in {@link Tracer tracer}, its current tracing span is marked as completed.
*
* @param responseCode Response status code if the span is in a HTTP call context.
* @param error {@link Throwable} that happened during the span or {@code null} if no exception occurred.
* @param context Additional metadata that is passed through the call stack.
*/
public static void end(int responseCode, Throwable error, Context context) {
TRACERS.forEach(tracer -> tracer.end(responseCode, error, context));
if (tracer == null) {
return;
}
tracer.end(responseCode, error, context);
}

/**
Expand All @@ -84,11 +86,9 @@ public static void end(int responseCode, Throwable error, Context context) {
* @return An updated {@link Context} object.
*/
public static Context setSpanName(String spanName, Context context) {
Copy link
Member

@srnagar srnagar May 13, 2020

Choose a reason for hiding this comment

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

Why does this API return Context? It's inconsistent with setAttribute() API which returns a void. Or, should setAttribute() return Context too since start() is also returning Context?

Copy link
Member Author

@samvaity samvaity May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

start() should return a Context as we update the context to hold the span that it just created/started.
setAttribute() shouldn't because we do not have anything to return. We are actually just using the span information from the provided Context.
setSpanName() I think this method is redundant to set the span name as we already use the provided context for that. I see it is only used in RestProxy here.

Context local = context;
for (Tracer tracer : TRACERS) {
local = tracer.setSpanName(spanName, context);
if (tracer == null) {
return context;
}

return local;
return tracer.setSpanName(spanName, context);
}
}