diff --git a/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java b/aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java index 6387aa0d7..1d97c4aed 100644 --- 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.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 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 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, 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, Object> entry : attributes.asMap().entrySet()) { Matcher matcher = attributeMatchers.get(entry.getKey().getKey()); if (matcher == null) { continue; diff --git a/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java b/aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java index bc7bdd3e7..920a5ffd4 100644 --- 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.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 ExactMatch { .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 @@ void matches() { .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 @@ void methodNotMatch() { 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 @@ void pathNotMatch() { 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 WildcardMatch { .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 @@ void methodNotMatch() { 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 @@ void hostNotMatch() { 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 @@ void pathNotMatch() { 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 =