[aws-xray] Update SamplerRulesApplier to recognize new HTTP/URL semconv.

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

@@ -19,6 +19,9 @@
 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 @@ private 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;
 
-    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();
+    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()) {
       Matcher matcher = attributeMatchers.get(entry.getKey().getKey());
       if (matcher == null) {
         continue;

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

@@ -29,6 +29,8 @@
 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.UrlAttributes;
 import io.opentelemetry.semconv.incubating.CloudIncubatingAttributes;
 import java.io.IOException;
 import java.io.UncheckedIOException;
@@ -72,6 +74,15 @@ class ExactMatch {
             .put(AttributeKey.longKey("speed"), 10)
             .build();
 
+    private final Attributes newSemCovAttributes =
+        Attributes.builder()
+            .put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
+            .put(NET_HOST_NAME, "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 +131,21 @@ void matches() {
           .isTrue();
     }
 
+    @Test
+    void matchesURLFullNewSemCov() {
+      assertThat(applier.matches(newSemCovAttributes, resource)).isTrue();
+
+      // http.url 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 +166,15 @@ void methodNotMatch() {
       assertThat(applier.matches(attributes, resource)).isFalse();
     }
 
+    @Test
+    void methodNewSemCovNotMatch() {
+      Attributes attributes =
+          this.newSemCovAttributes.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 +212,36 @@ void pathNotMatch() {
       assertThat(applier.matches(attributes, resource)).isFalse();
     }
 
+    @Test
+    void pathNewSemCovNotMatch() {
+      Attributes attributes =
+          this.newSemCovAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-you")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          this.newSemCovAttributes.toBuilder()
+              .remove(UrlAttributes.URL_PATH)
+              .put(UrlAttributes.URL_FULL, "scheme://host:port/instrument-you")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          this.newSemCovAttributes.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.newSemCovAttributes.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 +300,15 @@ class WildcardMatch {
             .put(AttributeKey.longKey("speed"), 10)
             .build();
 
+    private final Attributes newSemCovAttributes =
+        Attributes.builder()
+            .put(HttpAttributes.HTTP_REQUEST_METHOD, "GET")
+            .put(NET_HOST_NAME, "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 +391,30 @@ void methodNotMatch() {
       assertThat(applier.matches(attributes, resource)).isFalse();
     }
 
+    @Test
+    void newSemCovMethodMatches() {
+      Attributes attributes =
+          this.newSemCovAttributes.toBuilder()
+              .put(HttpAttributes.HTTP_REQUEST_METHOD, "BADGETGOOD")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          newSemCovAttributes.toBuilder().put(HttpAttributes.HTTP_REQUEST_METHOD, "BADGET").build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+      attributes =
+          newSemCovAttributes.toBuilder().put(HttpAttributes.HTTP_REQUEST_METHOD, "GETGET").build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+    }
+
+    @Test
+    void newSemCovMethodNotMatch() {
+      Attributes attributes =
+          newSemCovAttributes.toBuilder().put(HttpAttributes.HTTP_REQUEST_METHOD, "POST").build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes = removeAttribute(newSemCovAttributes, HttpAttributes.HTTP_REQUEST_METHOD);
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
     @Test
     void hostMatches() {
       Attributes attributes =
@@ -368,6 +466,37 @@ void pathNotMatch() {
       assertThat(applier.matches(attributes, resource)).isFalse();
     }
 
+    @Test
+    void pathNewSemCovMatches() {
+      Attributes attributes =
+          newSemCovAttributes.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 =
+          newSemCovAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-meafoo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isTrue();
+    }
+
+    @Test
+    void pathNewSemCovNotMatch() {
+      Attributes attributes =
+          newSemCovAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "/instrument-mea?foo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes =
+          newSemCovAttributes.toBuilder()
+              .put(UrlAttributes.URL_PATH, "foo/instrument-meafoo=bar&cat=")
+              .build();
+      assertThat(applier.matches(attributes, resource)).isFalse();
+      attributes = removeAttribute(newSemCovAttributes, UrlAttributes.URL_PATH);
+      assertThat(applier.matches(attributes, resource)).isFalse();
+    }
+
     @Test
     void attributeMatches() {
       Attributes attributes =