Skip to content

Commit

Permalink
Fix error in sample-rate minimum of BoundrySampler. (openzipkin#188)
Browse files Browse the repository at this point in the history
Force usage of float in float-comparison, 0.0001f >= 0.0001 evaluates to false.
Fix docs-error in Sampler to reflect change in default samplers made in Brave 3.6.1
  • Loading branch information
NegatioN authored and adriancole committed Jun 21, 2016
1 parent a7a33d2 commit 4c601d1
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public final class BoundarySampler extends Sampler {
public static Sampler create(float rate) {
if (rate == 0) return Sampler.NEVER_SAMPLE;
if (rate == 1.0) return ALWAYS_SAMPLE;
checkArgument(rate > 0.0001 && rate < 1, "rate should be between 0.0001 and 1: was %s", rate);
checkArgument(rate >= 0.0001f && rate < 1, "rate should be between 0.0001 and 1: was %s", rate);
final long boundary = (long) (rate * 10000); // safe cast as less <= 1
return new BoundarySampler(boundary);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public abstract class Sampler {
* <p>The sampler returned is good for low volumes of traffic (<100K requests), as it is precise.
* If you have high volumes of traffic, consider {@link BoundarySampler}.
*
* @param rate minimum sample rate is 0.0001, or 0.01% of traces
* @param rate minimum sample rate is 0.01, or 1% of traces
*/
public static Sampler create(float rate) {
return CountingSampler.create(rate);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.github.kristofa.brave;

import org.assertj.core.data.Percentage;
import org.junit.Test;

import static org.assertj.core.data.Percentage.withPercentage;

Expand All @@ -12,4 +13,9 @@ public class BoundarySamplerTest extends SamplerTest {
@Override Percentage expectedErrorRate() {
return withPercentage(10);
}

@Test
public void acceptsOneInTenThousandSampleRate() {
newSampler(0.0001f);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.github.kristofa.brave;

import org.assertj.core.data.Percentage;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import static org.assertj.core.data.Percentage.withPercentage;

Expand All @@ -12,4 +14,13 @@ public class CountingSamplerTest extends SamplerTest {
@Override Percentage expectedErrorRate() {
return withPercentage(0);
}


@Test
public void sampleRateMinimumOnePercent() throws Exception {
thrown.expect(IllegalArgumentException.class);
newSampler(0.0001f);
}


}

0 comments on commit 4c601d1

Please sign in to comment.