Skip to content

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

Merged
merged 5 commits into from
Jun 4, 2025
Merged

Added patch for the X-Ray Remote Sampler #1089

merged 5 commits into from
Jun 4, 2025

Conversation

AsakerMohd
Copy link
Contributor

@AsakerMohd AsakerMohd commented May 29, 2025

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:

  1. Cloned the opentelemetry-java-contrib repo
  2. Ran git checkout v1.39.0 to checkout to the same version used by the latest release (2.11.0)
  3. Made the changes in the SamplingRuleApplier.java, SamplingRuleApplierTest.java and the version.gradle.kts files
  4. Generated the patch file by running git 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:

  1. Created a local build of the ADOT Distro by running the following:
./gradlew clean
./scripts/local_patch.sh //applies the patches including the new contrib patch
./gradlew build //this builds everything under the adot repo including the sample apps. 
  1. Used the SpringBoot sample app which includes both the outgoing-http-call and the /aws-sdk-call APIs
  2. Started the AWS Adot Collector in a docker container to retrieve Sampling Rules and to export traces to X-Ray
  3. Started the Sample App using the following:
JAVA_TOOL_OPTIONS=" -javaagent:/Volumes/workplace/otel/aws-otel-java-instrumentation/otelagent/build/libs/aws-opentelemetry-agent-2.11.0-SNAPSHOT.jar" \
OTEL_METRICS_EXPORTER=none \
OTEL_LOGS_EXPORTER=none \
OTEL_AWS_APPLICATION_SIGNALS_ENABLED=true \
OTEL_AWS_APPLICATION_SIGNALS_EXPORTER_ENDPOINT=http://localhost:4316/v1/metrics \
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf \
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=http://localhost:4316/v1/traces \
OTEL_RESOURCE_ATTRIBUTES="service.name=$YOUR_SVC_NAME" \
java -jar /Volumes/workplace/otel/aws-otel-java-instrumentation/sample-apps/springboot/build/libs/springboot-2.11.0-SNAPSHOT.jar

Manual Testing:

For the manual testing. I created three sampling rules

  1. Default one with zero sampling
  2. Second one with path = /aws-sdk-call with 100% sampling
  3. Third one with path = /outgoing-http-call with 100% sampling

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:
Screenshot 2025-06-01 at 11 34 56 PM

After the change:
Screenshot 2025-06-01 at 11 29 54 PM

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

@AsakerMohd AsakerMohd requested a review from a team as a code owner May 29, 2025 23:37
Copy link
Contributor

@thpierce thpierce left a 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.

Copy link
Contributor

@thpierce thpierce left a 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?

@AsakerMohd
Copy link
Contributor Author

AsakerMohd commented Jun 2, 2025

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.

@AsakerMohd AsakerMohd merged commit 5255564 into main Jun 4, 2025
5 checks passed
@AsakerMohd AsakerMohd deleted the samplerFix branch June 4, 2025 19:30
This was referenced Jun 4, 2025
liustve added a commit that referenced this pull request Jun 5, 2025
*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>
lukeina2z added a commit to lukeina2z/aws-otel-java-instrumentation that referenced this pull request Aug 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants