[aws-xray] Update SamplerRulesApplier to recognize new HTTP/URL semconv. (#1959)

Co-authored-by: otelbot <197425009+otelbot@users.noreply.github.com>
2025-07-22 23:56:12 -07:00 · 2025-07-22 23:56:12 -07:00 · 28d67ea785
parent 1f5f0d25ec
commit 28d67ea785
2 changed files with 220 additions and 17 deletions
--- a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java
+++ b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java
@ -19,6 +19,9 @@ import io.opentelemetry.sdk.trace.data.LinkData;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
 import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
+import io.opentelemetry.semconv.HttpAttributes;
+import io.opentelemetry.semconv.ServerAttributes;
+import io.opentelemetry.semconv.UrlAttributes;
 import java.time.Duration;
 import java.util.Collections;
 import java.util.Date;
@ -57,6 +60,10 @@ final class SamplingRuleApplier {

  private static final Map<String, String> XRAY_CLOUD_PLATFORM;

+  // _OTHER request method:
+  // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/registry/attributes/http.md?plain=1#L96
+  private static final String _OTHER_REQUEST_METHOD = "_OTHER";
+
  static {
    Map<String, String> xrayCloudPlatform = new HashMap<>();
    xrayCloudPlatform.put(AWS_EC2, "AWS::EC2::Instance");
@ -175,25 +182,35 @@ final class SamplingRuleApplier {
  @SuppressWarnings("deprecation") // TODO
  boolean matches(Attributes attributes, Resource resource) {
    int matchedAttributes = 0;
-    String httpTarget = null;
-    String httpUrl = null;
-    String httpMethod = null;
-    String host = null;
+
+    String httpTarget = attributes.get(UrlAttributes.URL_PATH);
+    if (httpTarget == null) {
+      httpTarget = attributes.get(HTTP_TARGET);
+    }
+
+    String httpUrl = attributes.get(UrlAttributes.URL_FULL);
+    if (httpUrl == null) {
+      httpUrl = attributes.get(HTTP_URL);
+    }
+
+    String httpMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD);
+    if (httpMethod == null) {
+      httpMethod = attributes.get(HTTP_METHOD);
+    }
+
+    if (httpMethod != null && httpMethod.equals(_OTHER_REQUEST_METHOD)) {
+      httpMethod = attributes.get(HttpAttributes.HTTP_REQUEST_METHOD_ORIGINAL);
+    }
+
+    String host = attributes.get(ServerAttributes.SERVER_ADDRESS);
+    if (host == null) {
+      host = attributes.get(NET_HOST_NAME);
+      if (host == null) {
+        host = attributes.get(HTTP_HOST);
+      }
+    }

    for (Map.Entry<AttributeKey<?>, Object> entry : attributes.asMap().entrySet()) {
-      if (entry.getKey().equals(HTTP_TARGET)) {
-        httpTarget = (String) entry.getValue();
-      } else if (entry.getKey().equals(HTTP_URL)) {
-        httpUrl = (String) entry.getValue();
-      } else if (entry.getKey().equals(HTTP_METHOD)) {
-        httpMethod = (String) entry.getValue();
-      } else if (entry.getKey().equals(NET_HOST_NAME)) {
-        host = (String) entry.getValue();
-      } else if (entry.getKey().equals(HTTP_HOST)) {
-        // TODO (trask) remove support for deprecated http.host attribute
-        host = (String) entry.getValue();
-      }
-
      Matcher matcher = attributeMatchers.get(entry.getKey().getKey());
      if (matcher == null) {
        continue;
--- a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java
+++ b/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java
@ -29,6 +29,9 @@ import io.opentelemetry.sdk.resources.Resource;
 import io.opentelemetry.sdk.testing.time.TestClock;
 import io.opentelemetry.sdk.trace.samplers.SamplingDecision;
 import io.opentelemetry.sdk.trace.samplers.SamplingResult;
+import io.opentelemetry.semconv.HttpAttributes;
+import io.opentelemetry.semconv.ServerAttributes;
+import io.opentelemetry.semconv.UrlAttributes;
 import io.opentelemetry.semconv.incubating.CloudIncubatingAttributes;
 import java.io.IOException;
 import java.io.UncheckedIOException;
@ -72,6 +75,15 @@ class SamplingRuleApplierTest {
            .put(AttributeKey.longKey("speed"), 10)
            .build();

+    private final Attributes stableSemConvAttributes =
+        Attributes.builder()
+            .put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
+            .put(ServerAttributes.SERVER_ADDRESS, "opentelemetry.io")
+            .put(UrlAttributes.URL_PATH, "/instrument-me")
+            .put(AttributeKey.stringKey("animal"), "cat")
+            .put(AttributeKey.longKey("speed"), 10)
+            .build();
+
    // FixedRate set to 1.0 in rule and no reservoir
    @Test
    void fixedRateAlwaysSample() {
@ -120,6 +132,21 @@ class SamplingRuleApplierTest {
          .isTrue();
    }

+    @Test
+    void matchesURLFullStableSemConv() {
+      assertThat(applier.matches(stableSemConvAttributes, resource)).isTrue();
+
+      // url.full works too
+      assertThat(
+              applier.matches(
+                  attributes.toBuilder()
+                      .remove(HTTP_TARGET)
+                      .put(UrlAttributes.URL_FULL, "scheme://host:port/instrument-me")
+                      .build(),
+                  resource))
+          .isTrue();
+    }
+
    @Test
    void serviceNameNotMatch() {
      assertThat(
@ -140,6 +167,15 @@ class SamplingRuleApplierTest {
      assertThat(applier.matches(attributes, resource)).isFalse();
    }

+    @Test
+    void methodStableSemConvNotMatch() {
+      Attributes attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(HttpAttributes.HTTP_REQUEST_METHOD, "POST")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
    @Test
    void hostNotMatch() {
      // Replacing dot with character makes sure we're not accidentally treating dot as regex
@ -177,6 +213,36 @@ class SamplingRuleApplierTest {
      assertThat(applier.matches(attributes, resource)).isFalse();
    }

+    @Test
+    void pathStableSemConvNotMatch() {
+      Attributes attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-you")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .remove(UrlAttributes.URL_PATH)
+              .put(UrlAttributes.URL_FULL, "scheme://host:port/instrument-you")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .remove(UrlAttributes.URL_PATH)
+              .put(UrlAttributes.URL_FULL, "scheme://host:port")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+
+      // Correct path, but we ignore anyways since the URL is malformed per spec, scheme is always
+      // present.
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .remove(UrlAttributes.URL_PATH)
+              .put(UrlAttributes.URL_FULL, "host:port/instrument-me")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
    @Test
    void attributeNotMatch() {
      Attributes attributes =
@ -235,6 +301,15 @@ class SamplingRuleApplierTest {
            .put(AttributeKey.longKey("speed"), 10)
            .build();

+    private final Attributes stableSemConvAttributes =
+        Attributes.builder()
+            .put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
+            .put(ServerAttributes.SERVER_ADDRESS, "opentelemetry.io")
+            .put(UrlAttributes.URL_PATH, "/instrument-me?foo=bar&cat=meow")
+            .put(AttributeKey.stringKey("animal"), "cat")
+            .put(AttributeKey.longKey("speed"), 10)
+            .build();
+
    // FixedRate set to 0.0 in rule and no reservoir
    @Test
    void fixedRateNeverSample() {
@ -317,6 +392,36 @@ class SamplingRuleApplierTest {
      assertThat(applier.matches(attributes, resource)).isFalse();
    }

+    @Test
+    void stableSemConvMethodMatches() {
+      Attributes attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(HttpAttributes.HTTP_REQUEST_METHOD, "BADGETGOOD")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(HttpAttributes.HTTP_REQUEST_METHOD, "BADGET")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(HttpAttributes.HTTP_REQUEST_METHOD, "GETGET")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+    }
+
+    @Test
+    void stableSemConvMethodNotMatch() {
+      Attributes attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(HttpAttributes.HTTP_REQUEST_METHOD, "POST")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes = removeAttribute(stableSemConvAttributes, HttpAttributes.HTTP_REQUEST_METHOD);
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
    @Test
    void hostMatches() {
      Attributes attributes =
@ -345,6 +450,56 @@ class SamplingRuleApplierTest {
      assertThat(applier.matches(attributes, resource)).isFalse();
    }

+    @Test
+    void stableSemConvHostMatches() {
+      Attributes attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "alpha.opentelemetry.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "opfdnqtelemetry.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "opentglemetry.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "opentglemry.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "opentglemrz.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+    }
+
+    @Test
+    void stableSemConvHostNotMatch() {
+      Attributes attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "opentelemetryfio")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "opentgalemetry.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          this.stableSemConvAttributes.toBuilder()
+              .put(ServerAttributes.SERVER_ADDRESS, "alpha.oentelemetry.io")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes = removeAttribute(this.stableSemConvAttributes, ServerAttributes.SERVER_ADDRESS);
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
    @Test
    void pathMatches() {
      Attributes attributes =
@ -368,6 +523,37 @@ class SamplingRuleApplierTest {
      assertThat(applier.matches(attributes, resource)).isFalse();
    }

+    @Test
+    void pathStableSemConvMatches() {
+      Attributes attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-me?foo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      // Deceptive question mark, it's actually a wildcard :-)
+      attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-meafoo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+    }
+
+    @Test
+    void pathStableSemConvNotMatch() {
+      Attributes attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-mea?foo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          stableSemConvAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "foo/instrument-meafoo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes = removeAttribute(stableSemConvAttributes, UrlAttributes.URL_PATH);
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
    @Test
    void attributeMatches() {
      Attributes attributes =