-
Notifications
You must be signed in to change notification settings - Fork 64
Added patch for the X-Ray Remote Sampler #1089
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
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.
We need testing showing that this patch can takes effect and is working as expected.
Also please be more verbose in exactly how you generated this patch in the overview.
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.
Quick thought: Do we need to do this in other languages?
I checked the other languages and we are up to date and we don't have an issue there. They're looking at both the old and new sem covs. |
*Description of changes:* Merges changes from mainline to v2.11.1 Namely: #1085 and #1089 ``` The following dependencies are using the latest release version: - com.sparkjava:spark-core:2.9.4 - com.squareup.okhttp3:okhttp:4.12.0 - io.opentelemetry:opentelemetry-extension-aws:1.20.1 The following dependencies have later release versions: - com.amazonaws:aws-java-sdk-bom [1.12.599 -> 1.12.785] https://aws.amazon.com/sdkforjava - com.fasterxml.jackson:jackson-bom [2.16.0 -> 2.19.0] https://github.com/FasterXML/jackson-bom - com.github.ben-manes.versions:com.github.ben-manes.versions.gradle.plugin [0.50.0 -> 0.52.0] - com.google.guava:guava-bom [33.0.0-jre -> 33.4.8-jre] https://github.com/google/guava - com.google.protobuf:protobuf-bom [3.25.1 -> 4.31.1] https://developers.google.com/protocol-buffers/ - com.linecorp.armeria:armeria-bom [1.26.4 -> 1.32.5] https://armeria.dev/ - commons-logging:commons-logging [1.2 -> 1.3.5] https://commons.apache.org/proper/commons-logging/ - io.grpc:grpc-bom [1.59.1 -> 1.73.0] https://github.com/grpc/grpc-java - io.opentelemetry.contrib:opentelemetry-aws-resources [1.39.0-alpha -> 1.46.0-alpha] https://github.com/open-telemetry/opentelemetry-java-contrib - io.opentelemetry.contrib:opentelemetry-aws-xray [1.39.0-adot1 -> 1.46.0] https://github.com/open-telemetry/opentelemetry-java-contrib - io.opentelemetry.instrumentation:opentelemetry-instrumentation-bom-alpha [2.11.0-adot2-alpha -> 2.16.0-alpha] https://github.com/open-telemetry/opentelemetry-java-instrumentation - io.opentelemetry.javaagent:opentelemetry-javaagent [2.11.0-adot2 -> 2.16.0] https://github.com/open-telemetry/opentelemetry-java-instrumentation - io.opentelemetry.proto:opentelemetry-proto [1.0.0-alpha -> 1.7.0-alpha] https://github.com/open-telemetry/opentelemetry-proto-java - net.bytebuddy:byte-buddy [1.14.10 -> 1.17.5] https://bytebuddy.net - org.apache.logging.log4j:log4j-bom [2.21.1 -> 2.24.3] https://logging.apache.org/log4j/2.x/ - org.assertj:assertj-core [3.24.2 -> 3.27.3] https://assertj.github.io/doc/#assertj-core - org.curioswitch.curiostack:protobuf-jackson [2.2.0 -> 2.7.0] https://github.com/curioswitch/protobuf-jackson - org.junit:junit-bom [5.10.1 -> 5.13.0] https://junit.org/junit5/ - org.slf4j:slf4j-api [1.7.36 -> 2.0.17] http://www.slf4j.org - org.slf4j:slf4j-simple [1.7.36 -> 2.0.17] http://www.slf4j.org - org.springframework.boot:spring-boot-dependencies [2.7.17 -> 3.5.0] https://spring.io/projects/spring-boot - org.testcontainers:testcontainers-bom [1.19.3 -> 1.21.1] https://java.testcontainers.org - software.amazon.awssdk:bom [2.21.33 -> 2.31.56] https://aws.amazon.com/sdkforjava Gradle release-candidate updates: - Gradle: [8.10 -> 8.14.1] ``` By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Jonathan Lee <107072447+jj22ee@users.noreply.github.com> Co-authored-by: Thomas Pierce <thp@amazon.com> Co-authored-by: Michael He <53622546+yiyuan-he@users.noreply.github.com> Co-authored-by: ADOT Patch workflow <adot-patch-workflow@github.com> Co-authored-by: Prashant Srivastava <50466688+srprash@users.noreply.github.com> Co-authored-by: Mohamed Asaker <asakem@amazon.com>
)" This reverts commit 5255564.
Description of changes:
Since upstream has updated the HTTP semantic conventions, the X-Ray Remote Sampler has been outdated as it's been looking at the older attributes that are no longer emitted. I've created a patch to temporarily fix this until the change is merged upstream.
To create the patch file:
git checkout v1.39.0
to checkout to the same version used by the latest release (2.11.0)SamplingRuleApplier.java
,SamplingRuleApplierTest.java
and theversion.gradle.kts
filesgit diff > opentelemetry-java-contrib.patch
. This should be a temporary patch. I'm creating another CR in upstream to fix this issue. Once it's fixed, we will need to update the io.opentelemetry.contrib:opentelemetry-aws-xray version to the latest one.Testing:
Testing Setup:
outgoing-http-call
and the/aws-sdk-call
APIsManual Testing:
For the manual testing. I created three sampling rules
/aws-sdk-call
with 100% sampling/outgoing-http-call
with 100% samplingBefore 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:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.