Merge pull request #986 from DataDog/tyler/sampler-locking

Remove synchronization from RateByServiceSampler
This commit is contained in:
Tyler Benson 2019-09-10 15:43:27 -07:00 committed by GitHub
commit d10b99a5db
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 111 additions and 52 deletions

View File

@ -150,9 +150,14 @@ public class ClassLoaderMatcher {
@Override @Override
public boolean matches(final ClassLoader target) { public boolean matches(final ClassLoader target) {
if (target != null) { if (target != null) {
Boolean result = cache.get(target);
if (result != null) {
return result;
}
synchronized (target) { synchronized (target) {
if (cache.containsKey(target)) { result = cache.get(target);
return cache.get(target); if (result != null) {
return result;
} }
for (final String name : names) { for (final String name : names) {
if (target.getResource(Utils.getResourceName(name)) == null) { if (target.getResource(Utils.getResourceName(name)) == null) {
@ -184,9 +189,14 @@ public class ClassLoaderMatcher {
@Override @Override
public boolean matches(final ClassLoader target) { public boolean matches(final ClassLoader target) {
if (target != null) { if (target != null) {
Boolean result = cache.get(target);
if (result != null) {
return result;
}
synchronized (target) { synchronized (target) {
if (cache.containsKey(target)) { result = cache.get(target);
return cache.get(target); if (result != null) {
return result;
} }
try { try {
final Class<?> aClass = Class.forName(className, false, target); final Class<?> aClass = Class.forName(className, false, target);
@ -225,9 +235,14 @@ public class ClassLoaderMatcher {
@Override @Override
public boolean matches(final ClassLoader target) { public boolean matches(final ClassLoader target) {
if (target != null) { if (target != null) {
Boolean result = cache.get(target);
if (result != null) {
return result;
}
synchronized (target) { synchronized (target) {
if (cache.containsKey(target)) { result = cache.get(target);
return cache.get(target); if (result != null) {
return result;
} }
try { try {
final Class<?> aClass = Class.forName(className, false, target); final Class<?> aClass = Class.forName(className, false, target);

View File

@ -1,12 +1,17 @@
package datadog.trace.common.sampling; package datadog.trace.common.sampling;
import static java.util.Collections.singletonMap;
import static java.util.Collections.unmodifiableMap;
import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.NumericNode;
import datadog.opentracing.DDSpan; import datadog.opentracing.DDSpan;
import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.api.sampling.PrioritySampling;
import datadog.trace.common.writer.DDApi.ResponseListener; import datadog.trace.common.writer.DDApi.ResponseListener;
import java.util.HashMap; import java.util.HashMap;
import java.util.Iterator; import java.util.Iterator;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ThreadLocalRandom;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
/** /**
@ -16,22 +21,23 @@ import lombok.extern.slf4j.Slf4j;
*/ */
@Slf4j @Slf4j
public class RateByServiceSampler implements Sampler, ResponseListener { public class RateByServiceSampler implements Sampler, ResponseListener {
/** Key for setting the baseline rate */ /** Key for setting the default/baseline rate */
private static final String BASE_KEY = "service:,env:"; private static final String DEFAULT_KEY = "service:,env:";
/** Sampler to use if service+env is not in the map */
private RateSampler baseSampler = new RateSampler(1.0);
private final Map<String, RateSampler> serviceRates = new HashMap<String, RateSampler>(); private static final double DEFAULT_RATE = 1.0;
private volatile Map<String, RateSampler> serviceRates =
unmodifiableMap(singletonMap(DEFAULT_KEY, new RateSampler(DEFAULT_RATE)));
@Override @Override
public synchronized boolean sample(DDSpan span) { public boolean sample(final DDSpan span) {
// Priority sampling sends all traces to the core agent, including traces marked dropped. // Priority sampling sends all traces to the core agent, including traces marked dropped.
// This allows the core agent to collect stats on all traces. // This allows the core agent to collect stats on all traces.
return true; return true;
} }
/** If span is a root span, set the span context samplingPriority to keep or drop */ /** If span is a root span, set the span context samplingPriority to keep or drop */
public void initializeSamplingPriority(DDSpan span) { public void initializeSamplingPriority(final DDSpan span) {
if (span.isRootSpan()) { if (span.isRootSpan()) {
// Run the priority sampler on the new span // Run the priority sampler on the new span
setSamplingPriorityOnSpanContext(span); setSamplingPriorityOnSpanContext(span);
@ -42,16 +48,17 @@ public class RateByServiceSampler implements Sampler, ResponseListener {
} }
} }
private synchronized void setSamplingPriorityOnSpanContext(DDSpan span) { private void setSamplingPriorityOnSpanContext(final DDSpan span) {
final String serviceName = span.getServiceName(); final String serviceName = span.getServiceName();
final String env = getSpanEnv(span); final String env = getSpanEnv(span);
final String key = "service:" + serviceName + ",env:" + env; final String key = "service:" + serviceName + ",env:" + env;
final RateSampler sampler;
if (serviceRates.containsKey(key)) { final Map<String, RateSampler> rates = serviceRates;
sampler = serviceRates.get(key); RateSampler sampler = serviceRates.get(key);
} else { if (sampler == null) {
sampler = baseSampler; sampler = rates.get(DEFAULT_KEY);
} }
if (sampler.sample(span)) { if (sampler.sample(span)) {
span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP); span.setSamplingPriority(PrioritySampling.SAMPLER_KEEP);
} else { } else {
@ -59,32 +66,34 @@ public class RateByServiceSampler implements Sampler, ResponseListener {
} }
} }
private static String getSpanEnv(DDSpan span) { private static String getSpanEnv(final DDSpan span) {
return null == span.getTags().get("env") ? "" : String.valueOf(span.getTags().get("env")); return null == span.getTags().get("env") ? "" : String.valueOf(span.getTags().get("env"));
} }
@Override @Override
public void onResponse(String endpoint, JsonNode responseJson) { public void onResponse(final String endpoint, final JsonNode responseJson) {
JsonNode newServiceRates = responseJson.get("rate_by_service"); final JsonNode newServiceRates = responseJson.get("rate_by_service");
if (null != newServiceRates) { if (null != newServiceRates) {
log.debug("Update service sampler rates: {} -> {}", endpoint, responseJson); log.debug("Update service sampler rates: {} -> {}", endpoint, responseJson);
synchronized (this) { final Map<String, RateSampler> updatedServiceRates = new HashMap<>();
serviceRates.clear(); final Iterator<String> itr = newServiceRates.fieldNames();
Iterator<String> itr = newServiceRates.fieldNames(); while (itr.hasNext()) {
while (itr.hasNext()) { final String key = itr.next();
final String key = itr.next(); final JsonNode value = newServiceRates.get(key);
try { try {
final float val = Float.parseFloat(newServiceRates.get(key).toString()); if (value instanceof NumericNode) {
if (BASE_KEY.equals(key)) { updatedServiceRates.put(key, new RateSampler(value.doubleValue()));
baseSampler = new RateSampler(val); } else {
} else { log.debug("Unable to parse new service rate {} -> {}", key, value);
serviceRates.put(key, new RateSampler(val));
}
} catch (NumberFormatException nfe) {
log.debug("Unable to parse new service rate {} -> {}", key, newServiceRates.get(key));
} }
} catch (final NumberFormatException nfe) {
log.debug("Unable to parse new service rate {} -> {}", key, value);
} }
} }
if (!updatedServiceRates.containsKey(DEFAULT_KEY)) {
updatedServiceRates.put(DEFAULT_KEY, new RateSampler(DEFAULT_RATE));
}
serviceRates = unmodifiableMap(updatedServiceRates);
} }
} }
@ -99,18 +108,14 @@ public class RateByServiceSampler implements Sampler, ResponseListener {
/** The sample rate used */ /** The sample rate used */
private final double sampleRate; private final double sampleRate;
public RateSampler(final String sampleRate) {
this(sampleRate == null ? 1 : Double.valueOf(sampleRate));
}
/** /**
* Build an instance of the sampler. The Sample rate is fixed for each instance. * Build an instance of the sampler. The Sample rate is fixed for each instance.
* *
* @param sampleRate a number [0,1] representing the rate ratio. * @param sampleRate a number [0,1] representing the rate ratio.
*/ */
public RateSampler(double sampleRate) { private RateSampler(double sampleRate) {
if (sampleRate <= 0) { if (sampleRate < 0) {
sampleRate = 1; sampleRate = 1;
log.error("SampleRate is negative or null, disabling the sampler"); log.error("SampleRate is negative or null, disabling the sampler");
} else if (sampleRate > 1) { } else if (sampleRate > 1) {
@ -123,13 +128,13 @@ public class RateByServiceSampler implements Sampler, ResponseListener {
@Override @Override
public boolean doSample(final DDSpan span) { public boolean doSample(final DDSpan span) {
final boolean sample = Math.random() <= this.sampleRate; final boolean sample = ThreadLocalRandom.current().nextFloat() <= sampleRate;
log.debug("{} - Span is sampled: {}", span, sample); log.debug("{} - Span is sampled: {}", span, sample);
return sample; return sample;
} }
public double getSampleRate() { public double getSampleRate() {
return this.sampleRate; return sampleRate;
} }
@Override @Override

View File

@ -1,8 +1,11 @@
package datadog.opentracing package datadog.opentracing
import com.fasterxml.jackson.databind.ObjectMapper
import datadog.opentracing.propagation.ExtractedContext import datadog.opentracing.propagation.ExtractedContext
import datadog.opentracing.propagation.TagContext import datadog.opentracing.propagation.TagContext
import datadog.trace.agent.test.utils.ConfigUtils import datadog.trace.agent.test.utils.ConfigUtils
import datadog.trace.api.DDTags
import datadog.trace.api.sampling.PrioritySampling import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.common.sampling.RateByServiceSampler import datadog.trace.common.sampling.RateByServiceSampler
import datadog.trace.common.writer.ListWriter import datadog.trace.common.writer.ListWriter
@ -20,11 +23,17 @@ class DDSpanTest extends Specification {
} }
def writer = new ListWriter() def writer = new ListWriter()
def tracer = new DDTracer(DEFAULT_SERVICE_NAME, writer, new RateByServiceSampler(), [:]) def sampler = new RateByServiceSampler()
def tracer = new DDTracer(DEFAULT_SERVICE_NAME, writer, sampler, [:])
@Shared @Shared
def defaultSamplingPriority = PrioritySampling.SAMPLER_KEEP def defaultSamplingPriority = PrioritySampling.SAMPLER_KEEP
def setup() {
sampler.onResponse("test", new ObjectMapper()
.readTree('{"rate_by_service":{"service:,env:":1.0,"service:spock,env:":0.0}}'))
}
def "getters and setters"() { def "getters and setters"() {
setup: setup:
final DDSpanContext context = final DDSpanContext context =
@ -254,8 +263,29 @@ class DDSpanTest extends Specification {
new ExtractedContext("123", "456", 1, "789", [:], [:]) | false new ExtractedContext("123", "456", 1, "789", [:], [:]) | false
} }
def "setting forced tracing via tag"() { def "sampling priority set on init"() {
setup:
def span = tracer.buildSpan("test").start()
expect:
span.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP
when:
span.setTag(DDTags.SERVICE_NAME, "spock")
then:
// FIXME: priority currently only applies if service name set before span started.
span.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP
// span.getSamplingPriority() == PrioritySampling.SAMPLER_DROP
when:
span = tracer.buildSpan("test").withTag(DDTags.SERVICE_NAME, "spock").start()
then:
span.getSamplingPriority() == PrioritySampling.SAMPLER_DROP
}
def "setting forced tracing via tag"() {
setup: setup:
def span = tracer.buildSpan("root").start() def span = tracer.buildSpan("root").start()
if (tagName) { if (tagName) {

View File

@ -19,6 +19,7 @@ import static java.util.Collections.emptyMap
class SpanDecoratorTest extends Specification { class SpanDecoratorTest extends Specification {
static { static {
ConfigUtils.makeConfigInstanceModifiable()
ConfigUtils.updateConfig { ConfigUtils.updateConfig {
System.setProperty("dd.$Config.SPLIT_BY_TAGS", "sn.tag1,sn.tag2") System.setProperty("dd.$Config.SPLIT_BY_TAGS", "sn.tag1,sn.tag2")
} }

View File

@ -1,5 +1,6 @@
package datadog.opentracing.propagation package datadog.opentracing.propagation
import datadog.trace.agent.test.utils.ConfigUtils
import datadog.trace.api.Config import datadog.trace.api.Config
import io.opentracing.SpanContext import io.opentracing.SpanContext
import io.opentracing.propagation.TextMapExtractAdapter import io.opentracing.propagation.TextMapExtractAdapter
@ -11,6 +12,9 @@ import static datadog.trace.api.Config.PropagationStyle.B3
import static datadog.trace.api.Config.PropagationStyle.DATADOG import static datadog.trace.api.Config.PropagationStyle.DATADOG
class HttpExtractorTest extends Specification { class HttpExtractorTest extends Specification {
static {
ConfigUtils.makeConfigInstanceModifiable()
}
@Shared @Shared
String outOfRangeTraceId = UINT64_MAX.add(BigInteger.ONE) String outOfRangeTraceId = UINT64_MAX.add(BigInteger.ONE)

View File

@ -3,6 +3,7 @@ package datadog.opentracing.propagation
import datadog.opentracing.DDSpanContext import datadog.opentracing.DDSpanContext
import datadog.opentracing.DDTracer import datadog.opentracing.DDTracer
import datadog.opentracing.PendingTrace import datadog.opentracing.PendingTrace
import datadog.trace.agent.test.utils.ConfigUtils
import datadog.trace.api.Config import datadog.trace.api.Config
import datadog.trace.api.sampling.PrioritySampling import datadog.trace.api.sampling.PrioritySampling
import datadog.trace.common.writer.ListWriter import datadog.trace.common.writer.ListWriter
@ -13,6 +14,9 @@ import static datadog.trace.api.Config.PropagationStyle.B3
import static datadog.trace.api.Config.PropagationStyle.DATADOG import static datadog.trace.api.Config.PropagationStyle.DATADOG
class HttpInjectorTest extends Specification { class HttpInjectorTest extends Specification {
static {
ConfigUtils.makeConfigInstanceModifiable()
}
def "inject http headers"() { def "inject http headers"() {
setup: setup:

View File

@ -6,6 +6,8 @@ import datadog.opentracing.SpanFactory
import datadog.trace.common.sampling.RateByServiceSampler import datadog.trace.common.sampling.RateByServiceSampler
import spock.lang.Specification import spock.lang.Specification
import static datadog.trace.common.sampling.RateByServiceSampler.DEFAULT_KEY
class RateByServiceSamplerTest extends Specification { class RateByServiceSamplerTest extends Specification {
def "invalid rate -> 1"() { def "invalid rate -> 1"() {
@ -15,13 +17,13 @@ class RateByServiceSamplerTest extends Specification {
String response = '{"rate_by_service": {"service:,env:":' + rate + '}}' String response = '{"rate_by_service": {"service:,env:":' + rate + '}}'
serviceSampler.onResponse("traces", serializer.readTree(response)) serviceSampler.onResponse("traces", serializer.readTree(response))
expect: expect:
serviceSampler.baseSampler.sampleRate == expectedRate serviceSampler.serviceRates[DEFAULT_KEY].sampleRate == expectedRate
where: where:
rate | expectedRate rate | expectedRate
null | 1 null | 1
1 | 1 1 | 1
0 | 1 0 | 0.0
-5 | 1 -5 | 1
5 | 1 5 | 1
0.5 | 0.5 0.5 | 0.5
@ -33,22 +35,20 @@ class RateByServiceSamplerTest extends Specification {
ObjectMapper serializer = new ObjectMapper() ObjectMapper serializer = new ObjectMapper()
when: when:
String response = '{"rate_by_service": {"service:,env:":1.0, "service:spock,env:test":0.000001}}' String response = '{"rate_by_service": {"service:spock,env:test":0.0}}'
serviceSampler.onResponse("traces", serializer.readTree(response)) serviceSampler.onResponse("traces", serializer.readTree(response))
DDSpan span1 = SpanFactory.newSpanOf("foo", "bar") DDSpan span1 = SpanFactory.newSpanOf("foo", "bar")
serviceSampler.initializeSamplingPriority(span1) serviceSampler.initializeSamplingPriority(span1)
then: then:
span1.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP span1.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP
serviceSampler.sample(span1) serviceSampler.sample(span1)
// !serviceSampler.sample(SpanFactory.newSpanOf("spock", "test"))
when: when:
response = '{"rate_by_service": {"service:,env:":0.000001, "service:spock,env:test":1.0}}' response = '{"rate_by_service": {"service:spock,env:test":1.0}}'
serviceSampler.onResponse("traces", serializer.readTree(response)) serviceSampler.onResponse("traces", serializer.readTree(response))
DDSpan span2 = SpanFactory.newSpanOf("spock", "test") DDSpan span2 = SpanFactory.newSpanOf("spock", "test")
serviceSampler.initializeSamplingPriority(span2) serviceSampler.initializeSamplingPriority(span2)
then: then:
// !serviceSampler.sample(SpanFactory.newSpanOf("foo", "bar"))
span2.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP span2.getSamplingPriority() == PrioritySampling.SAMPLER_KEEP
serviceSampler.sample(span2) serviceSampler.sample(span2)
} }