Skip to content

Commit c72dada

Browse files
Adaptive Sampling: Ensure the highest priority sampling rule is matched (#1290)
### Changes This PR ensures that the matched rule in the adaptive sampling logic is the highest priority rule by checking if the rule is null before setting it. In essence, `matchedRule == null && ` was simply added. We do not `break` at this point as we also use the same loop to find the rule that was applied to the root service if the current span is generated by a downstream/dependent service. ### Testing - Manually tested against a sample application to generate statistics towards an account with multiple boost rules - Added a dummy sampling rule to the unit test for adaptive sampling that ensures the higher priority one is the one that has statistics incremented By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Thomas Pierce <thp@amazon.com>
1 parent 840aa44 commit c72dada

2 files changed

Lines changed: 33 additions & 9 deletions

File tree

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,7 +1064,7 @@ index 1d97c4ae..dd369f5f 100644
10641064
}
10651065
}
10661066
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
1067-
index 75977dc0..48bdeb0f 100644
1067+
index 75977dc0..944d4e45 100644
10681068
--- a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XrayRulesSampler.java
10691069
+++ b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/XrayRulesSampler.java
10701070
@@ -5,42 +5,81 @@
@@ -1321,7 +1321,7 @@ index 75977dc0..48bdeb0f 100644
13211321
+ ruleToReportTo = applier;
13221322
+ break;
13231323
+ }
1324-
+ if (applier.matches(spanData.getAttributes(), resource)) {
1324+
+ if (matchedRule == null && applier.matches(spanData.getAttributes(), resource)) {
13251325
+ matchedRule = applier;
13261326
+ }
13271327
+ }
@@ -2150,7 +2150,7 @@ index 920a5ffd..b7c21aa0 100644
21502150
try {
21512151
return OBJECT_MAPPER.readValue(
21522152
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
2153-
index 1ca8df34..14ebdbda 100644
2153+
index 1ca8df34..8e4993a7 100644
21542154
--- a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/XrayRulesSamplerTest.java
21552155
+++ b/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/XrayRulesSamplerTest.java
21562156
@@ -5,17 +5,28 @@
@@ -2394,7 +2394,7 @@ index 1ca8df34..14ebdbda 100644
23942394

23952395
// Minimum is batTarget, 5s from now
23962396
assertThat(sampler.nextTargetFetchTimeNanos())
2397-
@@ -169,6 +275,891 @@ class XrayRulesSamplerTest {
2397+
@@ -169,6 +275,913 @@ class XrayRulesSamplerTest {
23982398
assertThat(sampler.snapshot(Date.from(now))).hasSize(4);
23992399
}
24002400

@@ -2694,11 +2694,27 @@ index 1ca8df34..14ebdbda 100644
26942694
+ 0.0,
26952695
+ "*",
26962696
+ "*",
2697-
+ 4,
2697+
+ 2,
26982698
+ 0,
26992699
+ "*",
27002700
+ "*",
2701-
+ "default-rule",
2701+
+ "applied-boost-rule",
2702+
+ "*",
2703+
+ "*",
2704+
+ "*",
2705+
+ 1,
2706+
+ SamplingRateBoost.create(1, 300));
2707+
+ SamplingRule rule3 =
2708+
+ SamplingRule.create(
2709+
+ Collections.emptyMap(),
2710+
+ 0.0,
2711+
+ "*",
2712+
+ "*",
2713+
+ 3,
2714+
+ 0,
2715+
+ "*",
2716+
+ "*",
2717+
+ "lower-priority-boost-rule",
27022718
+ "*",
27032719
+ "*",
27042720
+ "*",
@@ -2726,7 +2742,7 @@ index 1ca8df34..14ebdbda 100644
27262742
+ Resource.getDefault(),
27272743
+ clock,
27282744
+ Sampler.alwaysOn(),
2729-
+ Arrays.asList(rule1, rule2),
2745+
+ Arrays.asList(rule1, rule2, rule3),
27302746
+ config);
27312747
+
27322748
+ Instant now = Instant.ofEpochSecond(0, clock.now());
@@ -2763,13 +2779,16 @@ index 1ca8df34..14ebdbda 100644
27632779
+ // Rules are ordered by priority, so cat-rule is first
27642780
+ assertThat(snapshot.get(0).getBoostStatisticsDocument().getTotalCount()).isEqualTo(0);
27652781
+ assertThat(snapshot.get(0).getBoostStatisticsDocument().getAnomalyCount()).isEqualTo(0);
2766-
+
27672782
+ assertThat(snapshot.get(0).getBoostStatisticsDocument().getSampledAnomalyCount()).isEqualTo(0);
2783+
+
27682784
+ assertThat(snapshot.get(1).getBoostStatisticsDocument().getTotalCount()).isEqualTo(3);
27692785
+ assertThat(snapshot.get(1).getBoostStatisticsDocument().getAnomalyCount()).isEqualTo(3);
2770-
+
27712786
+ assertThat(snapshot.get(1).getBoostStatisticsDocument().getSampledAnomalyCount()).isEqualTo(0);
27722787
+
2788+
+ assertThat(snapshot.get(2).getBoostStatisticsDocument().getTotalCount()).isEqualTo(0);
2789+
+ assertThat(snapshot.get(2).getBoostStatisticsDocument().getAnomalyCount()).isEqualTo(0);
2790+
+ assertThat(snapshot.get(2).getBoostStatisticsDocument().getSampledAnomalyCount()).isEqualTo(0);
2791+
+
27732792
+ // Mock trace coming from upstream service where it was sampled by cat-rule
27742793
+ when(spanDataMock.getTraceId()).thenReturn("TRACE_ID4");
27752794
+ when(readableSpanMock.getSpanContext())
@@ -2794,6 +2813,9 @@ index 1ca8df34..14ebdbda 100644
27942813
+ assertThat(snapshot.get(1).getBoostStatisticsDocument().getTotalCount()).isEqualTo(0);
27952814
+ assertThat(snapshot.get(1).getBoostStatisticsDocument().getAnomalyCount()).isEqualTo(0);
27962815
+ assertThat(snapshot.get(1).getBoostStatisticsDocument().getSampledAnomalyCount()).isEqualTo(0);
2816+
+ assertThat(snapshot.get(2).getBoostStatisticsDocument().getTotalCount()).isEqualTo(0);
2817+
+ assertThat(snapshot.get(2).getBoostStatisticsDocument().getAnomalyCount()).isEqualTo(0);
2818+
+ assertThat(snapshot.get(2).getBoostStatisticsDocument().getSampledAnomalyCount()).isEqualTo(0);
27972819
+
27982820
+ // Assert the trace ID cache is filled with appropriate data and is cleared after TTL passes
27992821
+ assertThat(sampler.getTraceUsageCache().asMap().size()).isEqualTo(4);

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+
- Adaptive Sampling: Ensure the highest priority sampling rule is matched
17+
([#1290](https://github.com/aws-observability/aws-otel-java-instrumentation/pull/1290))
1618
- Sign Lambda layer by AWS Signer
1719
([#1275](https://github.com/aws-observability/aws-otel-java-instrumentation/pull/1275))
1820
- Bump Netty version to 4.1.130 Final

0 commit comments

Comments
 (0)