diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/PercentageBasedSampler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/PercentageBasedSampler.java index 1fc3a0a88..5c879cd1c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/PercentageBasedSampler.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/PercentageBasedSampler.java @@ -4,20 +4,16 @@ import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; /** - * Sampler that based on the given percentage rate will allow sampling. - *
+ * This sampler is appropriate for low-traffic instrumentation (ex servers that each receive <100K + * requests), or those who do not provision random trace ids. It not appropriate for collectors as + * the sampling decision isn't idempotent (consistent based on trace id). * - * A couple of assumptions have to take place in order for the algorithm to work properly: - *
+ *
Taken from Zipkin project
* - * The value provided from sampler configuration in terms of percentage is an estimation. It might occur that amount - * of data sampled differs from the provided percentage. + *This counts to see how many out of 100 traces should be retained. This means that it is
+ * accurate in units of 100 traces.
*
* @author Marcin Grzejszczak
* @author Adrian Cole
@@ -25,21 +21,33 @@ import org.springframework.cloud.sleuth.Span;
*/
public class PercentageBasedSampler implements Sampler {
- private final SamplerProperties configuration;
+ private final int outOf100;
+
+ private int i = 0; // guarded by this
+ private boolean skipping = false; // guarded by this
public PercentageBasedSampler(SamplerProperties configuration) {
- this.configuration = configuration;
+ this.outOf100 = (int) (configuration.getPercentage() * 100.0f);;
}
@Override
public boolean isSampled(Span currentSpan) {
- long threshold = Math.abs(Long.MAX_VALUE * (int) (this.configuration.getPercentage() * 100)); // drops fractional percentage.
- if (currentSpan == null || threshold == 0L) {
+ if (this.outOf100 == 0 || currentSpan == null) {
return false;
+ } else if (this.outOf100 == 100) {
+ return true;
+ }
+ synchronized (this) {
+ boolean result = !this.skipping;
+ this.i = this.i + 1;
+ if (this.i == this.outOf100) {
+ this.skipping = true;
+ } else if (this.i == 100) {
+ this.i = 0;
+ this.skipping = false;
+ }
+ return result;
}
- long traceId = currentSpan.getTraceId();
- Long mod = Math.abs(traceId % 100);
- return mod.compareTo(threshold) <= 0;
}
}
diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java
index 9d927a42c..0ebb47a6d 100644
--- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java
+++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java
@@ -37,7 +37,6 @@ import org.springframework.cloud.netflix.ribbon.RibbonClient;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.SpanReporter;
import org.springframework.cloud.sleuth.Tracer;
-import org.springframework.cloud.sleuth.assertions.SleuthAssertions;
import org.springframework.cloud.sleuth.util.ExceptionUtils;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -60,126 +59,118 @@ import static org.assertj.core.api.BDDAssertions.then;
/**
* Related to https://github.com/spring-cloud/spring-cloud-sleuth/issues/257
+ *
* @author ryarabori
*/
-@RunWith(SpringJUnit4ClassRunner.class)
-@SpringApplicationConfiguration(classes = { FeignClientServerErrorTests.TestConfiguration.class })
+@RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(
+ classes = { FeignClientServerErrorTests.TestConfiguration.class })
@WebIntegrationTest(value = { "spring.application.name=fooservice" }, randomPort = true)
public class FeignClientServerErrorTests {
- @Autowired TestFeignInterface feignInterface;
- @Rule public OutputCapture capture = new OutputCapture();
+ @Autowired TestFeignInterface feignInterface;
+ @Rule public OutputCapture capture = new OutputCapture();
- @Before
- public void setup() {
- ExceptionUtils.setFail(true);
- }
+ @Before public void setup() {
+ ExceptionUtils.setFail(true);
+ }
- @Test
- public void shouldCloseSpanOnInternalServerError() throws InterruptedException {
- try {
- this.feignInterface.internalError();
- } catch (HystrixRuntimeException e) {
- }
+ @Test public void shouldCloseSpanOnInternalServerError() throws InterruptedException {
+ try {
+ this.feignInterface.internalError();
+ }
+ catch (HystrixRuntimeException e) {
+ }
- // ugly :/ waiting for rx thread to complete
- Thread.sleep(100);
- then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span");
- }
+ // ugly :/ waiting for rx thread to complete
+ Thread.sleep(100);
+ then(this.capture.toString())
+ .doesNotContain("Tried to close span but it is not the current span");
+ }
- @Test
- public void shouldCloseSpanOnNotFound() throws InterruptedException {
- try {
- this.feignInterface.notFound();
- } catch (HystrixRuntimeException e) {
- }
+ @Test public void shouldCloseSpanOnNotFound() throws InterruptedException {
+ try {
+ this.feignInterface.notFound();
+ }
+ catch (HystrixRuntimeException e) {
+ }
- // ugly :/ waiting for rx thread to complete
- Thread.sleep(100);
- then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span");
- }
+ // ugly :/ waiting for rx thread to complete
+ Thread.sleep(100);
+ then(this.capture.toString())
+ .doesNotContain("Tried to close span but it is not the current span");
+ }
- @Configuration
- @EnableAutoConfiguration
- @EnableFeignClients
- @RibbonClient(value = "fooservice", configuration = SimpleRibbonClientConfiguration.class)
- public static class TestConfiguration {
+ @Configuration @EnableAutoConfiguration @EnableFeignClients
+ @RibbonClient(value = "fooservice",
+ configuration = SimpleRibbonClientConfiguration.class)
+ public static class TestConfiguration {
- @Bean
- FooController fooController() {
- return new FooController();
- }
+ @Bean FooController fooController() {
+ return new FooController();
+ }
- @Bean
- Listener listener() {
- return new Listener();
- }
+ @Bean Listener listener() {
+ return new Listener();
+ }
- @LoadBalanced
- @Bean
- public RestTemplate restTemplate() {
- return new RestTemplate();
- }
+ @LoadBalanced @Bean public RestTemplate restTemplate() {
+ return new RestTemplate();
+ }
- }
+ }
- @FeignClient(value = "fooservice")
- public interface TestFeignInterface {
+ @FeignClient(value = "fooservice") public interface TestFeignInterface {
- @RequestMapping(method = RequestMethod.GET, value = "/internalerror")
- ResponseEntity