-
Notifications
You must be signed in to change notification settings - Fork 159
[aws-xray] Update SamplerRulesApplier to recognize new HTTP/URL semconv. #1959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
httpTarget = (String) entry.getValue(); | ||
} else if (entry.getKey().equals(HTTP_URL)) { | ||
} else if (entry.getKey().equals(HTTP_URL) || entry.getKey().equals(UrlAttributes.URL_FULL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
server instrumentations don't fill url.full
, would that be an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should be fine. It's there incase URL_PATH isn't being populated and the idea here is to then extract the path from URL_FULL.
aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java
Outdated
Show resolved
Hide resolved
aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java
Outdated
Show resolved
Hide resolved
🔧 The result from spotlessApply was committed to the PR branch. |
aws-xray/src/main/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplier.java
Outdated
Show resolved
Hide resolved
🔧 The result from spotlessApply was committed to the PR branch. |
aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java
Outdated
Show resolved
Hide resolved
aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java
Outdated
Show resolved
Hide resolved
aws-xray/src/test/java/io/opentelemetry/contrib/awsxray/SamplingRuleApplierTest.java
Outdated
Show resolved
Hide resolved
🔧 The result from spotlessApply was committed to the PR branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i ran the test. and also, lgtm.
Description:
The HTTP/URL semantic conventions were updated making the X-Ray Sampler outdated since it only recognizes the older HTTP semantic conventions. I've updated the SamplingRuleApplier to look at both the old and new conventions for backwards compatibility.
Testing:
Updated unit tests and are now passing.
Manual Testing:
For the manual testing. I created three sampling rules in the X-Ray console
/aws-sdk-call
with 100% sampling/outgoing-http-call
with 100% samplingI used a sample app with two APIs:
Before the change, calling any of the above APIs wasn't being matched when it should. After the change, the rules were being matched and each rule showed the correct stats.
Before the change:

After the change:
