Skip to content

Commit eb66327

Browse files
committed
Addressing code review suggestions.
1 parent 1d0ba27 commit eb66327

File tree

4 files changed

+43
-8
lines changed

4 files changed

+43
-8
lines changed

consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentAnyOf.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
import io.opentelemetry.context.Context;
1616
import io.opentelemetry.sdk.trace.data.LinkData;
1717
import java.util.List;
18+
import java.util.stream.Collectors;
19+
import java.util.stream.Stream;
20+
import javax.annotation.Nullable;
1821
import javax.annotation.concurrent.Immutable;
1922

2023
/**
@@ -41,7 +44,10 @@ final class ConsistentAnyOf extends ConsistentSampler {
4144

4245
this.delegates = delegates;
4346

44-
this.description = Stream.of(delegates).map(Object::toString).collect(Collectors.joining(",", "ConsistentAnyOf{", "}"));
47+
this.description =
48+
Stream.of(delegates)
49+
.map(Object::toString)
50+
.collect(Collectors.joining(",", "ConsistentAnyOf{", "}"));
4551
}
4652

4753
@Override

consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRateLimitingSampler.java

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,35 @@ public State(
157157
this.inverseAdaptationTimeNanos = NANOS_IN_SECONDS / adaptationTimeSeconds;
158158
this.targetSpansPerNanosecondLimit = NANOS_IN_SECONDS * targetSpansPerSecondLimit;
159159

160-
double t = 1.0 / (targetSpansPerSecondLimit * adaptationTimeSeconds);
161-
this.probabilitySmoothingFactor = t / (1.0 + t);
160+
this.probabilitySmoothingFactor =
161+
determineProbabilitySmoothingFactor(targetSpansPerSecondLimit, adaptationTimeSeconds);
162162

163163
this.state = new AtomicReference<>(new State(0, 0, nanoTimeSupplier.getAsLong(), 1.0));
164164
}
165165

166+
private static double determineProbabilitySmoothingFactor(
167+
double targetSpansPerSecondLimit, double adaptationTimeSeconds) {
168+
// The probability smoothing factor alpha will be the weight for the newly observed
169+
// probability P, while (1-alpha) will be the weight for the cumulative average probability
170+
// observed so far (newC = P * alpha + oldC * (1 - alpha)). Any smoothing factor
171+
// alpha from the interval (0.0, 1.0) is mathematically acceptable.
172+
// However, we'd like the weight associated with the newly observed data point to be inversely
173+
// proportional to the adaptation time (larger adaptation time will allow longer time for the
174+
// cumulative probability to stabilize) and inversely proportional to the order of magnitude of
175+
// the data points arriving within a given time unit (because with a lot of data points we can
176+
// afford to give a smaller weight to each single one). We do not know the true rate of Spans
177+
// coming in to get sampled, but we optimistically assume that the user knows what they are
178+
// doing and that the targetSpansPerSecondLimit will be of similar order of magnitude.
179+
180+
// First approximation of the probability smoothing factor alpha.
181+
double t = 1.0 / (targetSpansPerSecondLimit * adaptationTimeSeconds);
182+
183+
// We expect that t is a small number, but we have to make sure that alpha is smaller than 1.
184+
// Therefore we apply a "bending" transformation which almost preserves small values, but makes
185+
// sure that the result is within the expected interval.
186+
return t / (1.0 + t);
187+
}
188+
166189
private State updateState(State oldState, long currentNanoTime, double delegateProbability) {
167190
if (currentNanoTime <= oldState.lastNanoTime) {
168191
return new State(

consistent-sampling/src/main/java/io/opentelemetry/contrib/sampler/consistent56/ConsistentRuleBasedSampler.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import io.opentelemetry.context.Context;
1313
import io.opentelemetry.sdk.trace.data.LinkData;
1414
import java.util.List;
15+
import java.util.stream.Collectors;
16+
import java.util.stream.Stream;
1517
import javax.annotation.Nullable;
1618
import javax.annotation.concurrent.Immutable;
1719

@@ -28,11 +30,15 @@ final class ConsistentRuleBasedSampler extends ConsistentSampler {
2830

2931
private final String description;
3032

31-
ConsistentRuleBasedSampler(@Nullable SpanKind spanKindToMatch, @Nullable PredicatedSampler... samplers) {
33+
ConsistentRuleBasedSampler(
34+
@Nullable SpanKind spanKindToMatch, @Nullable PredicatedSampler... samplers) {
3235
this.spanKindToMatch = spanKindToMatch;
33-
this.samplers = (samplers != null)?samplers:new PredicatedSampler[0];
36+
this.samplers = (samplers != null) ? samplers : new PredicatedSampler[0];
3437

35-
this.description = Stream.of(samplers).collect(Collectors.joining(",", "ConsistentRuleBasedSampler{", "}"));
38+
this.description =
39+
Stream.of(samplers)
40+
.map((s) -> s.getSampler().getDescription())
41+
.collect(Collectors.joining(",", "ConsistentRuleBasedSampler{", "}"));
3642
}
3743

3844
@Override

consistent-sampling/src/test/java/io/opentelemetry/contrib/sampler/consistent56/CoinFlipSampler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import io.opentelemetry.context.Context;
1313
import io.opentelemetry.sdk.trace.data.LinkData;
1414
import java.util.List;
15-
import java.util.Random;
15+
import java.util.SplittableRandom;
1616
import javax.annotation.concurrent.Immutable;
1717

1818
/**
@@ -22,7 +22,7 @@
2222
@Immutable
2323
final class CoinFlipSampler extends ConsistentSampler {
2424

25-
private static final SplittableRandomrandom = new SplittableRandom(0x160a50a2073e17e6L);
25+
private static final SplittableRandom random = new SplittableRandom(0x160a50a2073e17e6L);
2626

2727
private final ComposableSampler samplerA;
2828
private final ComposableSampler samplerB;

0 commit comments

Comments
 (0)