Skip to content

Commit

Permalink
Fix open-telemetry#376: AwsXrayRemoteSampler doesn’t poll for update
Browse files Browse the repository at this point in the history
In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing the jitter in a `long` as well.
  • Loading branch information
felixscheinost committed Jun 30, 2022
1 parent 7c23e02 commit fbf3c6e
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.opentelemetry.sdk.trace.samplers.Sampler;
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
import java.io.Closeable;
import java.time.Duration;
import java.time.Instant;
import java.util.Date;
import java.util.List;
Expand Down Expand Up @@ -49,7 +50,7 @@ public final class AwsXrayRemoteSampler implements Sampler, Closeable {
// Unique per-sampler client ID, generated as a random string.
private final String clientId;
private final long pollingIntervalNanos;
private final int jitterNanos;
private final long jitterNanos;

@Nullable private volatile ScheduledFuture<?> pollFuture;
@Nullable private volatile ScheduledFuture<?> fetchTargetsFuture;
Expand Down Expand Up @@ -94,8 +95,8 @@ public static AwsXrayRemoteSamplerBuilder newBuilder(Resource resource) {
sampler = initialSampler;

this.pollingIntervalNanos = pollingIntervalNanos;
// Add ~1% of jitter. Truncating to int is safe for any practical polling interval.
jitterNanos = (int) (pollingIntervalNanos / 100);
// Add ~1% of jitter
jitterNanos = pollingIntervalNanos / 100;

// Execute first update right away on the executor thread.
executor.execute(this::getAndUpdateSampler);
Expand Down Expand Up @@ -148,10 +149,23 @@ private void getAndUpdateSampler() {
}

private void scheduleSamplerUpdate() {
long delay = pollingIntervalNanos + RANDOM.nextInt(jitterNanos);
@SuppressWarnings("OptionalGetWithoutIsPresent") // getAsLong: RANDOM.longs should always provide a value, so we can do an unchecked unwrap here
long delay = pollingIntervalNanos + RANDOM.longs(0, jitterNanos).findFirst().getAsLong();
pollFuture = executor.schedule(this::getAndUpdateSampler, delay, TimeUnit.NANOSECONDS);
}

/**
* returns the duration until the next scheduled sampler update or null if no next update is scheduled yet
*/
@Nullable
public Duration getNextSamplerUpdateScheduledDuration() {
ScheduledFuture<?> pollFuture = this.pollFuture;
if (pollFuture == null) {
return null;
}
return Duration.ofNanos(pollFuture.getDelay(TimeUnit.NANOSECONDS));
}

private void fetchTargets() {
if (!(sampler instanceof XrayRulesSampler)) {
throw new IllegalStateException("Programming bug.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,28 @@ void defaultInitialSampler() {
}
}

// https://github.com/open-telemetry/opentelemetry-java-contrib/issues/376
@Test
void testJitterTruncation() {
sampler.close();
sampler =
AwsXrayRemoteSampler.newBuilder(Resource.empty())
.setInitialSampler(Sampler.alwaysOn())
.setEndpoint(server.httpUri().toString())
.setPollingInterval(Duration.ofMinutes(5))
.build();

assertThat(sampler.getNextSamplerUpdateScheduledDuration())
.isNull();

await()
.untilAsserted(
() -> {
assertThat(sampler.getNextSamplerUpdateScheduledDuration())
.isCloseTo(Duration.ofMinutes(5), Duration.ofSeconds(10));
});
}

private static SamplingDecision doSample(Sampler sampler, String name) {
return sampler
.shouldSample(
Expand Down

0 comments on commit fbf3c6e

Please sign in to comment.