Fix #376: AwsXrayRemoteSampler doesn’t poll for update (#377)

In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`. Switch to storing the jitter in a `long` as well.
open-telemetry · Jul 6, 2022 · dd4d335 · dd4d335
1 parent 82d0178
commit dd4d335
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 4 deletions.
diff --git a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSampler.java b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSampler.java
@@ -17,8 +17,10 @@
 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.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
@@ -49,7 +51,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 Iterator<Long> jitterNanos;
 
   @Nullable private volatile ScheduledFuture<?> pollFuture;
   @Nullable private volatile ScheduledFuture<?> fetchTargetsFuture;
@@ -94,8 +96,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 = RANDOM.longs(0, pollingIntervalNanos / 100).iterator();
 
     // Execute first update right away on the executor thread.
     executor.execute(this::getAndUpdateSampler);
@@ -148,10 +150,25 @@ private void getAndUpdateSampler() {
   }
 
   private void scheduleSamplerUpdate() {
-    long delay = pollingIntervalNanos + RANDOM.nextInt(jitterNanos);
+    long delay = pollingIntervalNanos + jitterNanos.next();
     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.
+   *
+   * <p>only used for testing.
+   */
+  @Nullable
+  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.");

diff --git a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSamplerTest.java b/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSamplerTest.java
@@ -168,6 +168,25 @@ void defaultInitialSampler() {
     }
   }
 
+  // https://github.com/open-telemetry/opentelemetry-java-contrib/issues/376
+  @Test
+  void testJitterTruncation() {
+    try (AwsXrayRemoteSampler samplerWithLongerPollingInterval =
+        AwsXrayRemoteSampler.newBuilder(Resource.empty())
+            .setInitialSampler(Sampler.alwaysOn())
+            .setEndpoint(server.httpUri().toString())
+            .setPollingInterval(Duration.ofMinutes(5))
+            .build()) {
+      assertThat(samplerWithLongerPollingInterval.getNextSamplerUpdateScheduledDuration()).isNull();
+      await()
+          .untilAsserted(
+              () -> {
+                assertThat(samplerWithLongerPollingInterval.getNextSamplerUpdateScheduledDuration())
+                    .isCloseTo(Duration.ofMinutes(5), Duration.ofSeconds(10));
+              });
+    }
+  }
+
   private static SamplingDecision doSample(Sampler sampler, String name) {
     return sampler
         .shouldSample(