Skip to content

Commit

Permalink
Fix #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 everything in milliseconds instead. This should be a non-breaking change as all the nanoseconds related logic isn’t part of the public API.
  • Loading branch information
felixscheinost committed Jun 29, 2022
1 parent 7c23e02 commit 6051652
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
/** Remote sampler that gets sampling configuration from AWS X-Ray. */
public final class AwsXrayRemoteSampler implements Sampler, Closeable {

static final long DEFAULT_TARGET_INTERVAL_NANOS = TimeUnit.SECONDS.toNanos(10);
static final long DEFAULT_TARGET_INTERVAL_MILLIS = TimeUnit.SECONDS.toMillis(10);

private static final Random RANDOM = new Random();
private static final Logger logger = Logger.getLogger(AwsXrayRemoteSampler.class.getName());
Expand All @@ -48,8 +48,8 @@ public final class AwsXrayRemoteSampler implements Sampler, Closeable {
private final ScheduledExecutorService executor;
// 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 pollingIntervalMillis;
private final int jitterMillis;

@Nullable private volatile ScheduledFuture<?> pollFuture;
@Nullable private volatile ScheduledFuture<?> fetchTargetsFuture;
Expand All @@ -71,7 +71,7 @@ public static AwsXrayRemoteSamplerBuilder newBuilder(Resource resource) {
Clock clock,
String endpoint,
Sampler initialSampler,
long pollingIntervalNanos) {
long pollingIntervalMillis) {
this.resource = resource;
this.clock = clock;
this.initialSampler = initialSampler;
Expand All @@ -93,9 +93,10 @@ public static AwsXrayRemoteSamplerBuilder newBuilder(Resource resource) {

sampler = initialSampler;

this.pollingIntervalNanos = pollingIntervalNanos;
this.pollingIntervalMillis = pollingIntervalMillis;
// Add ~1% of jitter. Truncating to int is safe for any practical polling interval.
jitterNanos = (int) (pollingIntervalNanos / 100);
// Math.max: Account for polling intervals < 100ms. Use at least 1 ms of jitter.
jitterMillis = Math.max(1, (int) (pollingIntervalMillis / 100));

// Execute first update right away on the executor thread.
executor.execute(this::getAndUpdateSampler);
Expand Down Expand Up @@ -139,7 +140,7 @@ private void getAndUpdateSampler() {
}
fetchTargetsFuture =
executor.schedule(
this::fetchTargets, DEFAULT_TARGET_INTERVAL_NANOS, TimeUnit.NANOSECONDS);
this::fetchTargets, DEFAULT_TARGET_INTERVAL_MILLIS, TimeUnit.MILLISECONDS);
}
} catch (Throwable t) {
logger.log(Level.FINE, "Failed to update sampler", t);
Expand All @@ -148,8 +149,8 @@ private void getAndUpdateSampler() {
}

private void scheduleSamplerUpdate() {
long delay = pollingIntervalNanos + RANDOM.nextInt(jitterNanos);
pollFuture = executor.schedule(this::getAndUpdateSampler, delay, TimeUnit.NANOSECONDS);
long delay = pollingIntervalMillis + RANDOM.nextInt(jitterMillis);
pollFuture = executor.schedule(this::getAndUpdateSampler, delay, TimeUnit.MILLISECONDS);
}

private void fetchTargets() {
Expand Down Expand Up @@ -177,7 +178,7 @@ private void fetchTargets() {
// Might be a transient API failure, try again after a default interval.
fetchTargetsFuture =
executor.schedule(
this::fetchTargets, DEFAULT_TARGET_INTERVAL_NANOS, TimeUnit.NANOSECONDS);
this::fetchTargets, DEFAULT_TARGET_INTERVAL_MILLIS, TimeUnit.MILLISECONDS);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public final class AwsXrayRemoteSamplerBuilder {
private Clock clock = Clock.getDefault();
private String endpoint = DEFAULT_ENDPOINT;
@Nullable private Sampler initialSampler;
private long pollingIntervalNanos = TimeUnit.SECONDS.toNanos(DEFAULT_POLLING_INTERVAL_SECS);
private long pollingIntervalMillis = TimeUnit.SECONDS.toMillis(DEFAULT_POLLING_INTERVAL_SECS);

AwsXrayRemoteSamplerBuilder(Resource resource) {
this.resource = resource;
Expand All @@ -48,7 +48,7 @@ public AwsXrayRemoteSamplerBuilder setEndpoint(String endpoint) {
*/
public AwsXrayRemoteSamplerBuilder setPollingInterval(Duration delay) {
requireNonNull(delay, "delay");
return setPollingInterval(delay.toNanos(), TimeUnit.NANOSECONDS);
return setPollingInterval(delay.toMillis(), TimeUnit.MILLISECONDS);
}

/**
Expand All @@ -60,7 +60,7 @@ public AwsXrayRemoteSamplerBuilder setPollingInterval(long delay, TimeUnit unit)
if (delay < 0) {
throw new IllegalArgumentException("delay must be non-negative");
}
pollingIntervalNanos = unit.toNanos(delay);
pollingIntervalMillis = unit.toMillis(delay);
return this;
}

Expand Down Expand Up @@ -94,6 +94,6 @@ public AwsXrayRemoteSampler build() {
new RateLimitingSampler(1, clock), Sampler.traceIdRatioBased(0.05)));
}
return new AwsXrayRemoteSampler(
resource, clock, endpoint, initialSampler, pollingIntervalNanos);
resource, clock, endpoint, initialSampler, pollingIntervalMillis);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ SamplingRuleApplier withTarget(SamplingTargetDocument target, Date now) {
long intervalNanos =
target.getIntervalSecs() != null
? TimeUnit.SECONDS.toNanos(target.getIntervalSecs())
: AwsXrayRemoteSampler.DEFAULT_TARGET_INTERVAL_NANOS;
: TimeUnit.MILLISECONDS.toNanos(AwsXrayRemoteSampler.DEFAULT_TARGET_INTERVAL_MILLIS);
long newNextSnapshotTimeNanos = clock.nanoTime() + intervalNanos;

return new SamplingRuleApplier(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -108,15 +109,20 @@ long nextTargetFetchTimeNanos() {
.mapToLong(SamplingRuleApplier::getNextSnapshotTimeNanos)
.min()
// There is always at least one rule in practice so this should never be exercised.
.orElseGet(() -> clock.nanoTime() + AwsXrayRemoteSampler.DEFAULT_TARGET_INTERVAL_NANOS);
.orElseGet(
() ->
clock.nanoTime()
+ TimeUnit.MILLISECONDS.toNanos(
AwsXrayRemoteSampler.DEFAULT_TARGET_INTERVAL_MILLIS));
}

XrayRulesSampler withTargets(
Map<String, SamplingTargetDocument> ruleTargets,
Set<String> requestedTargetRuleNames,
Date now) {
long defaultNextSnapshotTimeNanos =
clock.nanoTime() + AwsXrayRemoteSampler.DEFAULT_TARGET_INTERVAL_NANOS;
clock.nanoTime()
+ TimeUnit.MILLISECONDS.toNanos(AwsXrayRemoteSampler.DEFAULT_TARGET_INTERVAL_MILLIS);
SamplingRuleApplier[] newAppliers =
Arrays.stream(ruleAppliers)
.map(
Expand Down

0 comments on commit 6051652

Please sign in to comment.