Skip to content

Commit 28d67ea

Browse files
AsakerMohdotelbot[bot]
AsakerMohd
and
otelbot[bot]
authored
[aws-xray] Update SamplerRulesApplier to recognize new HTTP/URL semconv. (open-telemetry#1959)
Co-authored-by: otelbot <[email protected]>
1 parent 1f5f0d2 commit 28d67ea

File tree

2 files changed

+219
-16
lines changed

2 files changed

+219
-16
lines changed

‎aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
import io.opentelemetry.sdk.trace.samplers.Sampler;
2020
import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
2121
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
22+
import io.opentelemetry.semconv.HttpAttributes;
23+
import io.opentelemetry.semconv.ServerAttributes;
24+
import io.opentelemetry.semconv.UrlAttributes;
2225
import java.time.Duration;
2326
import java.util.Collections;
2427
import java.util.Date;
@@ -57,6 +60,10 @@ final class SamplingRuleApplier {
5760

5861
private static final Map<String, String> XRAY_CLOUD_PLATFORM;
5962

63+
// _OTHER request method:
64+
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/registry/attributes/http.md?plain=1#L96
65+
private static final String _OTHER_REQUEST_METHOD = "_OTHER";
66+
6067
static {
6168
Map<String, String> xrayCloudPlatform = new HashMap<>();
6269
xrayCloudPlatform.put(AWS_EC2, "AWS::EC2::Instance");
@@ -175,25 +182,35 @@ private SamplingRuleApplier(
175182
@SuppressWarnings("deprecation") // TODO
176183
boolean matches(Attributes attributes, Resource resource) {
177184
int matchedAttributes = 0;
178-
String httpTarget = null;
179-
String httpUrl = null;
180-
String httpMethod = null;
181-
String host = null;
182185

183-
for (Map.Entry<AttributeKey<?>, Object> entry : attributes.asMap().entrySet()) {
184-
if (entry.getKey().equals(HTTP_TARGET)) {
185-
httpTarget = (String) entry.getValue();
186-
} else if (entry.getKey().equals(HTTP_URL)) {
187-
httpUrl = (String) entry.getValue();
188-
} else if (entry.getKey().equals(HTTP_METHOD)) {
189-
httpMethod = (String) entry.getValue();
190-
} else if (entry.getKey().equals(NET_HOST_NAME)) {
191-
host = (String) entry.getValue();
192-
} else if (entry.getKey().equals(HTTP_HOST)) {
193-
// TODO (trask) remove support for deprecated http.host attribute
194-
host = (String) entry.getValue();
186+
String httpTarget = attributes.get(UrlAttributes.URL_PATH);
187+
if (httpTarget == null) {
188+
httpTarget = attributes.get(HTTP_TARGET);
189+
}
190+
191+
String httpUrl = attributes.get(UrlAttributes.URL_FULL);
192+
if (httpUrl == null) {
193+
httpUrl = attributes.get(HTTP_URL);
194+
}
195+
196+
String httpMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD);
197+
if (httpMethod == null) {
198+
httpMethod = attributes.get(HTTP_METHOD);
199+
}
200+
201+
if (httpMethod != null && httpMethod.equals(_OTHER_REQUEST_METHOD)) {
202+
httpMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD_ORIGINAL);
203+
}
204+
205+
String host = attributes.get(ServerAttributes.SERVER_ADDRESS);
206+
if (host == null) {
207+
host = attributes.get(NET_HOST_NAME);
208+
if (host == null) {
209+
host = attributes.get(HTTP_HOST);
195210
}
211+
}
196212

213+
for (Map.Entry<AttributeKey<?>, Object> entry : attributes.asMap().entrySet()) {
197214
Matcher matcher = attributeMatchers.get(entry.getKey().getKey());
198215
if (matcher == null) {
199216
continue;

‎aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
import io.opentelemetry.sdk.testing.time.TestClock;
3030
import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
3131
import io.opentelemetry.sdk.trace.samplers.SamplingResult;
32+
import io.opentelemetry.semconv.HttpAttributes;
33+
import io.opentelemetry.semconv.ServerAttributes;
34+
import io.opentelemetry.semconv.UrlAttributes;
3235
import io.opentelemetry.semconv.incubating.CloudIncubatingAttributes;
3336
import java.io.IOException;
3437
import java.io.UncheckedIOException;
@@ -72,6 +75,15 @@ class ExactMatch {
7275
.put(AttributeKey.longKey("speed"), 10)
7376
.build();
7477

78+
private final Attributes stableSemConvAttributes =
79+
Attributes.builder()
80+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
81+
.put(ServerAttributes.SERVER_ADDRESS, "opentelemetry.io")
82+
.put(UrlAttributes.URL_PATH, "/instrument-me")
83+
.put(AttributeKey.stringKey("animal"), "cat")
84+
.put(AttributeKey.longKey("speed"), 10)
85+
.build();
86+
7587
// FixedRate set to 1.0 in rule and no reservoir
7688
@Test
7789
void fixedRateAlwaysSample() {
@@ -120,6 +132,21 @@ void matches() {
120132
.isTrue();
121133
}
122134

135+
@Test
136+
void matchesURLFullStableSemConv() {
137+
assertThat(applier.matches(stableSemConvAttributes, resource)).isTrue();
138+
139+
// url.full works too
140+
assertThat(
141+
applier.matches(
142+
attributes.toBuilder()
143+
.remove(HTTP_TARGET)
144+
.put(UrlAttributes.URL_FULL, "scheme://host:port/instrument-me")
145+
.build(),
146+
resource))
147+
.isTrue();
148+
}
149+
123150
@Test
124151
void serviceNameNotMatch() {
125152
assertThat(
@@ -140,6 +167,15 @@ void methodNotMatch() {
140167
assertThat(applier.matches(attributes, resource)).isFalse();
141168
}
142169

170+
@Test
171+
void methodStableSemConvNotMatch() {
172+
Attributes attributes =
173+
this.stableSemConvAttributes.toBuilder()
174+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "POST")
175+
.build();
176+
assertThat(applier.matches(attributes, resource)).isFalse();
177+
}
178+
143179
@Test
144180
void hostNotMatch() {
145181
// Replacing dot with character makes sure we're not accidentally treating dot as regex
@@ -177,6 +213,36 @@ void pathNotMatch() {
177213
assertThat(applier.matches(attributes, resource)).isFalse();
178214
}
179215

216+
@Test
217+
void pathStableSemConvNotMatch() {
218+
Attributes attributes =
219+
this.stableSemConvAttributes.toBuilder()
220+
.put(UrlAttributes.URL_PATH, "/instrument-you")
221+
.build();
222+
assertThat(applier.matches(attributes, resource)).isFalse();
223+
attributes =
224+
this.stableSemConvAttributes.toBuilder()
225+
.remove(UrlAttributes.URL_PATH)
226+
.put(UrlAttributes.URL_FULL, "scheme://host:port/instrument-you")
227+
.build();
228+
assertThat(applier.matches(attributes, resource)).isFalse();
229+
attributes =
230+
this.stableSemConvAttributes.toBuilder()
231+
.remove(UrlAttributes.URL_PATH)
232+
.put(UrlAttributes.URL_FULL, "scheme://host:port")
233+
.build();
234+
assertThat(applier.matches(attributes, resource)).isFalse();
235+
236+
// Correct path, but we ignore anyways since the URL is malformed per spec, scheme is always
237+
// present.
238+
attributes =
239+
this.stableSemConvAttributes.toBuilder()
240+
.remove(UrlAttributes.URL_PATH)
241+
.put(UrlAttributes.URL_FULL, "host:port/instrument-me")
242+
.build();
243+
assertThat(applier.matches(attributes, resource)).isFalse();
244+
}
245+
180246
@Test
181247
void attributeNotMatch() {
182248
Attributes attributes =
@@ -235,6 +301,15 @@ class WildcardMatch {
235301
.put(AttributeKey.longKey("speed"), 10)
236302
.build();
237303

304+
private final Attributes stableSemConvAttributes =
305+
Attributes.builder()
306+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
307+
.put(ServerAttributes.SERVER_ADDRESS, "opentelemetry.io")
308+
.put(UrlAttributes.URL_PATH, "/instrument-me?foo=bar&cat=meow")
309+
.put(AttributeKey.stringKey("animal"), "cat")
310+
.put(AttributeKey.longKey("speed"), 10)
311+
.build();
312+
238313
// FixedRate set to 0.0 in rule and no reservoir
239314
@Test
240315
void fixedRateNeverSample() {
@@ -317,6 +392,36 @@ void methodNotMatch() {
317392
assertThat(applier.matches(attributes, resource)).isFalse();
318393
}
319394

395+
@Test
396+
void stableSemConvMethodMatches() {
397+
Attributes attributes =
398+
this.stableSemConvAttributes.toBuilder()
399+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "BADGETGOOD")
400+
.build();
401+
assertThat(applier.matches(attributes, resource)).isTrue();
402+
attributes =
403+
stableSemConvAttributes.toBuilder()
404+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "BADGET")
405+
.build();
406+
assertThat(applier.matches(attributes, resource)).isTrue();
407+
attributes =
408+
stableSemConvAttributes.toBuilder()
409+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "GETGET")
410+
.build();
411+
assertThat(applier.matches(attributes, resource)).isTrue();
412+
}
413+
414+
@Test
415+
void stableSemConvMethodNotMatch() {
416+
Attributes attributes =
417+
stableSemConvAttributes.toBuilder()
418+
.put(HttpAttributes.HTTP_REQUEST_METHOD, "POST")
419+
.build();
420+
assertThat(applier.matches(attributes, resource)).isFalse();
421+
attributes = removeAttribute(stableSemConvAttributes, HttpAttributes.HTTP_REQUEST_METHOD);
422+
assertThat(applier.matches(attributes, resource)).isFalse();
423+
}
424+
320425
@Test
321426
void hostMatches() {
322427
Attributes attributes =
@@ -345,6 +450,56 @@ void hostNotMatch() {
345450
assertThat(applier.matches(attributes, resource)).isFalse();
346451
}
347452

453+
@Test
454+
void stableSemConvHostMatches() {
455+
Attributes attributes =
456+
this.stableSemConvAttributes.toBuilder()
457+
.put(ServerAttributes.SERVER_ADDRESS, "alpha.opentelemetry.io")
458+
.build();
459+
assertThat(applier.matches(attributes, resource)).isTrue();
460+
attributes =
461+
this.stableSemConvAttributes.toBuilder()
462+
.put(ServerAttributes.SERVER_ADDRESS, "opfdnqtelemetry.io")
463+
.build();
464+
assertThat(applier.matches(attributes, resource)).isTrue();
465+
attributes =
466+
this.stableSemConvAttributes.toBuilder()
467+
.put(ServerAttributes.SERVER_ADDRESS, "opentglemetry.io")
468+
.build();
469+
assertThat(applier.matches(attributes, resource)).isTrue();
470+
attributes =
471+
this.stableSemConvAttributes.toBuilder()
472+
.put(ServerAttributes.SERVER_ADDRESS, "opentglemry.io")
473+
.build();
474+
assertThat(applier.matches(attributes, resource)).isTrue();
475+
attributes =
476+
this.stableSemConvAttributes.toBuilder()
477+
.put(ServerAttributes.SERVER_ADDRESS, "opentglemrz.io")
478+
.build();
479+
assertThat(applier.matches(attributes, resource)).isTrue();
480+
}
481+
482+
@Test
483+
void stableSemConvHostNotMatch() {
484+
Attributes attributes =
485+
this.stableSemConvAttributes.toBuilder()
486+
.put(ServerAttributes.SERVER_ADDRESS, "opentelemetryfio")
487+
.build();
488+
assertThat(applier.matches(attributes, resource)).isFalse();
489+
attributes =
490+
this.stableSemConvAttributes.toBuilder()
491+
.put(ServerAttributes.SERVER_ADDRESS, "opentgalemetry.io")
492+
.build();
493+
assertThat(applier.matches(attributes, resource)).isFalse();
494+
attributes =
495+
this.stableSemConvAttributes.toBuilder()
496+
.put(ServerAttributes.SERVER_ADDRESS, "alpha.oentelemetry.io")
497+
.build();
498+
assertThat(applier.matches(attributes, resource)).isFalse();
499+
attributes = removeAttribute(this.stableSemConvAttributes, ServerAttributes.SERVER_ADDRESS);
500+
assertThat(applier.matches(attributes, resource)).isFalse();
501+
}
502+
348503
@Test
349504
void pathMatches() {
350505
Attributes attributes =
@@ -368,6 +523,37 @@ void pathNotMatch() {
368523
assertThat(applier.matches(attributes, resource)).isFalse();
369524
}
370525

526+
@Test
527+
void pathStableSemConvMatches() {
528+
Attributes attributes =
529+
stableSemConvAttributes.toBuilder()
530+
.put(UrlAttributes.URL_PATH, "/instrument-me?foo=bar&cat=")
531+
.build();
532+
assertThat(applier.matches(attributes, resource)).isTrue();
533+
// Deceptive question mark, it's actually a wildcard :-)
534+
attributes =
535+
stableSemConvAttributes.toBuilder()
536+
.put(UrlAttributes.URL_PATH, "/instrument-meafoo=bar&cat=")
537+
.build();
538+
assertThat(applier.matches(attributes, resource)).isTrue();
539+
}
540+
541+
@Test
542+
void pathStableSemConvNotMatch() {
543+
Attributes attributes =
544+
stableSemConvAttributes.toBuilder()
545+
.put(UrlAttributes.URL_PATH, "/instrument-mea?foo=bar&cat=")
546+
.build();
547+
assertThat(applier.matches(attributes, resource)).isFalse();
548+
attributes =
549+
stableSemConvAttributes.toBuilder()
550+
.put(UrlAttributes.URL_PATH, "foo/instrument-meafoo=bar&cat=")
551+
.build();
552+
assertThat(applier.matches(attributes, resource)).isFalse();
553+
attributes = removeAttribute(stableSemConvAttributes, UrlAttributes.URL_PATH);
554+
assertThat(applier.matches(attributes, resource)).isFalse();
555+
}
556+
371557
@Test
372558
void attributeMatches() {
373559
Attributes attributes =

0 commit comments

Comments
 (0)