Skip to content

Commit

Permalink
[improvement] Set the default sampler to random with 1% (#158)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
diogoholanda authored and bulldozer-bot[bot] committed May 21, 2019
1 parent c2aa08c commit 7ad39a2
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion tracing/src/main/java/com/palantir/tracing/Tracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 7ad39a2

Please sign in to comment.