From c3f9ab2e5f1e4a4d74e8a8383976906d5422b56c Mon Sep 17 00:00:00 2001
From: Felix Scheinost <fesc@symentis.com>
Date: Thu, 30 Jun 2022 11:29:06 +0200
Subject: [PATCH] =?UTF-8?q?Fix=20#376:=20`AwsXrayRemoteSampler`=20doesn?=
 =?UTF-8?q?=E2=80=99t=20poll=20for=20update?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In the default configuration `pollingIntervalNanos = 3 * 10^11` so `pollingIntervalMillis / 100 > Integer.MAX_VALUE`.

Switch to storing the jitter in a `long` as well.
---
 .../contrib/awsxray/AwsXrayRemoteSampler.java | 25 ++++++++++++++++---
 .../awsxray/AwsXrayRemoteSamplerTest.java     | 21 ++++++++++++++++
 2 files changed, 42 insertions(+), 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
index 0c2c26a6c..57c080395 100644
--- 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
index e5cc76fb4..41c889eb0 100644
--- 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,27 @@ 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(