Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Commit

Permalink
Make GlobalTracer uses more threadsafe (#269)
Browse files Browse the repository at this point in the history 
* Make GlobalTracer uses more threadsafe

As discussed in #186 replace `void register(Tracer)` with a more
atomic `boolean registerIfAbsent(Tracer)` method.

* Throw NPE (with message) for registerIfAbsent(null) calls

* Add '@deprecated' annotation + replace nested if by '&&'.

* Remove registerIfAbsent(Tracer) as unwanted duplicate

* Replace TracerSupplier interface with Callable<Tracer>

* Document thrown exceptions

* Combine javadoc for checked + unchecked exceptions from provider

* Wording for rethrown exceptions from provider

* Add brackets around 'if' statement

* Add unit tests for registerIfAbsent + restore tests for register methods

* Clarify wording used in unit test
  • Loading branch information
@sjoerdtalsma @pavolloffay
sjoerdtalsma authored and pavolloffay committed Jul 10, 2018
1 parent cb40981 commit 67f080c
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 42 deletions.
99 changes: 72 additions & 27 deletions opentracing-util/src/main/java/io/opentracing/util/GlobalTracer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@

import io.opentracing.ScopeManager;
import io.opentracing.Span;
import io.opentracing.noop.NoopTracer;
import io.opentracing.noop.NoopTracerFactory;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.noop.NoopTracer;
import io.opentracing.noop.NoopTracerFactory;
import io.opentracing.propagation.Format;

import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.concurrent.Callable;

/**
* Global tracer that forwards all methods to another tracer that can be
Expand All @@ -49,7 +48,6 @@
* default value.
*/
public final class GlobalTracer implements Tracer {
private static final Logger LOGGER = Logger.getLogger(GlobalTracer.class.getName());

/**
* Singleton instance.
Expand Down Expand Up @@ -84,43 +82,75 @@ public static Tracer get() {
return INSTANCE;
}

/**
* Identify whether a {@link Tracer} has previously been registered.
* <p>
* This check is useful in scenarios where more than one component may be responsible
* for registering a tracer. For example, when using a Java Agent, it will need to determine
* if the application has already registered a tracer, and if not attempt to resolve and
* register one itself.
*
* @return Whether a tracer has been registered
*/
public static synchronized boolean isRegistered() {
return !(GlobalTracer.tracer instanceof NoopTracer);
}

/**
* Register a {@link Tracer} to back the behaviour of the {@link #get() global tracer}.
* <p>
* Registration is a one-time operation, attempting to call it more often will result in a runtime exception.
* The tracer is provided through a {@linkplain Callable} that will only be called if the global tracer is absent.
* Registration is a one-time operation. Once a tracer has been registered, all attempts at re-registering
* will return {@code false}.
* <p>
* Every application intending to use the global tracer is responsible for registering it once
* during its initialization.
*
* @param tracer Tracer to use as global tracer.
* @throws RuntimeException if there is already a current tracer registered
* @param provider Provider for the tracer to use as global tracer.
* @return {@code true} if the provided tracer was registered as a result of this call,
* {@code false} otherwise.
* @throws NullPointerException if the tracer provider is {@code null} or provides a {@code null} Tracer.
* @throws RuntimeException any exception thrown by the provider gets rethrown,
* checked exceptions will be wrapped into appropriate runtime exceptions.
*/
public static synchronized void register(final Tracer tracer) {
if (tracer == null) {
throw new NullPointerException("Cannot register GlobalTracer <null>.");
}
if (tracer instanceof GlobalTracer) {
LOGGER.log(Level.FINE, "Attempted to register the GlobalTracer as delegate of itself.");
return; // no-op
}
if (isRegistered() && !GlobalTracer.tracer.equals(tracer)) {
throw new IllegalStateException("There is already a current global Tracer registered.");
public static synchronized boolean registerIfAbsent(final Callable<Tracer> provider) {
requireNonNull(provider, "Cannot register GlobalTracer from provider <null>.");
if (!isRegistered()) {
try {
final Tracer suppliedTracer = requireNonNull(provider.call(), "Cannot register GlobalTracer <null>.");
if (!(suppliedTracer instanceof GlobalTracer)) {
GlobalTracer.tracer = suppliedTracer;
return true;
}
} catch (RuntimeException rte) {
throw rte; // Re-throw as-is
} catch (Exception ex) {
throw new IllegalStateException("Exception obtaining tracer from provider: " + ex.getMessage(), ex);
}
}
GlobalTracer.tracer = tracer;
return false;
}

/**
* Identify whether a {@link Tracer} has previously been registered.
* Register a {@link Tracer} to back the behaviour of the {@link #get() global tracer}.
* <p>
* This check is useful in scenarios where more than one component may be responsible
* for registering a tracer. For example, when using a Java Agent, it will need to determine
* if the application has already registered a tracer, and if not attempt to resolve and
* register one itself.
* Registration is a one-time operation, attempting to call it more often will result in a runtime exception.
* <p>
* Every application intending to use the global tracer is responsible for registering it once
* during its initialization.
*
* @return Whether a tracer has been registered
* @param tracer Tracer to use as global tracer.
* @throws RuntimeException if there is already a current tracer registered
* @see #registerIfAbsent(Callable)
* @deprecated Please use 'registerIfAbsent' instead which does not attempt a double registration.
*/
public static synchronized boolean isRegistered() {
return !(GlobalTracer.tracer instanceof NoopTracer);
@Deprecated
public static void register(final Tracer tracer) {
if (!registerIfAbsent(provide(tracer))
&& !tracer.equals(GlobalTracer.tracer)
&& !(tracer instanceof GlobalTracer)) {
throw new IllegalStateException("There is already a current global Tracer registered.");
}
}

@Override
Expand Down Expand Up @@ -152,4 +182,19 @@ public Span activeSpan() {
public String toString() {
return GlobalTracer.class.getSimpleName() + '{' + tracer + '}';
}

private static Callable<Tracer> provide(final Tracer tracer) {
return new Callable<Tracer>() {
public Tracer call() {
return tracer;
}
};
}

private static <T> T requireNonNull(T value, String message) {
if (value == null) {
throw new NullPointerException(message);
}
return value;
}
}
97 changes: 82 additions & 15 deletions opentracing-util/src/test/java/io/opentracing/util/GlobalTracerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,22 @@
*/
package io.opentracing.util;

import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.noop.NoopSpanBuilder;
import io.opentracing.propagation.Format;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import static org.hamcrest.CoreMatchers.anyOf;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is;
Expand All @@ -26,21 +42,6 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.noop.NoopSpanBuilder;
import io.opentracing.propagation.Format;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

public class GlobalTracerTest {

@Before
Expand Down Expand Up @@ -90,6 +91,57 @@ public void testRegisterNull() {
GlobalTracer.register(null);
}

@Test(expected = NullPointerException.class)
public void testRegisterIfAbsentNullSupplier() {
GlobalTracer.registerIfAbsent(null);
}

@Test(expected = NullPointerException.class)
public void testRegisterIfAbsentNullTracer() {
GlobalTracer.registerIfAbsent(provide(null));
}

@Test
public void testRegisterIfAbsent_multipleTracers() {
assertThat(GlobalTracer.registerIfAbsent(provide(mock(Tracer.class))), is(true));
assertThat(GlobalTracer.registerIfAbsent(provide(mock(Tracer.class))), is(false));
assertThat(GlobalTracer.registerIfAbsent(provide(mock(Tracer.class))), is(false));
}

@Test
public void testRegisterIfAbsent_multiple_sameTracer() {
final Tracer mockTracer = mock(Tracer.class);
assertThat(GlobalTracer.registerIfAbsent(provide(mockTracer)), is(true));
assertThat(GlobalTracer.registerIfAbsent(provide(mockTracer)), is(false));
}

@Test
public void testRegisterIfAbsent_globalTracer() {
assertThat(GlobalTracer.registerIfAbsent(provide(GlobalTracer.get())), is(false));
}

@Test
public void testRegisterIfAbsent_runtimeException() {
RuntimeException thrownRuntimeException = new IllegalStateException("Expected runtime exception");
try {
GlobalTracer.registerIfAbsent(throwing(thrownRuntimeException));
fail("Runtime exception expected");
} catch (RuntimeException expected) {
assertThat(expected, is(sameInstance(thrownRuntimeException)));
}
}

@Test
public void testRegisterIfAbsent_checkedException() {
Exception thrownCheckedException = new Exception("Expected checked exception");
try {
GlobalTracer.registerIfAbsent(throwing(thrownCheckedException));
fail("Runtime exception expected");
} catch (RuntimeException expected) {
assertThat(expected.getCause(), is(sameInstance((Throwable) thrownCheckedException)));
}
}

@Test
public void testNoopTracerByDefault() {
Tracer.SpanBuilder spanBuilder = GlobalTracer.get().buildSpan("my-operation");
Expand Down Expand Up @@ -190,4 +242,19 @@ public void testIsRegistered() {
assertThat("Should be registered", GlobalTracer.isRegistered(), is(true));
}

private static Callable<Tracer> provide(final Tracer tracer) {
return new Callable<Tracer>() {
public Tracer call() {
return tracer;
}
};
}

private static Callable<Tracer> throwing(final Exception exception) {
return new Callable<Tracer>() {
public Tracer call() throws Exception {
throw exception;
}
};
}
}

0 comments on commit 67f080c

Please sign in to comment.