diff --git a/brave-tests/src/main/java/brave/test/propagation/CurrentTraceContextTest.java b/brave-tests/src/main/java/brave/test/propagation/CurrentTraceContextTest.java index c63bb92933..55a6208513 100644 --- a/brave-tests/src/main/java/brave/test/propagation/CurrentTraceContextTest.java +++ b/brave-tests/src/main/java/brave/test/propagation/CurrentTraceContextTest.java @@ -4,6 +4,7 @@ import brave.propagation.CurrentTraceContext; import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.TraceContext; +import brave.test.util.ClassLoaders; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -14,20 +15,23 @@ import java.util.function.Supplier; import org.junit.Test; +import static brave.test.util.ClassLoaders.assertRunIsUnloadableWithSupplier; +import static brave.test.util.ClassLoaders.newInstance; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; public abstract class CurrentTraceContextTest { - protected abstract CurrentTraceContext newCurrentTraceContext(); - protected final CurrentTraceContext currentTraceContext; protected final TraceContext context = TraceContext.newBuilder().traceIdHigh(-1L).traceId(1L).spanId(1L).build(); protected final TraceContext context2 = context.toBuilder().parentId(context.spanId()).spanId(-2L).build(); + protected abstract Class> currentSupplier(); + protected CurrentTraceContextTest() { - currentTraceContext = newCurrentTraceContext(); + currentTraceContext = newInstance(currentSupplier(), getClass().getClassLoader()).get(); } protected void verifyImplicitContext(@Nullable TraceContext context) { @@ -141,12 +145,12 @@ protected void is_inheritable(CurrentTraceContext inheritableCurrentTraceContext // submitting a job grows the pool, attaching the context to its thread try (Scope scope = inheritableCurrentTraceContext.newScope(context)) { - assertThat(service.submit(() -> inheritableCurrentTraceContext.get()).get()) + assertThat(service.submit(inheritableCurrentTraceContext::get).get()) .isEqualTo(context); } // same thread executes the next job and still has the same context (leaked and not cleaned up) - assertThat(service.submit(() -> inheritableCurrentTraceContext.get()).get()) + assertThat(service.submit(inheritableCurrentTraceContext::get).get()) .isEqualTo(context); service.shutdownNow(); @@ -165,7 +169,7 @@ protected void is_inheritable(CurrentTraceContext inheritableCurrentTraceContext throw (Exception) e.getCause(); } - assertThat(service.submit(() -> currentTraceContext.get()).get()) + assertThat(service.submit(currentTraceContext::get).get()) .isNull(); verifyImplicitContext(null); @@ -244,4 +248,45 @@ protected void is_inheritable(CurrentTraceContext inheritableCurrentTraceContext verifyImplicitContext(context0); } } + + @Test public void unloadable_unused() { + assertRunIsUnloadableWithSupplier(Unused.class, currentSupplier()); + } + + static class Unused extends ClassLoaders.ConsumerRunnable { + @Override public void accept(CurrentTraceContext currentTraceContext) { + } + } + + @Test public void unloadable_afterScopeClose() { + assertRunIsUnloadableWithSupplier(ClosedScope.class, currentSupplier()); + } + + static class ClosedScope extends ClassLoaders.ConsumerRunnable { + @Override public void accept(CurrentTraceContext current) { + try (Scope ws = current.newScope(TraceContext.newBuilder().traceId(1L).spanId(2L).build())) { + } + } + } + + /** + * TODO: While it is an instrumentation bug to not close a scope, we should be tolerant. For + * example, considering weak references or similar. + */ + @SuppressWarnings("CheckReturnValue") + @Test public void notUnloadable_whenScopeLeaked() { + try { + assertRunIsUnloadableWithSupplier(LeakedScope.class, currentSupplier()); + failBecauseExceptionWasNotThrown(AssertionError.class); + } catch (AssertionError e) { + // clear the leaked scope so other tests don't break + currentTraceContext.newScope(null); + } + } + + static class LeakedScope extends ClassLoaders.ConsumerRunnable { + @Override public void accept(CurrentTraceContext current) { + current.newScope(TraceContext.newBuilder().traceId(1L).spanId(2L).build()); + } + } } diff --git a/brave-tests/src/main/java/brave/test/propagation/PropagationTest.java b/brave-tests/src/main/java/brave/test/propagation/PropagationTest.java index e3e97b7143..1884f625ae 100644 --- a/brave-tests/src/main/java/brave/test/propagation/PropagationTest.java +++ b/brave-tests/src/main/java/brave/test/propagation/PropagationTest.java @@ -6,15 +6,19 @@ import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; import brave.propagation.TraceContextOrSamplingFlags; +import brave.test.util.ClassLoaders; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.Supplier; import org.junit.Test; +import static brave.test.util.ClassLoaders.assertRunIsUnloadableWithSupplier; +import static brave.test.util.ClassLoaders.newInstance; import static org.assertj.core.api.Assertions.assertThat; public abstract class PropagationTest { - protected abstract Propagation propagation(); + protected abstract Class>> propagationSupplier(); protected abstract void inject(Map map, @Nullable String traceId, @Nullable String parentId, @Nullable String spanId, @Nullable Boolean sampled, @@ -37,6 +41,12 @@ protected abstract void inject(Map map, @Nullable String traceId, .parentId(rootSpan.spanId()) .spanId(2).build(); + protected final Propagation propagation; + + protected PropagationTest() { + propagation = newInstance(propagationSupplier(), getClass().getClassLoader()).get(); + } + @Test public void verifyRoundTrip_rootSpan() throws Exception { inject(map, "0000000000000001", null, "0000000000000001", true, null); @@ -93,7 +103,8 @@ protected abstract void inject(Map map, @Nullable String traceId, } /** - * When the caller propagates IDs, but not a sampling decision, the current process should decide. + * When the caller propagates IDs, but not a sampling decision, the current process should + * decide. */ @Test public void verifyRoundTrip_externallyProvidedIds() { inject(map, "0000000000000001", null, "0000000000000001", null, null); @@ -102,14 +113,14 @@ protected abstract void inject(Map map, @Nullable String traceId, } void verifyRoundTrip(TraceContextOrSamplingFlags expected) { - TraceContextOrSamplingFlags extracted = propagation().extractor(mapEntry).extract(map); + TraceContextOrSamplingFlags extracted = propagation.extractor(mapEntry).extract(map); assertThat(extracted) .isEqualTo(expected); Map injected = new LinkedHashMap<>(); if (expected.context() != null) { - propagation().injector(mapEntry).inject(expected.context(), injected); + propagation.injector(mapEntry).inject(expected.context(), injected); } else { inject(injected, expected.samplingFlags()); } @@ -120,7 +131,7 @@ void verifyRoundTrip(TraceContextOrSamplingFlags expected) { protected static class MapEntry implements Propagation.Getter, K>, Propagation.Setter, K> { - public MapEntry(){ + public MapEntry() { } @Override public void put(Map carrier, K key, String value) { @@ -131,4 +142,31 @@ public MapEntry(){ return carrier.get(key); } } + + @Test public void unloadable_unused() { + assertRunIsUnloadableWithSupplier(Unused.class, propagationSupplier()); + } + + static class Unused extends ClassLoaders.ConsumerRunnable> { + @Override public void accept(Propagation propagation) { + } + } + + @Test public void unloadable_afterBasicUsage() { + assertRunIsUnloadableWithSupplier(BasicUsage.class, propagationSupplier()); + } + + static class BasicUsage extends ClassLoaders.ConsumerRunnable> { + @Override public void accept(Propagation propagation) { + TraceContext.Injector> injector = propagation.injector(Map::put); + TraceContext.Extractor> extractor = propagation.extractor(Map::get); + + TraceContext ctx = TraceContext.newBuilder().traceId(1L).spanId(2L).sampled(false).build(); + Map map = new LinkedHashMap<>(); + injector.inject(ctx, map); + + assertThat(extractor.extract(map).context()) + .isEqualToComparingFieldByField(ctx); + } + } } diff --git a/brave-tests/src/main/java/brave/test/util/ClassLoaders.java b/brave-tests/src/main/java/brave/test/util/ClassLoaders.java new file mode 100644 index 0000000000..a388b35af2 --- /dev/null +++ b/brave-tests/src/main/java/brave/test/util/ClassLoaders.java @@ -0,0 +1,145 @@ +package brave.test.util; + +import java.io.File; +import java.lang.ref.WeakReference; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.net.MalformedURLException; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.function.Consumer; +import java.util.function.Supplier; + +import static org.assertj.core.api.Assertions.assertThat; + +public final class ClassLoaders { + + /** Runs the assertion in a new classloader. Needed when you are creating parameterized tests */ + public static void assertRunIsUnloadableWithSupplier( + Class> assertion, + Class> supplier + ) { + String property = assertion.getName() + ".supplier"; // assumes assertion is run only once + System.setProperty(property, supplier.getName()); + try { + assertRunIsUnloadable(assertion, assertion.getClassLoader()); + } finally { + System.getProperties().remove(property); + } + } + + public static abstract class ConsumerRunnable implements Runnable, Consumer { + Class> subjectSupplier; + + protected ConsumerRunnable() { + try { + subjectSupplier = + (Class) Class.forName(System.getProperty(getClass().getName() + ".supplier")); + } catch (ClassNotFoundException e) { + throw new AssertionError(e); + } + } + + @Override public void run() { + accept(newInstance(subjectSupplier, getClass().getClassLoader()).get()); + } + } + + /** Validating instance creator that ensures the supplier type is static or top-level */ + public static T newInstance(Class type, ClassLoader loader) { + assertThat(type) + .withFailMessage(type + " should be a static member class") + .satisfies(c -> { + assertThat(c.isLocalClass()).isFalse(); + assertThat(Modifier.isPublic(c.getModifiers())).isFalse(); + }); + + try { + Class classToInstantiate = (Class) loader.loadClass(type.getName()); + Constructor ctor = classToInstantiate.getDeclaredConstructor(); + ctor.setAccessible(true); + + return ctor.newInstance(); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e); + } + } + + /** Runs the type in a new classloader that recreates brave classes */ + public static void assertRunIsUnloadable(Class runnable, ClassLoader parent) { + WeakReference loader; + try { + loader = invokeRunFromNewClassLoader(runnable, parent); + } catch (Exception e) { + throw new AssertionError(e); + } + + blockOnGC(); + + assertThat(loader.get()) + .withFailMessage(runnable + " includes state that couldn't be garbage collected") + .isNull(); + } + + static void blockOnGC() { + System.gc(); + try { + Thread.sleep(200L); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(e); + } + } + + static WeakReference invokeRunFromNewClassLoader( + Class runnable, ClassLoader parent) throws Exception { + + ClassLoader loader = ClassLoaders.reloadClassNamePrefix(parent, "brave"); + Runnable instance = newInstance(runnable, loader); + + // ensure the classes are indeed different + assertThat(instance.getClass()).isNotSameAs(runnable); + + Method run = instance.getClass().getMethod("run"); + run.setAccessible(true); + + run.invoke(instance); + + return new WeakReference<>(loader); + } + + /** + * Creates a new classloader that reloads types matching the given prefix. This is used to test + * behavior such a leaked type in a thread local. + * + *

This works by using a bridge loader over the normal system one. The bridge loader always + * loads new classes when they are prefixed by brave types. + * + *

This approach is the same as what's used in Android's {@code tests.util.ClassLoaderBuilder} + * See https://android.googlesource.com/platform/libcore/+/master/support/src/test/java/tests/util/ClassLoaderBuilder.java + */ + static ClassLoader reloadClassNamePrefix(ClassLoader parent, String prefix) { + ClassLoader bridge = new ClassLoader(parent) { + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (name.startsWith(prefix)) throw new ClassNotFoundException("reloading type: " + name); + return super.loadClass(name, resolve); + } + }; + try { + return new URLClassLoader(classpathToUrls(), bridge); + } catch (MalformedURLException e) { + throw new IllegalStateException("Couldn't read the current system classpath", e); + } + } + + static URL[] classpathToUrls() throws MalformedURLException { + String[] classpath = System.getProperty("java.class.path").split(File.pathSeparator, -1); + URL[] result = new URL[classpath.length]; + for (int i = 0; i < classpath.length; i++) { + result[i] = new File(classpath[i]).toURI().toURL(); + } + return result; + } +} diff --git a/brave-tests/src/test/java/brave/TracerClassLoaderTest.java b/brave-tests/src/test/java/brave/TracerClassLoaderTest.java new file mode 100644 index 0000000000..97e14cb244 --- /dev/null +++ b/brave-tests/src/test/java/brave/TracerClassLoaderTest.java @@ -0,0 +1,19 @@ +package brave; + +import org.junit.Test; +import zipkin2.Span; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; + +public class TracerClassLoaderTest { + @Test public void unloadable_withLoggingReporter() { + assertRunIsUnloadable(UsingLoggingReporter.class, getClass().getClassLoader()); + } + + static class UsingLoggingReporter implements Runnable { + @Override public void run() { + Tracing.LoggingReporter reporter = new Tracing.LoggingReporter(); + reporter.report(Span.newBuilder().traceId("a").id("b").build()); + } + } +} diff --git a/brave-tests/src/test/java/brave/TracingClassLoaderTest.java b/brave-tests/src/test/java/brave/TracingClassLoaderTest.java new file mode 100644 index 0000000000..ff1d5ed07c --- /dev/null +++ b/brave-tests/src/test/java/brave/TracingClassLoaderTest.java @@ -0,0 +1,32 @@ +package brave; + +import org.junit.Test; +import zipkin2.reporter.Reporter; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; + +public class TracingClassLoaderTest { + + @Test public void unloadable_afterClose() { + assertRunIsUnloadable(ClosesTracing.class, getClass().getClassLoader()); + } + + static class ClosesTracing implements Runnable { + @Override public void run() { + try (Tracing tracing = Tracing.newBuilder().spanReporter(Reporter.NOOP).build()) { + } + } + } + + @Test public void unloadable_afterBasicUsage() { + assertRunIsUnloadable(BasicUsage.class, getClass().getClassLoader()); + } + + static class BasicUsage implements Runnable { + @Override public void run() { + try (Tracing tracing = Tracing.newBuilder().spanReporter(Reporter.NOOP).build()) { + tracing.tracer().newTrace().start().finish(); + } + } + } +} diff --git a/brave-tests/src/test/java/brave/internal/PlatformClassLoaderTest.java b/brave-tests/src/test/java/brave/internal/PlatformClassLoaderTest.java new file mode 100644 index 0000000000..17e1eea37c --- /dev/null +++ b/brave-tests/src/test/java/brave/internal/PlatformClassLoaderTest.java @@ -0,0 +1,70 @@ +package brave.internal; + +import brave.test.util.ClassLoaders; +import org.junit.Test; + +import static java.net.InetSocketAddress.createUnresolved; +import static org.assertj.core.api.Assertions.assertThat; + +public class PlatformClassLoaderTest { + @Test public void unloadable_afterGet() { + assertRunIsUnloadable(GetPlatform.class); + } + + static class GetPlatform implements Runnable { + @Override public void run() { + Platform platform = Platform.get(); + assertThat(platform).isNotNull(); + } + } + + @Test public void unloadable_afterGetLinkLocalIp() { + assertRunIsUnloadable(GetPlatformLinkLocalIp.class); + } + + static class GetPlatformLinkLocalIp implements Runnable { + @Override public void run() { + Platform platform = Platform.get(); + platform.linkLocalIp(); + } + } + + @Test public void unloadable_afterGetNextTraceIdHigh() { + assertRunIsUnloadable(GetPlatformNextTraceIdHigh.class); + } + + static class GetPlatformNextTraceIdHigh implements Runnable { + @Override public void run() { + Platform platform = Platform.get(); + assertThat(platform.nextTraceIdHigh()).isNotZero(); + } + } + + @Test public void unloadable_afterGetHostString() { + assertRunIsUnloadable(GetPlatformHostString.class); + } + + static class GetPlatformHostString implements Runnable { + @Override public void run() { + Platform platform = Platform.get(); + assertThat(platform.getHostString(createUnresolved("1.2.3.4", 0))) + .isNotNull(); + } + } + + @Test public void unloadable_afterGetClock() { + assertRunIsUnloadable(GetPlatformClock.class); + } + + static class GetPlatformClock implements Runnable { + @Override public void run() { + Platform platform = Platform.get(); + assertThat(platform.clock().currentTimeMicroseconds()) + .isPositive(); + } + } + + void assertRunIsUnloadable(Class runnable) { + ClassLoaders.assertRunIsUnloadable(runnable, getClass().getClassLoader()); + } +} diff --git a/brave-tests/src/test/java/brave/internal/recorder/PendingSpansClassLoaderTest.java b/brave-tests/src/test/java/brave/internal/recorder/PendingSpansClassLoaderTest.java new file mode 100644 index 0000000000..c76223a2e2 --- /dev/null +++ b/brave-tests/src/test/java/brave/internal/recorder/PendingSpansClassLoaderTest.java @@ -0,0 +1,31 @@ +package brave.internal.recorder; + +import brave.internal.Platform; +import brave.propagation.TraceContext; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.Test; +import zipkin2.Endpoint; +import zipkin2.reporter.Reporter; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; + +public class PendingSpansClassLoaderTest { + + @Test public void unloadable_afterCreateAndRemove() { + assertRunIsUnloadable(CreateAndRemove.class, getClass().getClassLoader()); + } + + static class CreateAndRemove implements Runnable { + @Override public void run() { + PendingSpans pendingSpans = new PendingSpans( + Endpoint.newBuilder().serviceName("unknown").build(), + Platform.get().clock(), + Reporter.NOOP, + new AtomicBoolean()); + + TraceContext context = TraceContext.newBuilder().traceId(1).spanId(2).build(); + pendingSpans.getOrCreate(context, true); + pendingSpans.remove(context); + } + } +} diff --git a/brave-tests/src/test/java/brave/internal/recorder/SpanReporterClassLoaderTest.java b/brave-tests/src/test/java/brave/internal/recorder/SpanReporterClassLoaderTest.java new file mode 100644 index 0000000000..93b63ba39f --- /dev/null +++ b/brave-tests/src/test/java/brave/internal/recorder/SpanReporterClassLoaderTest.java @@ -0,0 +1,52 @@ +package brave.internal.recorder; + +import brave.propagation.TraceContext; +import java.util.concurrent.atomic.AtomicBoolean; +import org.junit.Test; +import zipkin2.Endpoint; +import zipkin2.reporter.Reporter; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; + +public class SpanReporterClassLoaderTest { + + @Test public void unloadable_afterBasicUsage() { + assertRunIsUnloadable(BasicUsage.class, getClass().getClassLoader()); + } + + static class BasicUsage implements Runnable { + @Override public void run() { + SpanReporter reporter = new SpanReporter( + Endpoint.newBuilder().serviceName("unknown").build(), + Reporter.NOOP, + new AtomicBoolean()); + + TraceContext context = TraceContext.newBuilder().traceId(1).spanId(2).build(); + MutableSpan span = new MutableSpan(); + span.name("get /users/{userId}"); + span.startTimestamp(1L); + span.tag("http.method", "GET"); + span.annotate(2L, "cache.miss"); + span.finishTimestamp(3L); + + reporter.report(context, span); + } + } + + @Test public void unloadable_afterErrorReporting() { + assertRunIsUnloadable(ErrorReporting.class, getClass().getClassLoader()); + } + + static class ErrorReporting implements Runnable { + @Override public void run() { + SpanReporter reporter = new SpanReporter( + Endpoint.newBuilder().serviceName("unknown").build(), + s -> { + throw new RuntimeException(); + }, + new AtomicBoolean()); + + reporter.report(TraceContext.newBuilder().traceId(1).spanId(2).build(), new MutableSpan()); + } + } +} diff --git a/brave-tests/src/test/java/brave/propagation/B3PropagationTest.java b/brave-tests/src/test/java/brave/propagation/B3PropagationTest.java index 2b1dbbab6e..32fa069e64 100644 --- a/brave-tests/src/test/java/brave/propagation/B3PropagationTest.java +++ b/brave-tests/src/test/java/brave/propagation/B3PropagationTest.java @@ -4,13 +4,21 @@ import brave.internal.Nullable; import brave.test.propagation.PropagationTest; import java.util.Map; +import java.util.function.Supplier; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; public class B3PropagationTest extends PropagationTest { - @Override protected Propagation propagation() { - return Propagation.B3_STRING; + + @Override protected Class>> propagationSupplier() { + return PropagationSupplier.class; + } + + static class PropagationSupplier implements Supplier> { + @Override public Propagation get() { + return Propagation.B3_STRING; + } } @Override protected void inject(Map map, @Nullable String traceId, @@ -32,66 +40,66 @@ public class B3PropagationTest extends PropagationTest { } @Test public void extractTraceContext_sampledFalse() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("X-B3-Sampled", "false"); - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.NOT_SAMPLED); } @Test public void extractTraceContext_sampledFalseUpperCase() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("X-B3-Sampled", "FALSE"); - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.NOT_SAMPLED); } @Test public void extractTraceContext_malformed() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("X-B3-TraceId", "463ac35c9f6413ad48485a3953bb6124"); // ok map.put("X-B3-SpanId", "48485a3953bb6124"); // ok map.put("X-B3-ParentSpanId", "-"); // not ok - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.EMPTY); } @Test public void extractTraceContext_malformed_sampled() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("X-B3-TraceId", "-"); // not ok map.put("X-B3-Sampled", "1"); // ok - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.EMPTY); } @Test public void extractTraceContext_debug_with_ids() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("X-B3-TraceId", "463ac35c9f6413ad48485a3953bb6124"); // ok map.put("X-B3-SpanId", "48485a3953bb6124"); // ok map.put("X-B3-Flags", "1"); // accidentally missing sampled flag - TraceContext result = propagation().extractor(mapEntry).extract(map).context(); + TraceContext result = propagation.extractor(mapEntry).extract(map).context(); assertThat(result.sampled()) .isTrue(); } @Test public void extractTraceContext_singleHeaderFormat() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("b3", "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7"); - TraceContext result = propagation().extractor(mapEntry).extract(map).context(); + TraceContext result = propagation.extractor(mapEntry).extract(map).context(); assertThat(result.traceIdString()) .isEqualTo("4bf92f3577b34da6a3ce929d0e0e4736"); diff --git a/brave-tests/src/test/java/brave/propagation/B3SinglePropagationTest.java b/brave-tests/src/test/java/brave/propagation/B3SinglePropagationTest.java index 1c69075360..4cfece1ef5 100644 --- a/brave-tests/src/test/java/brave/propagation/B3SinglePropagationTest.java +++ b/brave-tests/src/test/java/brave/propagation/B3SinglePropagationTest.java @@ -3,13 +3,20 @@ import brave.internal.Nullable; import brave.test.propagation.PropagationTest; import java.util.Map; +import java.util.function.Supplier; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; public class B3SinglePropagationTest extends PropagationTest { - @Override protected Propagation propagation() { - return Propagation.B3_SINGLE_STRING; + @Override protected Class>> propagationSupplier() { + return PropagationSupplier.class; + } + + static class PropagationSupplier implements Supplier> { + @Override public Propagation get() { + return Propagation.B3_SINGLE_STRING; + } } @Override protected void inject(Map map, @Nullable String traceId, @@ -40,41 +47,41 @@ static char sampledChar(@Nullable Boolean sampled, @Nullable Boolean debug) { } @Test public void extractTraceContext_sampledFalse() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("b3", "0"); - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.NOT_SAMPLED); } @Test public void extractTraceContext_malformed() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("b3", "not-a-tumor"); - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.EMPTY); } @Test public void extractTraceContext_malformed_uuid() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("b3", "b970dafd-0d95-40aa-95d8-1d8725aebe40"); - SamplingFlags result = propagation().extractor(mapEntry).extract(map).samplingFlags(); + SamplingFlags result = propagation.extractor(mapEntry).extract(map).samplingFlags(); assertThat(result) .isEqualTo(SamplingFlags.EMPTY); } @Test public void extractTraceContext_debug_with_ids() { - MapEntry mapEntry = new MapEntry(); + MapEntry mapEntry = new MapEntry<>(); map.put("b3", "4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-d"); - TraceContext result = propagation().extractor(mapEntry).extract(map).context(); + TraceContext result = propagation.extractor(mapEntry).extract(map).context(); assertThat(result.debug()) .isTrue(); diff --git a/brave-tests/src/test/java/brave/propagation/DefaultCurrentTraceContextTest.java b/brave-tests/src/test/java/brave/propagation/DefaultCurrentTraceContextTest.java new file mode 100644 index 0000000000..5920cc69b9 --- /dev/null +++ b/brave-tests/src/test/java/brave/propagation/DefaultCurrentTraceContextTest.java @@ -0,0 +1,28 @@ +package brave.propagation; + +import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; +import org.junit.Before; +import org.junit.Test; + +public class DefaultCurrentTraceContextTest extends CurrentTraceContextTest { + + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return CurrentTraceContext.Default.create(); + } + } + + @Test public void is_inheritable() throws Exception { + super.is_inheritable(CurrentTraceContext.Default.inheritable()); + } + + @Before public void ensureNoOtherTestsTaint() { + CurrentTraceContext.Default.INHERITABLE.set(null); + CurrentTraceContext.Default.DEFAULT.set(null); + } +} diff --git a/brave-tests/src/test/java/brave/propagation/StrictCurrentTraceContextTest.java b/brave-tests/src/test/java/brave/propagation/StrictCurrentTraceContextTest.java index 256ae5138e..150cbca11b 100644 --- a/brave-tests/src/test/java/brave/propagation/StrictCurrentTraceContextTest.java +++ b/brave-tests/src/test/java/brave/propagation/StrictCurrentTraceContextTest.java @@ -1,14 +1,21 @@ package brave.propagation; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; public class StrictCurrentTraceContextTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return new StrictCurrentTraceContext(); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return new StrictCurrentTraceContext(); + } } @Test public void scope_enforcesCloseOnSameThread() throws InterruptedException { diff --git a/brave-tests/src/test/java/brave/propagation/ThreadLocalCurrentTraceContextTest.java b/brave-tests/src/test/java/brave/propagation/ThreadLocalCurrentTraceContextTest.java index f8cd821167..23eaa83148 100644 --- a/brave-tests/src/test/java/brave/propagation/ThreadLocalCurrentTraceContextTest.java +++ b/brave-tests/src/test/java/brave/propagation/ThreadLocalCurrentTraceContextTest.java @@ -1,21 +1,17 @@ package brave.propagation; import brave.test.propagation.CurrentTraceContextTest; -import org.junit.Before; -import org.junit.Test; +import java.util.function.Supplier; public class ThreadLocalCurrentTraceContextTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return CurrentTraceContext.Default.create(); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; } - @Test public void is_inheritable() throws Exception { - super.is_inheritable(CurrentTraceContext.Default.inheritable()); - } - - @Before public void ensureNoOtherTestsTaint() { - CurrentTraceContext.Default.INHERITABLE.set(null); - CurrentTraceContext.Default.DEFAULT.set(null); + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return ThreadLocalCurrentTraceContext.newBuilder().build(); + } } } diff --git a/brave-tests/src/test/java/brave/propagation/ThreadLocalSpanClassLoaderTest.java b/brave-tests/src/test/java/brave/propagation/ThreadLocalSpanClassLoaderTest.java new file mode 100644 index 0000000000..08c2cc43fe --- /dev/null +++ b/brave-tests/src/test/java/brave/propagation/ThreadLocalSpanClassLoaderTest.java @@ -0,0 +1,66 @@ +package brave.propagation; + +import brave.Tracing; +import org.junit.Test; +import zipkin2.reporter.Reporter; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; + +public class ThreadLocalSpanClassLoaderTest { + + @Test public void noop_unloadable() { + assertRunIsUnloadable(CurrentTracerUnassigned.class, getClass().getClassLoader()); + } + + static class CurrentTracerUnassigned implements Runnable { + @Override public void run() { + ThreadLocalSpan.CURRENT_TRACER.next(); + } + } + + @Test public void currentTracer_basicUsage_unloadable() { + assertRunIsUnloadable(ExplicitTracerBasicUsage.class, getClass().getClassLoader()); + } + + static class ExplicitTracerBasicUsage implements Runnable { + @Override public void run() { + try (Tracing tracing = Tracing.newBuilder().spanReporter(Reporter.NOOP).build()) { + ThreadLocalSpan tlSpan = ThreadLocalSpan.create(tracing.tracer()); + + tlSpan.next(); + tlSpan.remove().finish(); + } + } + } + + @Test public void explicitTracer_basicUsage_unloadable() { + assertRunIsUnloadable(CurrentTracerBasicUsage.class, getClass().getClassLoader()); + } + + static class CurrentTracerBasicUsage implements Runnable { + @Override public void run() { + try (Tracing tracing = Tracing.newBuilder().spanReporter(Reporter.NOOP).build()) { + ThreadLocalSpan tlSpan = ThreadLocalSpan.CURRENT_TRACER; + + tlSpan.next(); + tlSpan.remove().finish(); + } + } + } + + /** + * TODO: While it is an instrumentation bug to not complete a thread-local span, we should be + * tolerant, for example considering weak references or similar. + */ + @Test(expected = AssertionError.class) public void unfinishedSpan_preventsUnloading() { + assertRunIsUnloadable(CurrentTracerDoesntFinishSpan.class, getClass().getClassLoader()); + } + + static class CurrentTracerDoesntFinishSpan implements Runnable { + @Override public void run() { + try (Tracing tracing = Tracing.newBuilder().spanReporter(Reporter.NOOP).build()) { + ThreadLocalSpan.CURRENT_TRACER.next(); + } + } + } +} diff --git a/brave-tests/src/test/java/brave/propagation/TraceContextClassLoaderTest.java b/brave-tests/src/test/java/brave/propagation/TraceContextClassLoaderTest.java new file mode 100644 index 0000000000..b213a42852 --- /dev/null +++ b/brave-tests/src/test/java/brave/propagation/TraceContextClassLoaderTest.java @@ -0,0 +1,18 @@ +package brave.propagation; + +import org.junit.Test; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; + +public class TraceContextClassLoaderTest { + + @Test public void unloadable_afterBasicUsage() { + assertRunIsUnloadable(BasicUsage.class, getClass().getClassLoader()); + } + + static class BasicUsage implements Runnable { + @Override public void run() { + TraceContext.newBuilder().traceId(1).spanId(2).build(); + } + } +} diff --git a/brave-tests/src/test/java/brave/test/util/ClassLoadersTest.java b/brave-tests/src/test/java/brave/test/util/ClassLoadersTest.java new file mode 100644 index 0000000000..6972a12e4c --- /dev/null +++ b/brave-tests/src/test/java/brave/test/util/ClassLoadersTest.java @@ -0,0 +1,70 @@ +package brave.test.util; + +import java.lang.ref.WeakReference; +import org.junit.Test; + +import static brave.test.util.ClassLoaders.assertRunIsUnloadable; +import static org.assertj.core.api.Assertions.assertThat; + +public class ClassLoadersTest { + static class Foo { + } + + @Test public void createdNonDelegating_cantSeeCurrentClasspath() throws Exception { + Foo foo = new Foo(); // load the class + + ClassLoader loader = + ClassLoaders.reloadClassNamePrefix(getClass().getClassLoader(), getClass().getName()); + assertThat(loader.loadClass(Foo.class.getName())) + .isNotSameAs(foo.getClass()); + } + + static class PresentThreadLocalWithSystemType implements Runnable { + ThreadLocal local = new ThreadLocal<>(); + + @Override public void run() { + local.set("foo"); + } + } + + @Test public void assertRunIsUnloadable_threadLocalWithSystemClassIsUnloadable() { + assertRunIsUnloadable(PresentThreadLocalWithSystemType.class, getClass().getClassLoader()); + } + + static class AbsentThreadLocalWithApplicationType implements Runnable { + ThreadLocal local = new ThreadLocal<>(); + + @Override public void run() { + } + } + + @Test public void assertRunIsUnloadable_absentThreadLocalWithOurClassIsUnloadable() { + assertRunIsUnloadable(AbsentThreadLocalWithApplicationType.class, getClass().getClassLoader()); + } + + static class PresentThreadLocalWithApplicationType implements Runnable { + ThreadLocal local = new ThreadLocal<>(); + + @Override public void run() { + local.set(new ClassLoadersTest()); + } + } + + @Test(expected = AssertionError.class) + public void assertRunIsUnloadable_threadLocalWithOurClassIsntUnloadable() { + assertRunIsUnloadable(PresentThreadLocalWithApplicationType.class, getClass().getClassLoader()); + } + + static class PresentThreadLocalWithWeakRefToApplicationType implements Runnable { + ThreadLocal> local = new ThreadLocal<>(); + + @Override public void run() { + local.set(new WeakReference<>(new ClassLoadersTest())); + } + } + + @Test public void assertRunIsUnloadable_threadLocalWithWeakRefToOurClassIsUnloadable() { + assertRunIsUnloadable(PresentThreadLocalWithWeakRefToApplicationType.class, + getClass().getClassLoader()); + } +} diff --git a/brave/src/main/java/brave/Tracer.java b/brave/src/main/java/brave/Tracer.java index 7c070f13db..57dea8da6f 100644 --- a/brave/src/main/java/brave/Tracer.java +++ b/brave/src/main/java/brave/Tracer.java @@ -9,6 +9,7 @@ import brave.internal.recorder.PendingSpans; import brave.internal.recorder.SpanReporter; import brave.propagation.CurrentTraceContext; +import brave.propagation.CurrentTraceContext.Scope; import brave.propagation.Propagation; import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; @@ -441,7 +442,7 @@ public ScopedSpan startScopedSpanWithParent(String name, @Nullable TraceContext if (parent == null) parent = currentTraceContext.get(); TraceContext context = parent != null ? nextContext(parent) : newRootContext(); - CurrentTraceContext.Scope scope = currentTraceContext.newScope(context); + Scope scope = currentTraceContext.newScope(context); if (isNoop(context)) return new NoopScopedSpan(context, scope); PendingSpan pendingSpan = pendingSpans.getOrCreate(context, true); @@ -454,10 +455,10 @@ public ScopedSpan startScopedSpanWithParent(String name, @Nullable TraceContext /** A span remains in the scope it was bound to until close is called. */ public static final class SpanInScope implements Closeable { - final CurrentTraceContext.Scope scope; + final Scope scope; // This type hides the SPI type and allows us to double-check the SPI didn't return null. - SpanInScope(CurrentTraceContext.Scope scope) { + SpanInScope(Scope scope) { if (scope == null) throw new NullPointerException("scope == null"); this.scope = scope; } diff --git a/brave/src/main/java/brave/Tracing.java b/brave/src/main/java/brave/Tracing.java index dc7bcc57ed..1df0049654 100644 --- a/brave/src/main/java/brave/Tracing.java +++ b/brave/src/main/java/brave/Tracing.java @@ -13,6 +13,8 @@ import java.io.Closeable; import java.util.Locale; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Level; +import java.util.logging.Logger; import zipkin2.reporter.AsyncReporter; import zipkin2.reporter.Reporter; import zipkin2.reporter.Sender; @@ -302,7 +304,7 @@ public Builder errorParser(ErrorParser errorParser) { public Tracing build() { if (clock == null) clock = Platform.get().clock(); if (localIp == null) localIp = Platform.get().linkLocalIp(); - if (reporter == null) reporter = Platform.get().reporter(); + if (reporter == null) reporter = new LoggingReporter(); return new Default(this); } @@ -310,6 +312,20 @@ public Tracing build() { } } + static final class LoggingReporter implements Reporter { + final Logger logger = Logger.getLogger(Tracer.class.getName()); + + @Override public void report(zipkin2.Span span) { + if (span == null) throw new NullPointerException("span == null"); + if (!logger.isLoggable(Level.INFO)) return; + logger.info(span.toString()); + } + + @Override public String toString() { + return "LoggingReporter{name=" + logger.getName() + "}"; + } + } + static final class Default extends Tracing { final Tracer tracer; final Propagation.Factory propagationFactory; diff --git a/brave/src/main/java/brave/internal/Platform.java b/brave/src/main/java/brave/internal/Platform.java index 041f90e33a..c20217d42b 100644 --- a/brave/src/main/java/brave/internal/Platform.java +++ b/brave/src/main/java/brave/internal/Platform.java @@ -1,8 +1,6 @@ package brave.internal; import brave.Clock; -import brave.Tracer; -import brave.Tracing; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.NetworkInterface; @@ -10,41 +8,23 @@ import java.util.Enumeration; import java.util.Random; import java.util.logging.Level; +import java.util.logging.LogRecord; import java.util.logging.Logger; import org.jvnet.animal_sniffer.IgnoreJRERequirement; -import zipkin2.Span; -import zipkin2.reporter.Reporter; /** - * Access to platform-specific features and implements a default logging spanReporter. + * Access to platform-specific features. + * + *

Note: Logging is centralized here to avoid classloader problems. * *

Originally designed by OkHttp team, derived from {@code okhttp3.internal.platform.Platform} */ public abstract class Platform { - static final Logger logger = Logger.getLogger(Tracer.class.getName()); - private static final Platform PLATFORM = findPlatform(); + private static final Logger LOG = Logger.getLogger(brave.Tracer.class.getName()); volatile String linkLocalIp; - public Reporter reporter() { - return LoggingReporter.INSTANCE; - } - - enum LoggingReporter implements Reporter { - INSTANCE; - - @Override public void report(Span span) { - if (!logger.isLoggable(Level.INFO)) return; - if (span == null) throw new NullPointerException("span == null"); - logger.info(span.toString()); - } - - @Override public String toString() { - return "LoggingReporter{name=" + logger.getName() + "}"; - } - } - /** Guards {@link InetSocketAddress#getHostString()}, as it isn't available until Java 7 */ @Nullable public abstract String getHostString(InetSocketAddress socket); @@ -73,9 +53,7 @@ String produceLinkLocalIp() { } } catch (Exception e) { // don't crash the caller if there was a problem reading nics. - if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, "error reading nics", e); - } + log("error reading nics", e); } return null; } @@ -84,6 +62,22 @@ public static Platform get() { return PLATFORM; } + /** Like {@link Logger#log(Level, String) */ + public void log(String msg, @Nullable Throwable thrown) { + if (!LOG.isLoggable(Level.FINE)) return; // fine level to not fill logs + LOG.log(Level.FINE, msg, thrown); + } + + /** Like {@link Logger#log(Level, String, Object)}, except with a throwable arg */ + public void log(String msg, Object param1, @Nullable Throwable thrown) { + if (!LOG.isLoggable(Level.FINE)) return; // fine level to not fill logs + LogRecord lr = new LogRecord(Level.FINE, msg); + Object params[] = {param1}; + lr.setParameters(params); + if (thrown != null) lr.setThrown(thrown); + LOG.log(lr); + } + /** Attempt to match the host runtime to a capable Platform implementation. */ static Platform findPlatform() { Platform jre9 = Jre9.buildIfSupported(); @@ -108,7 +102,7 @@ static Platform findPlatform() { public abstract long randomLong(); /** - * Returns the high 8-bytes for use in {@link Tracing.Builder#traceId128Bit 128-bit trace IDs}. + * Returns the high 8-bytes for {@link brave.Tracing.Builder#traceId128Bit 128-bit trace IDs}. * *

The upper 4-bytes are epoch seconds and the lower 4-bytes are random. This makes it * convertible to Amazon diff --git a/brave/src/main/java/brave/internal/recorder/MutableSpanConverter.java b/brave/src/main/java/brave/internal/recorder/MutableSpanConverter.java index 23a4ccf9b4..6f250c086b 100644 --- a/brave/src/main/java/brave/internal/recorder/MutableSpanConverter.java +++ b/brave/src/main/java/brave/internal/recorder/MutableSpanConverter.java @@ -1,23 +1,14 @@ package brave.internal.recorder; +import brave.internal.recorder.MutableSpan.AnnotationConsumer; +import brave.internal.recorder.MutableSpan.TagConsumer; import zipkin2.Span; // internal until we figure out how the api should sit. -public final class MutableSpanConverter { - static final MutableSpan.TagConsumer TAG_CONSUMER = new MutableSpan.TagConsumer() { - @Override public void accept(Span.Builder target, String key, String value) { - target.putTag(key, value); - } - }; - - static final MutableSpan.AnnotationConsumer ANNOTATION_CONSUMER = - new MutableSpan.AnnotationConsumer() { - @Override public void accept(Span.Builder target, long timestamp, String value) { - target.addAnnotation(timestamp, value); - } - }; +final class MutableSpanConverter + implements TagConsumer, AnnotationConsumer { - public static void convert(MutableSpan span, Span.Builder result) { + void convert(MutableSpan span, Span.Builder result) { result.name(span.name()); long start = span.startTimestamp(), finish = span.finishTimestamp(); @@ -38,8 +29,16 @@ public static void convert(MutableSpan span, Span.Builder result) { .port(span.remotePort()) .build()); } - span.forEachTag(TAG_CONSUMER, result); - span.forEachAnnotation(ANNOTATION_CONSUMER, result); + span.forEachTag(this, result); + span.forEachAnnotation(this, result); if (span.shared()) result.shared(true); } + + @Override public void accept(Span.Builder target, String key, String value) { + target.putTag(key, value); + } + + @Override public void accept(Span.Builder target, long timestamp, String value) { + target.addAnnotation(timestamp, value); + } } diff --git a/brave/src/main/java/brave/internal/recorder/PendingSpans.java b/brave/src/main/java/brave/internal/recorder/PendingSpans.java index d0695033c9..5adb01ef32 100644 --- a/brave/src/main/java/brave/internal/recorder/PendingSpans.java +++ b/brave/src/main/java/brave/internal/recorder/PendingSpans.java @@ -31,6 +31,7 @@ public final class PendingSpans extends ReferenceQueue { // Eventhough we only put by RealKey, we allow get and remove by LookupKey final ConcurrentMap delegate = new ConcurrentHashMap<>(64); // Used when flushing spans + final MutableSpanConverter converter = new MutableSpanConverter(); final Endpoint localEndpoint; final Clock clock; final Reporter reporter; @@ -127,7 +128,7 @@ void reportOrphanedSpans() { .localEndpoint(localEndpoint) .addAnnotation(flushTime, "brave.flush"); - MutableSpanConverter.convert(value.state, builderWithContextData); + converter.convert(value.state, builderWithContextData); reporter.report(builderWithContextData.build()); } } @@ -230,10 +231,11 @@ static int generateHashCode(long traceIdHigh, long traceId, long spanId, boolean public List snapshot() { List result = new ArrayList<>(); zipkin2.Span.Builder spanBuilder = zipkin2.Span.newBuilder(); + MutableSpanConverter converter = new MutableSpanConverter(); for (Map.Entry entry : delegate.entrySet()) { PendingSpans.RealKey contextKey = (PendingSpans.RealKey) entry.getKey(); spanBuilder.clear().traceId(contextKey.traceIdHigh, contextKey.traceId).id(contextKey.spanId); - MutableSpanConverter.convert(entry.getValue().state, spanBuilder); + converter.convert(entry.getValue().state, spanBuilder); result.add(spanBuilder.build()); spanBuilder.clear(); } diff --git a/brave/src/main/java/brave/internal/recorder/SpanReporter.java b/brave/src/main/java/brave/internal/recorder/SpanReporter.java index 7104a67696..f03637f251 100644 --- a/brave/src/main/java/brave/internal/recorder/SpanReporter.java +++ b/brave/src/main/java/brave/internal/recorder/SpanReporter.java @@ -1,18 +1,16 @@ package brave.internal.recorder; +import brave.internal.Platform; import brave.propagation.TraceContext; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Level; -import java.util.logging.Logger; import zipkin2.Endpoint; import zipkin2.Span; import zipkin2.reporter.Reporter; public final class SpanReporter implements Reporter { - static final Logger logger = Logger.getLogger(SpanReporter.class.getName()); - final Endpoint localEndpoint; final Reporter delegate; + final MutableSpanConverter converter = new MutableSpanConverter(); final AtomicBoolean noop; public SpanReporter(Endpoint localEndpoint, Reporter delegate, AtomicBoolean noop) { @@ -29,7 +27,7 @@ public void report(TraceContext context, MutableSpan span) { .debug(context.debug()) .localEndpoint(localEndpoint); - MutableSpanConverter.convert(span, builderWithContextData); + converter.convert(span, builderWithContextData); report(builderWithContextData.build()); } @@ -38,9 +36,7 @@ public void report(TraceContext context, MutableSpan span) { try { delegate.report(span); } catch (RuntimeException e) { - if (logger.isLoggable(Level.FINE)) { // fine level to not fill logs - logger.log(Level.FINE, "error reporting " + span, e); - } + Platform.get().log("error reporting {0}", span, e); } } diff --git a/brave/src/main/java/brave/propagation/B3SingleFormat.java b/brave/src/main/java/brave/propagation/B3SingleFormat.java index 1e3a05cfe9..9deed06b03 100644 --- a/brave/src/main/java/brave/propagation/B3SingleFormat.java +++ b/brave/src/main/java/brave/propagation/B3SingleFormat.java @@ -2,15 +2,14 @@ import brave.internal.HexCodec; import brave.internal.Nullable; +import brave.internal.Platform; import java.nio.ByteBuffer; import java.util.Collections; -import java.util.logging.Logger; import static brave.internal.HexCodec.writeHexLong; import static brave.internal.TraceContexts.FLAG_DEBUG; import static brave.internal.TraceContexts.FLAG_SAMPLED; import static brave.internal.TraceContexts.FLAG_SAMPLED_SET; -import static java.util.logging.Level.FINE; /** * This format corresponds to the propagation key "b3" (or "B3"), which delimits fields in the @@ -43,7 +42,6 @@ *

See B3 Propagation */ public final class B3SingleFormat { - static final Logger logger = Logger.getLogger(B3SingleFormat.class.getName()); static final int FORMAT_MAX_LENGTH = 32 + 1 + 16 + 2 + 16; // traceid128-spanid-1-parentid /** @@ -136,7 +134,7 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3) { public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, int beginIndex, int endIndex) { if (beginIndex == endIndex) { - logger.log(FINE, "Invalid input: empty"); + Platform.get().log("Invalid input: empty", null); return null; } @@ -147,10 +145,10 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, i // At this point we minimally expect a traceId-spanId pair if (endIndex < 16 + 1 + 16 /* traceid64-spanid */) { - logger.fine("Invalid input: truncated"); + Platform.get().log("Invalid input: truncated", null); return null; } else if (endIndex > FORMAT_MAX_LENGTH) { - logger.fine("Invalid input: too long"); + Platform.get().log("Invalid input: too long", null); return null; } @@ -167,13 +165,13 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, i if (!checkHyphen(b3, pos++)) return null; if (traceIdHigh == 0L && traceId == 0L) { - logger.fine("Invalid input: expected a 16 or 32 lower hex trace ID at offset 0"); + Platform.get().log("Invalid input: expected a 16 or 32 lower hex trace ID at offset 0", null); return null; } long spanId = tryParse16HexCharacters(b3, pos, endIndex); if (spanId == 0L) { - logger.log(FINE, "Invalid input: expected a 16 lower hex span ID at offset {0}", pos); + Platform.get().log("Invalid input: expected a 16 lower hex span ID at offset {0}", pos, null); return null; } pos += 16; // spanid @@ -186,7 +184,7 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, i // If it is absent, but a parent ID is (which is strange), we'll have at least 17 characters. // Therefore, if we have less than two characters, the input is truncated. if (endIndex == pos + 1) { - logger.fine("Invalid input: truncated"); + Platform.get().log("Invalid input: truncated", null); return null; } if (!checkHyphen(b3, pos++)) return null; @@ -202,14 +200,15 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, i if (endIndex > pos) { // If we are at this point, we should have a parent ID, encoded as "-[0-9a-f]{16}" if (endIndex != pos + 17) { - logger.fine("Invalid input: truncated"); + Platform.get().log("Invalid input: truncated", null); return null; } if (!checkHyphen(b3, pos++)) return null; parentId = tryParse16HexCharacters(b3, pos, endIndex); if (parentId == 0L) { - logger.log(FINE, "Invalid input: expected a 16 lower hex parent ID at offset {0}", pos); + Platform.get() + .log("Invalid input: expected a 16 lower hex parent ID at offset {0}", pos, null); return null; } } @@ -233,7 +232,7 @@ static TraceContextOrSamplingFlags tryParseSamplingFlags(CharSequence b3, int po static boolean checkHyphen(CharSequence b3, int pos) { if (b3.charAt(pos) == '-') return true; - logger.log(FINE, "Invalid input: expected a hyphen(-) delimiter offset {0}", pos); + Platform.get().log("Invalid input: expected a hyphen(-) delimiter offset {0}", pos, null); return false; } @@ -264,7 +263,7 @@ static int parseFlags(CharSequence b3, int pos) { } static void logInvalidSampled(int pos) { - logger.log(FINE, "Invalid input: expected 0, 1 or d for sampled at offset {0}", pos); + Platform.get().log("Invalid input: expected 0, 1 or d for sampled at offset {0}", pos, null); } static byte[] asciiToNewByteArray(char[] buffer, int length) { diff --git a/brave/src/main/java/brave/propagation/ThreadLocalSpan.java b/brave/src/main/java/brave/propagation/ThreadLocalSpan.java index 0101a5abf0..5cb04f6dd8 100644 --- a/brave/src/main/java/brave/propagation/ThreadLocalSpan.java +++ b/brave/src/main/java/brave/propagation/ThreadLocalSpan.java @@ -6,7 +6,6 @@ import brave.Tracing; import brave.internal.Nullable; import java.util.ArrayDeque; -import java.util.Deque; /** * This type allows you to place a span in scope in one method and access it in another without @@ -80,37 +79,21 @@ public class ThreadLocalSpan { * null. Use this when you have no other means to get a reference to the tracer. For example, JDBC * connections, as they often initialize prior to the tracing component. */ - public static final ThreadLocalSpan CURRENT_TRACER = new ThreadLocalSpan(null) { - @Override Tracer tracer() { - return Tracing.currentTracer(); - } - }; + public static final ThreadLocalSpan CURRENT_TRACER = new ThreadLocalSpan(null); public static ThreadLocalSpan create(Tracer tracer) { if (tracer == null) throw new NullPointerException("tracer == null"); return new ThreadLocalSpan(tracer); } - /** - * This keeps track of a stack with a normal array dequeue. Redundant stacking of the same span is - * not possible because there is no api to place an arbitrary span in scope using this api. - */ - @SuppressWarnings("ThreadLocalUsage") // intentional: to support multiple Tracer instances - final ThreadLocal> currentSpanInScope = - new ThreadLocal>() { - @Override protected Deque initialValue() { - return new ArrayDeque<>(); - } - }; - - final Tracer tracer; + @Nullable final Tracer tracer; ThreadLocalSpan(Tracer tracer) { this.tracer = tracer; } Tracer tracer() { - return tracer; + return tracer != null ? tracer : Tracing.currentTracer(); } /** @@ -121,8 +104,8 @@ Tracer tracer() { Tracer tracer = tracer(); if (tracer == null) return null; Span next = tracer.nextSpan(extracted); - SpanAndScope spanAndScope = new SpanAndScope(next, tracer.withSpanInScope(next)); - currentSpanInScope.get().addFirst(spanAndScope); + Object[] spanAndScope = {next, tracer.withSpanInScope(next)}; + getCurrentSpanInScopeStack().addFirst(spanAndScope); return next; } @@ -134,8 +117,8 @@ Tracer tracer() { Tracer tracer = tracer(); if (tracer == null) return null; Span next = tracer.nextSpan(); - SpanAndScope spanAndScope = new SpanAndScope(next, tracer.withSpanInScope(next)); - currentSpanInScope.get().addFirst(spanAndScope); + Object[] spanAndScope = {next, tracer.withSpanInScope(next)}; + getCurrentSpanInScopeStack().addFirst(spanAndScope); return next; } @@ -150,40 +133,29 @@ Tracer tracer() { @Nullable public Span remove() { Tracer tracer = tracer(); Span currentSpan = tracer != null ? tracer.currentSpan() : null; - SpanAndScope scope = currentSpanInScope.get().pollFirst(); - if (scope == null) return currentSpan; + Object[] spanAndScope = getCurrentSpanInScopeStack().pollFirst(); + if (spanAndScope == null) return currentSpan; - scope.scope.close(); - assert scope.span.equals(currentSpan) : - "Misalignment: scoped span " + scope.span + " != current span " + currentSpan; + Span span = (Span) spanAndScope[0]; + ((SpanInScope) spanAndScope[1]).close(); + assert span.equals(currentSpan) : + "Misalignment: scoped span " + span + " != current span " + currentSpan; return currentSpan; } - /** Allows state checks when nesting spans */ - static final class SpanAndScope { - - final Span span; - final SpanInScope scope; - - SpanAndScope(Span span, SpanInScope scope) { - this.span = span; - this.scope = scope; - } - - @Override public boolean equals(Object o) { - if (o == this) return true; - if (!(o instanceof SpanAndScope)) return false; - SpanAndScope that = (SpanAndScope) o; - return span.equals(that.span) && scope.equals(that.scope); - } + /** + * This keeps track of a stack with a normal array dequeue. Redundant stacking of the same span is + * not possible because there is no api to place an arbitrary span in scope using this api. + */ + @SuppressWarnings("ThreadLocalUsage") // intentional: to support multiple Tracer instances + final ThreadLocal> currentSpanInScopeStack = new ThreadLocal<>(); - @Override public int hashCode() { - int h = 1; - h *= 1000003; - h ^= span.hashCode(); - h *= 1000003; - h ^= scope.hashCode(); - return h; + ArrayDeque getCurrentSpanInScopeStack() { + ArrayDeque stack = currentSpanInScopeStack.get(); + if (stack == null) { + stack = new ArrayDeque<>(); + currentSpanInScopeStack.set(stack); } + return stack; } } diff --git a/brave/src/main/java/brave/propagation/TraceContext.java b/brave/src/main/java/brave/propagation/TraceContext.java index 9c41f60a3a..e6b1fcac76 100644 --- a/brave/src/main/java/brave/propagation/TraceContext.java +++ b/brave/src/main/java/brave/propagation/TraceContext.java @@ -1,11 +1,10 @@ package brave.propagation; import brave.internal.Nullable; +import brave.internal.Platform; import brave.internal.TraceContexts; import java.util.Collections; import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; import static brave.internal.HexCodec.lenientLowerHexToUnsignedLong; import static brave.internal.HexCodec.writeHexLong; @@ -25,8 +24,6 @@ */ //@Immutable public final class TraceContext extends SamplingFlags { - static final Logger LOG = Logger.getLogger(TraceContext.class.getName()); - /** * Used to send the trace context downstream. For example, as http headers. * @@ -298,7 +295,7 @@ final boolean parseTraceId(String traceIdString, Object key) { if (traceIdIndex > 0) { traceIdHigh = lenientLowerHexToUnsignedLong(traceIdString, 0, traceIdIndex); if (traceIdHigh == 0) { - maybeLogNotLowerHex(key, traceIdString); + maybeLogNotLowerHex(traceIdString); return false; } } @@ -306,7 +303,7 @@ final boolean parseTraceId(String traceIdString, Object key) { // right-most up to 16 characters are the low bits traceId = lenientLowerHexToUnsignedLong(traceIdString, traceIdIndex, length); if (traceId == 0) { - maybeLogNotLowerHex(key, traceIdString); + maybeLogNotLowerHex(traceIdString); return false; } return true; @@ -321,7 +318,7 @@ final boolean parseParentId(Propagation.Getter getter, C carrier, K parentId = lenientLowerHexToUnsignedLong(parentIdString, 0, length); if (parentId != 0) return true; - maybeLogNotLowerHex(key, parentIdString); + maybeLogNotLowerHex(parentIdString); return false; } @@ -334,7 +331,7 @@ final boolean parseSpanId(Propagation.Getter getter, C carrier, K k spanId = lenientLowerHexToUnsignedLong(spanIdString, 0, length); if (spanId == 0) { - maybeLogNotLowerHex(key, spanIdString); + maybeLogNotLowerHex(spanIdString); return false; } return true; @@ -342,22 +339,23 @@ final boolean parseSpanId(Propagation.Getter getter, C carrier, K k boolean invalidIdLength(Object key, int length, int max) { if (length > 1 && length <= max) return false; - if (LOG.isLoggable(Level.FINE)) { - LOG.fine(key + " should be a 1 to " + max + " character lower-hex string with no prefix"); - } + + assert max == 32 || max == 16; + Platform.get().log(max == 32 + ? "{0} should be a 1 to 32 character lower-hex string with no prefix" + : "{0} should be a 1 to 16 character lower-hex string with no prefix", key, null); + return true; } boolean isNull(Object key, String maybeNull) { if (maybeNull != null) return false; - if (LOG.isLoggable(Level.FINE)) LOG.fine(key + " was null"); + Platform.get().log("{0} was null", key, null); return true; } - void maybeLogNotLowerHex(Object key, String notLowerHex) { - if (LOG.isLoggable(Level.FINE)) { - LOG.fine(key + ": " + notLowerHex + " is not a lower-hex string"); - } + void maybeLogNotLowerHex(String notLowerHex) { + Platform.get().log("{0} is not a lower-hex string", notLowerHex, null); } public final TraceContext build() { diff --git a/brave/src/test/java/brave/TracerTest.java b/brave/src/test/java/brave/TracerTest.java index 4f9c28b509..d27798f503 100644 --- a/brave/src/test/java/brave/TracerTest.java +++ b/brave/src/test/java/brave/TracerTest.java @@ -59,6 +59,13 @@ public class TracerTest { Tracing.current().close(); } + @Test public void reporter_hasNiceToString() { + tracer = Tracing.newBuilder().build().tracer(); + + assertThat(tracer.spanReporter) + .hasToString("LoggingReporter{name=brave.Tracer}"); + } + @Test public void sampler() { Sampler sampler = new Sampler() { @Override public boolean isSampled(long traceId) { diff --git a/brave/src/test/java/brave/internal/PlatformTest.java b/brave/src/test/java/brave/internal/PlatformTest.java index 892acdcb87..4061b39118 100644 --- a/brave/src/test/java/brave/internal/PlatformTest.java +++ b/brave/src/test/java/brave/internal/PlatformTest.java @@ -43,11 +43,6 @@ public class PlatformTest { .hasToString("Clock.systemUTC().instant()"); } - @Test public void reporter_hasNiceToString() { - assertThat(platform.reporter()) - .hasToString("LoggingReporter{name=brave.Tracer}"); - } - // example from X-Amzn-Trace-Id: Root=1-5759e988-bd862e3fe1be46a994272793;Sampled=1 @Test public void randomLong_epochSecondsPlusRandom() { mockStatic(System.class); diff --git a/brave/src/test/java/brave/internal/recorder/MutableSpanConverterTest.java b/brave/src/test/java/brave/internal/recorder/MutableSpanConverterTest.java index bc06ead352..e8d92f8538 100644 --- a/brave/src/test/java/brave/internal/recorder/MutableSpanConverterTest.java +++ b/brave/src/test/java/brave/internal/recorder/MutableSpanConverterTest.java @@ -12,6 +12,8 @@ import static org.assertj.core.api.Assertions.entry; public class MutableSpanConverterTest { + final MutableSpanConverter converter = new MutableSpanConverter(); + @Test public void minimumDurationIsOne() { MutableSpan span = new MutableSpan(); @@ -152,16 +154,16 @@ void flush(Kind braveKind, Span.Kind span2Kind) { MutableSpan finishWithTimestamp = new MutableSpan(); finishWithTimestamp.finishTimestamp(2L); Span.Builder finishWithTimestampBuilder = Span.newBuilder(); - MutableSpanConverter.convert(finishWithTimestamp, finishWithTimestampBuilder); + converter.convert(finishWithTimestamp, finishWithTimestampBuilder); MutableSpan finishWithNoTimestamp = new MutableSpan(); finishWithNoTimestamp.finishTimestamp(0L); Span.Builder finishWithNoTimestampBuilder = Span.newBuilder(); - MutableSpanConverter.convert(finishWithNoTimestamp, finishWithNoTimestampBuilder); + converter.convert(finishWithNoTimestamp, finishWithNoTimestampBuilder); MutableSpan flush = new MutableSpan(); Span.Builder flushBuilder = Span.newBuilder(); - MutableSpanConverter.convert(flush, flushBuilder); + converter.convert(flush, flushBuilder); assertThat(finishWithTimestampBuilder) .isEqualToComparingFieldByFieldRecursively(finishWithNoTimestampBuilder) @@ -170,7 +172,7 @@ void flush(Kind braveKind, Span.Kind span2Kind) { Span convert(MutableSpan span) { Span.Builder result = Span.newBuilder().traceId(0L, 1L).id(1L); - MutableSpanConverter.convert(span, result); + converter.convert(span, result); return result.build(); } } diff --git a/context/log4j12/src/test/java/brave/context/log4j12/MDCCurrentTraceContextTest.java b/context/log4j12/src/test/java/brave/context/log4j12/MDCCurrentTraceContextTest.java index 67d85bd62f..2ed87ff0f0 100644 --- a/context/log4j12/src/test/java/brave/context/log4j12/MDCCurrentTraceContextTest.java +++ b/context/log4j12/src/test/java/brave/context/log4j12/MDCCurrentTraceContextTest.java @@ -5,6 +5,7 @@ import brave.propagation.CurrentTraceContext; import brave.propagation.TraceContext; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.apache.log4j.MDC; import org.junit.ComparisonFailure; import org.junit.Test; @@ -13,8 +14,14 @@ public class MDCCurrentTraceContextTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return MDCCurrentTraceContext.create(); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return MDCCurrentTraceContext.create(); + } } @Test public void is_inheritable() throws Exception { diff --git a/context/log4j12/src/test/java/brave/context/log4j12/MDCScopeDecoratorTest.java b/context/log4j12/src/test/java/brave/context/log4j12/MDCScopeDecoratorTest.java index 35d941752a..b10d8a07c2 100644 --- a/context/log4j12/src/test/java/brave/context/log4j12/MDCScopeDecoratorTest.java +++ b/context/log4j12/src/test/java/brave/context/log4j12/MDCScopeDecoratorTest.java @@ -6,6 +6,7 @@ import brave.propagation.ThreadLocalCurrentTraceContext; import brave.propagation.TraceContext; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.apache.log4j.MDC; import org.junit.ComparisonFailure; import org.junit.Test; @@ -14,10 +15,16 @@ public class MDCScopeDecoratorTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return ThreadLocalCurrentTraceContext.newBuilder() - .addScopeDecorator(MDCScopeDecorator.create()) - .build(); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(MDCScopeDecorator.create()) + .build(); + } } @Test(expected = ComparisonFailure.class) // Log4J 1.2.x MDC is inheritable by default diff --git a/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextCurrentTraceContextTest.java b/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextCurrentTraceContextTest.java index 3220fa8a14..1a5a1f64c5 100644 --- a/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextCurrentTraceContextTest.java +++ b/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextCurrentTraceContextTest.java @@ -5,14 +5,21 @@ import brave.propagation.CurrentTraceContext; import brave.propagation.TraceContext; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.apache.logging.log4j.ThreadContext; import static org.assertj.core.api.Assertions.assertThat; public class ThreadContextCurrentTraceContextTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return ThreadContextCurrentTraceContext.create(CurrentTraceContext.Default.create()); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return ThreadContextCurrentTraceContext.create(CurrentTraceContext.Default.create()); + } } @Override protected void verifyImplicitContext(@Nullable TraceContext context) { diff --git a/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextScopeDecoratorTest.java b/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextScopeDecoratorTest.java index 3d49103a59..da96fc5a0c 100644 --- a/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextScopeDecoratorTest.java +++ b/context/log4j2/src/test/java/brave/context/log4j2/ThreadContextScopeDecoratorTest.java @@ -6,16 +6,23 @@ import brave.propagation.ThreadLocalCurrentTraceContext; import brave.propagation.TraceContext; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.apache.logging.log4j.ThreadContext; import static org.assertj.core.api.Assertions.assertThat; public class ThreadContextScopeDecoratorTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return ThreadLocalCurrentTraceContext.newBuilder() - .addScopeDecorator(ThreadContextScopeDecorator.create()) - .build(); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(ThreadContextScopeDecorator.create()) + .build(); + } } @Override protected void verifyImplicitContext(@Nullable TraceContext context) { diff --git a/context/slf4j/src/test/java/brave/context/slf4j/MDCCurrentTraceContextTest.java b/context/slf4j/src/test/java/brave/context/slf4j/MDCCurrentTraceContextTest.java index 1f14e08901..c47db64cfe 100644 --- a/context/slf4j/src/test/java/brave/context/slf4j/MDCCurrentTraceContextTest.java +++ b/context/slf4j/src/test/java/brave/context/slf4j/MDCCurrentTraceContextTest.java @@ -5,14 +5,21 @@ import brave.propagation.CurrentTraceContext; import brave.propagation.TraceContext; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.slf4j.MDC; import static org.assertj.core.api.Assertions.assertThat; public class MDCCurrentTraceContextTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return MDCCurrentTraceContext.create(CurrentTraceContext.Default.create()); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return MDCCurrentTraceContext.create(CurrentTraceContext.Default.create()); + } } @Override protected void verifyImplicitContext(@Nullable TraceContext context) { diff --git a/context/slf4j/src/test/java/brave/context/slf4j/MDCScopeDecoratorTest.java b/context/slf4j/src/test/java/brave/context/slf4j/MDCScopeDecoratorTest.java index ff2e0ed8bd..66d9ab563a 100644 --- a/context/slf4j/src/test/java/brave/context/slf4j/MDCScopeDecoratorTest.java +++ b/context/slf4j/src/test/java/brave/context/slf4j/MDCScopeDecoratorTest.java @@ -6,16 +6,23 @@ import brave.propagation.ThreadLocalCurrentTraceContext; import brave.propagation.TraceContext; import brave.test.propagation.CurrentTraceContextTest; +import java.util.function.Supplier; import org.slf4j.MDC; import static org.assertj.core.api.Assertions.assertThat; public class MDCScopeDecoratorTest extends CurrentTraceContextTest { - @Override protected CurrentTraceContext newCurrentTraceContext() { - return ThreadLocalCurrentTraceContext.newBuilder() - .addScopeDecorator(MDCScopeDecorator.create()) - .build(); + @Override protected Class> currentSupplier() { + return CurrentSupplier.class; + } + + static class CurrentSupplier implements Supplier { + @Override public CurrentTraceContext get() { + return ThreadLocalCurrentTraceContext.newBuilder() + .addScopeDecorator(MDCScopeDecorator.create()) + .build(); + } } protected void verifyImplicitContext(@Nullable TraceContext context) { diff --git a/instrumentation/benchmarks/pom.xml b/instrumentation/benchmarks/pom.xml index 02ca261c18..430ecfaa58 100644 --- a/instrumentation/benchmarks/pom.xml +++ b/instrumentation/benchmarks/pom.xml @@ -158,16 +158,6 @@ - - ${project.groupId} - brave-instrumentation-spring-rabbit - ${project.version} - - - org.springframework.amqp - spring-rabbit - ${spring-rabbit.version} - ${project.groupId} diff --git a/instrumentation/benchmarks/src/main/java/brave/internal/recorder/MutableSpanConverterBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/internal/recorder/MutableSpanConverterBenchmarks.java index fae1cfe2ae..e4a2fc7f3b 100644 --- a/instrumentation/benchmarks/src/main/java/brave/internal/recorder/MutableSpanConverterBenchmarks.java +++ b/instrumentation/benchmarks/src/main/java/brave/internal/recorder/MutableSpanConverterBenchmarks.java @@ -41,6 +41,7 @@ @State(Scope.Thread) @Threads(1) public class MutableSpanConverterBenchmarks { + final MutableSpanConverter converter = new MutableSpanConverter(); final MutableSpan serverMutableSpan = newServerMutableSpan(); final MutableSpan bigClientMutableSpan = newBigClientMutableSpan(); @@ -50,13 +51,13 @@ public class MutableSpanConverterBenchmarks { */ @Benchmark public Span.Builder convertServerSpan() { Span.Builder builder = Span.newBuilder(); - MutableSpanConverter.convert(serverMutableSpan, builder); + converter.convert(serverMutableSpan, builder); return builder; } @Benchmark public Span.Builder convertBigClientSpan() { Span.Builder builder = Span.newBuilder(); - MutableSpanConverter.convert(bigClientMutableSpan, builder); + converter.convert(bigClientMutableSpan, builder); return builder; } diff --git a/instrumentation/grpc/src/main/java/brave/grpc/TagContextBinaryMarshaller.java b/instrumentation/grpc/src/main/java/brave/grpc/TagContextBinaryMarshaller.java index 4b5611b369..a62923a539 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/TagContextBinaryMarshaller.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/TagContextBinaryMarshaller.java @@ -1,12 +1,10 @@ package brave.grpc; +import brave.internal.Platform; import io.grpc.Metadata.BinaryMarshaller; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; -import java.util.logging.Logger; - -import static java.util.logging.Level.FINE; /** * This logs instead of throwing exceptions. @@ -15,7 +13,6 @@ * https://github.com/census-instrumentation/opencensus-java/blob/master/impl_core/src/main/java/io/opencensus/implcore/tags/propagation/SerializationUtils.java */ final class TagContextBinaryMarshaller implements BinaryMarshaller> { - static final Logger logger = Logger.getLogger(TagContextBinaryMarshaller.class.getName()); static final byte VERSION = 0, TAG_FIELD_ID = 0; static final byte[] EMPTY_BYTES = {}; @@ -43,7 +40,7 @@ public Map parseBytes(byte[] buf) { Buffer bytes = new Buffer(buf); byte version = bytes.readByte(); if (version != VERSION) { - logger.log(FINE, "Invalid input: unsupported version {0}", version); + Platform.get().log("Invalid input: unsupported version {0}", version, null); return null; } @@ -56,7 +53,7 @@ public Map parseBytes(byte[] buf) { if (val == null) break; result.put(key, val); } else { - logger.log(FINE, "Invalid input: expected TAG_FIELD_ID at offset {0}", bytes.pos); + Platform.get().log("Invalid input: expected TAG_FIELD_ID at offset {0}", bytes.pos, null); break; } } @@ -132,7 +129,7 @@ String readLengthPrefixed() { private int readVarint(byte b1) { int b2 = buf[pos++]; if ((b2 & 0xf0) != 0) { - logger.log(FINE, "Greater than 14-bit varint at position {0}", pos); + Platform.get().log("Greater than 14-bit varint at position {0}", pos, null); return -1; } return b1 & 0x7f | b2 << 28; diff --git a/instrumentation/grpc/src/main/java/brave/grpc/TraceContextBinaryFormat.java b/instrumentation/grpc/src/main/java/brave/grpc/TraceContextBinaryFormat.java index 624f015939..71d80e5131 100644 --- a/instrumentation/grpc/src/main/java/brave/grpc/TraceContextBinaryFormat.java +++ b/instrumentation/grpc/src/main/java/brave/grpc/TraceContextBinaryFormat.java @@ -2,12 +2,11 @@ import brave.grpc.GrpcPropagation.Tags; import brave.internal.Nullable; +import brave.internal.Platform; import brave.propagation.TraceContext; import java.util.Collections; -import java.util.logging.Logger; import static com.google.common.base.Preconditions.checkNotNull; -import static java.util.logging.Level.FINE; /** * This logs instead of throwing exceptions. @@ -16,7 +15,6 @@ * https://github.com/census-instrumentation/opencensus-specs/blob/master/encodings/BinaryEncoding.md */ final class TraceContextBinaryFormat { - static final Logger logger = Logger.getLogger(TraceContextBinaryFormat.class.getName()); static final byte VERSION = 0, TRACE_ID_FIELD_ID = 0, SPAN_ID_FIELD_ID = 1, @@ -45,11 +43,11 @@ static byte[] toBytes(TraceContext traceContext) { if (bytes == null) throw new NullPointerException("bytes == null"); // programming error if (bytes.length == 0) return null; if (bytes[0] != VERSION) { - logger.log(FINE, "Invalid input: unsupported version {0}", bytes[0]); + Platform.get().log("Invalid input: unsupported version {0}", bytes[0], null); return null; } if (bytes.length < FORMAT_LENGTH - 2 /* sampled field + bit is optional */) { - logger.fine("Invalid input: truncated"); + Platform.get().log("Invalid input: truncated", null); return null; } long traceIdHigh, traceId, spanId; @@ -60,7 +58,7 @@ static byte[] toBytes(TraceContext traceContext) { traceId = readLong(bytes, pos + 8); pos += 16; } else { - logger.log(FINE, "Invalid input: expected trace ID at offset {0}", pos); + Platform.get().log("Invalid input: expected trace ID at offset {0}", pos, null); return null; } if (bytes[pos] == SPAN_ID_FIELD_ID) { @@ -68,7 +66,7 @@ static byte[] toBytes(TraceContext traceContext) { spanId = readLong(bytes, pos); pos += 8; } else { - logger.log(FINE, "Invalid input: expected span ID at offset {0}", pos); + Platform.get().log("Invalid input: expected span ID at offset {0}", pos, null); return null; } // The trace options field is optional. However, when present, it should be valid. @@ -76,7 +74,7 @@ static byte[] toBytes(TraceContext traceContext) { if (bytes.length > pos && bytes[pos] == TRACE_OPTION_FIELD_ID) { pos++; if (bytes.length < pos + 1) { - logger.log(FINE, "Invalid input: truncated"); + Platform.get().log("Invalid input: truncated", null); return null; } sampled = bytes[pos] == 1;