Skip to content

Commit 0a358cb

Browse files
Optimize GetSamplingTargets calls by removing empty statistics documents (#1298)
1 parent 071363e commit 0a358cb

2 files changed

Lines changed: 134 additions & 1 deletion

File tree

.github/patches/opentelemetry-java-contrib.patch

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,32 @@
1+
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
2+
index bfb2eee6..d9adedfe 100644
3+
--- a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSampler.java
4+
+++ b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSampler.java
5+
@@ -235,7 +235,8 @@ public final class AwsXrayRemoteSampler implements Sampler, Closeable {
6+
statisticsSnapshot.stream()
7+
.forEach(
8+
snapshot -> {
9+
- if (snapshot.getStatisticsDocument() != null) {
10+
+ if (snapshot.getStatisticsDocument() != null
11+
+ && snapshot.getStatisticsDocument().getRequestCount() > 0) {
12+
statistics.add(snapshot.getStatisticsDocument());
13+
}
14+
if (snapshot.getBoostStatisticsDocument() != null
15+
@@ -243,6 +244,12 @@ public final class AwsXrayRemoteSampler implements Sampler, Closeable {
16+
boostStatistics.add(snapshot.getBoostStatisticsDocument());
17+
}
18+
});
19+
+
20+
+ if (statistics.isEmpty() && boostStatistics.isEmpty()) {
21+
+ fetchTargetsFuture =
22+
+ executor.schedule(this::fetchTargets, DEFAULT_TARGET_INTERVAL_NANOS, NANOSECONDS);
23+
+ return;
24+
+ }
25+
Set<String> requestedTargetRuleNames =
26+
statistics.stream().map(SamplingStatisticsDocument::getRuleName).collect(toSet());
27+
128
diff --git a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XrayRulesSampler.java b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XrayRulesSampler.java
2-
index f2ab7c2d..7dcf8347 100644
29+
index f2ab7c2d..f0448176 100644
330
--- a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XrayRulesSampler.java
431
+++ b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XrayRulesSampler.java
532
@@ -263,7 +263,7 @@ final class XrayRulesSampler implements Sampler {
@@ -11,6 +38,98 @@ index f2ab7c2d..7dcf8347 100644
1138
matchedRule = applier;
1239
}
1340
}
41+
@@ -323,14 +323,8 @@ final class XrayRulesSampler implements Sampler {
42+
if (target != null) {
43+
return rule.withTarget(target, now, currentNanoTime);
44+
}
45+
- if (requestedTargetRuleNames.contains(rule.getRuleName())) {
46+
- // In practice X-Ray should return a target for any rule we requested but
47+
- // do a defensive check here in case. If we requested a target but got nothing
48+
- // back assume the default interval.
49+
- return rule.withNextSnapshotTimeNanos(defaultNextSnapshotTimeNanos);
50+
- }
51+
// Target not requested, will be updated in a future target fetch.
52+
- return rule;
53+
+ return rule.withNextSnapshotTimeNanos(defaultNextSnapshotTimeNanos);
54+
})
55+
.toArray(SamplingRuleApplier[]::new);
56+
return new XrayRulesSampler(
57+
diff --git a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XraySamplerClient.java b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XraySamplerClient.java
58+
index ef47ba72..f7b60eea 100644
59+
--- a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XraySamplerClient.java
60+
+++ b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XraySamplerClient.java
61+
@@ -51,7 +51,7 @@ final class XraySamplerClient {
62+
63+
private static final ObjectMapper OBJECT_MAPPER =
64+
new ObjectMapper()
65+
- .setDefaultPropertyInclusion(JsonInclude.Include.NON_EMPTY)
66+
+ .setDefaultPropertyInclusion(JsonInclude.Include.NON_NULL)
67+
// AWS APIs return timestamps as floats.
68+
.registerModule(
69+
new SimpleModule().addDeserializer(Date.class, new FloatDateDeserializer()))
70+
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
71+
index 7a75f377..8ce009c0 100644
72+
--- a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSamplerTest.java
73+
+++ b/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/AwsXrayRemoteSamplerTest.java
74+
@@ -33,6 +33,7 @@ import java.io.IOException;
75+
import java.io.UncheckedIOException;
76+
import java.time.Duration;
77+
import java.util.Collections;
78+
+import java.util.concurrent.atomic.AtomicInteger;
79+
import java.util.concurrent.atomic.AtomicReference;
80+
import org.junit.jupiter.api.AfterEach;
81+
import org.junit.jupiter.api.BeforeEach;
82+
@@ -69,6 +70,7 @@ class AwsXrayRemoteSamplerTest {
83+
84+
private static final AtomicReference<byte[]> rulesResponse = new AtomicReference<>();
85+
private static final AtomicReference<byte[]> targetsResponse = new AtomicReference<>();
86+
+ private static final AtomicInteger targetsRequestCount = new AtomicInteger(0);
87+
88+
private static final String TRACE_ID = TraceId.fromLongs(1, 2);
89+
90+
@@ -92,6 +94,7 @@ class AwsXrayRemoteSamplerTest {
91+
sb.service(
92+
"/SamplingTargets",
93+
(ctx, req) -> {
94+
+ targetsRequestCount.incrementAndGet();
95+
byte[] response = AwsXrayRemoteSamplerTest.targetsResponse.get();
96+
if (response == null) {
97+
// Error out until the test configures a response, the sampler will use the
98+
@@ -120,6 +123,7 @@ class AwsXrayRemoteSamplerTest {
99+
void tearDown() {
100+
sampler.close();
101+
rulesResponse.set(null);
102+
+ targetsRequestCount.set(0);
103+
}
104+
105+
@Test
106+
@@ -163,6 +167,26 @@ class AwsXrayRemoteSamplerTest {
107+
});
108+
}
109+
110+
+ @Test
111+
+ void doesNotCallGetSamplingTargetsWhenNoStatistics() {
112+
+ rulesResponse.set(RULE_RESPONSE_1);
113+
+ targetsResponse.set(TARGETS_RESPONSE);
114+
+
115+
+ await()
116+
+ .untilAsserted(
117+
+ () -> {
118+
+ assertThat(sampler.getDescription()).contains("XrayRulesSampler");
119+
+ });
120+
+
121+
+ await()
122+
+ .pollDelay(Duration.ofMillis(100))
123+
+ .atMost(Duration.ofMillis(200))
124+
+ .untilAsserted(
125+
+ () -> {
126+
+ assertThat(targetsRequestCount.get()).isEqualTo(0);
127+
+ });
128+
+ }
129+
+
130+
@Test
131+
void defaultInitialSampler() {
132+
try (AwsXrayRemoteSampler sampler = AwsXrayRemoteSampler.newBuilder(Resource.empty()).build()) {
14133
diff --git a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/XrayRulesSamplerTest.java b/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/XrayRulesSamplerTest.java
15134
index c8a8dead..be314c1c 100644
16135
--- a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/XrayRulesSamplerTest.java
@@ -83,6 +202,18 @@ index c8a8dead..be314c1c 100644
83202

84203
// Assert the trace ID cache is filled with appropriate data and is cleared after TTL passes
85204
assertThat(sampler.getTraceUsageCache().asMap().size()).isEqualTo(4);
205+
diff --git a/aws-xray/src/test/resources/get-sampling-targets-request.json b/aws-xray/src/test/resources/get-sampling-targets-request.json
206+
index a8ad4540..83e32229 100644
207+
--- a/aws-xray/src/test/resources/get-sampling-targets-request.json
208+
+++ b/aws-xray/src/test/resources/get-sampling-targets-request.json
209+
@@ -16,5 +16,6 @@
210+
"SampledCount":31,
211+
"BorrowCount":0
212+
}
213+
- ]
214+
+ ],
215+
+ "SamplingBoostStatisticsDocuments":[]
216+
}
86217
diff --git a/version.gradle.kts b/version.gradle.kts
87218
index 74967704..d08094f7 100644
88219
--- a/version.gradle.kts

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ If your change does not need a CHANGELOG entry, add the "skip changelog" label t
1313

1414
## Unreleased
1515

16+
- Optimize GetSamplingTargets calls by removing empty statistics documents
17+
([#1298](https://github.com/aws-observability/aws-otel-java-instrumentation/pull/1298))
1618
- Ugrade to OTel v2.23.0 and Contrib v1.52.0
1719
([#1292](https://github.com/aws-observability/aws-otel-java-instrumentation/pull/1292))
1820
- Adaptive Sampling: Ensure the highest priority sampling rule is matched

0 commit comments

Comments
 (0)