From 7ad39a2050af1dee5b0715002dfa8369f72dc6ff Mon Sep 17 00:00:00 2001
From: Diogo Holanda <diogoholanda@hotmail.com>
Date: Tue, 21 May 2019 15:04:37 +0100
Subject: [PATCH] [improvement] Set the default sampler to random with 1%
 (#158)

## Before this PR
The default sampler would sample all traces. I would claim this is not a great default and is somewhat error prone. It expects all consumers to override it to an appropriate value, with the penalty to failing to doing so being a high number of tracing logs that puts a lot of pressure in logging pipelines.

## After this PR
The default sampler is random with 1% probability of sampling.
---
 .../palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java   | 1 +
 tracing/src/main/java/com/palantir/tracing/Tracer.java         | 2 +-
 .../java/com/palantir/tracing/AsyncSlf4jSpanObserverTest.java  | 3 +++
 .../src/test/java/com/palantir/tracing/AsyncTracerTest.java    | 3 ++-
 4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java
index 3cfab479e..09ae66963 100644
--- a/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java
+++ b/tracing-okhttp3/src/test/java/com/palantir/tracing/okhttp3/OkhttpTraceInterceptorTest.java
@@ -105,6 +105,7 @@ public void testPopulatesNewTrace_whenParentTraceIsPresent() throws IOException
 
     @Test
     public void testAddsIsSampledHeader_whenTraceIsObservable() throws IOException {
+        Tracer.initTrace(Optional.of(true), Tracers.randomId());
         OkhttpTraceInterceptor.INSTANCE.intercept(chain);
         verify(chain).proceed(requestCaptor.capture());
         assertThat(requestCaptor.getValue().header(TraceHttpHeaders.IS_SAMPLED)).isEqualTo("1");
diff --git a/tracing/src/main/java/com/palantir/tracing/Tracer.java b/tracing/src/main/java/com/palantir/tracing/Tracer.java
index 95dd56e7e..106ea9e60 100644
--- a/tracing/src/main/java/com/palantir/tracing/Tracer.java
+++ b/tracing/src/main/java/com/palantir/tracing/Tracer.java
@@ -59,7 +59,7 @@ private Tracer() {}
     private static volatile Consumer<Span> compositeObserver = span -> { };
 
     // Thread-safe since stateless
-    private static volatile TraceSampler sampler = AlwaysSampler.INSTANCE;
+    private static volatile TraceSampler sampler = new RandomSampler(0.01f);
 
     /**
      * Creates a new trace, but does not set it as the current trace. The new trace is {@link Trace#isObservable
diff --git a/tracing/src/test/java/com/palantir/tracing/AsyncSlf4jSpanObserverTest.java b/tracing/src/test/java/com/palantir/tracing/AsyncSlf4jSpanObserverTest.java
index 440e208a5..7b8c3c94d 100644
--- a/tracing/src/test/java/com/palantir/tracing/AsyncSlf4jSpanObserverTest.java
+++ b/tracing/src/test/java/com/palantir/tracing/AsyncSlf4jSpanObserverTest.java
@@ -39,6 +39,7 @@
 import java.net.InetAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.stream.Collectors;
 import org.jmock.lib.concurrent.DeterministicScheduler;
@@ -98,6 +99,7 @@ public void testJsonFormatToLog() throws Exception {
         DeterministicScheduler executor = new DeterministicScheduler();
         Tracer.subscribe(TEST_OBSERVER, AsyncSlf4jSpanObserver.of(
                 "serviceName", Inet4Address.getLoopbackAddress(), logger, executor));
+        Tracer.initTrace(Optional.of(true), Tracers.randomId());
         Tracer.startSpan("operation");
         Span span = Tracer.completeSpan().get();
         verify(appender, never()).doAppend(any(ILoggingEvent.class)); // async logger only fires when executor runs
@@ -118,6 +120,7 @@ public void testJsonFormatToLog() throws Exception {
     public void testDefaultConstructorDeterminesIpAddress() throws Exception {
         DeterministicScheduler executor = new DeterministicScheduler();
         Tracer.subscribe(TEST_OBSERVER, AsyncSlf4jSpanObserver.of("serviceName", executor));
+        Tracer.initTrace(Optional.of(true), Tracers.randomId());
         Tracer.startSpan("operation");
         Span span = Tracer.completeSpan().get();
 
diff --git a/tracing/src/test/java/com/palantir/tracing/AsyncTracerTest.java b/tracing/src/test/java/com/palantir/tracing/AsyncTracerTest.java
index e949f9c83..259cf2af5 100644
--- a/tracing/src/test/java/com/palantir/tracing/AsyncTracerTest.java
+++ b/tracing/src/test/java/com/palantir/tracing/AsyncTracerTest.java
@@ -42,7 +42,8 @@ public void doesNotLeakEnqueueSpan() {
 
     @Test
     public void completesBothDeferredSpans() {
-        Tracer.initTrace(Optional.empty(), "defaultTraceId");
+        Tracer.initTrace(Optional.of(true), "defaultTraceId");
+        Tracer.startSpan("defaultSpan");
         AsyncTracer asyncTracer = new AsyncTracer();
         List<String> observedSpans = Lists.newArrayList();
         Tracer.subscribe(