Skip to content

Commit

Permalink
Onboarding ADOT Java to v2.x - with otel v2.10.0 (#975)
Browse files Browse the repository at this point in the history
*Description of changes:*
Onboarding ADOT Java to v2.x, with the latest
`opentelemetry-java-instrumentation` dependency version `v2.10.0`.
Update the workflows, adot core implementation codes, contract tests and
smoke tests with following change:
**A.  Workflows:**
1. Update `.github/patches/opentelemetry-java-instrumentation.patch`
file with the same code change as previous patch based on upstream
v2.10.0.
2. Upgrade contract testing build env with JDK 23.

**B. Core repo:**
1. Upgrade `com.github.johnrengelman.shadow` to `com.gradleup.shadow` as
it has been marked under maintenance mode:
https://github.com/GradleUp/shadow?tab=readme-ov-file#gradle-shadow
2. Upgrade to new ktlint version 1.4.0, and apply latest spotless
format.
3. Upgrade gradle to 8.10
4. Upgrade `com.diffplug.spotless` to newer version 6.25.0
5. Upgrade `com.google.cloud.tools.jib` to newer version 3.4.4
6. Update the unit test to check the migrated semantic conventions.
7. AWS Resource detector is [set as disabled by
default](open-telemetry/opentelemetry-java-instrumentation#10754),
enable it by setting environment variable
`"otel.resource.providers.aws.enabled"` to be `true`.

**C. Contract tests:**
1. Use the latest `kotlin("jvm")` version to be compatible if the new
Java class from upstream.
2. Add `OTEL_EXPORTER_OTLP_PROTOCOL` to `grpc` for instrumentation as in
[version
v2.0.0](https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v2.0.0):
> The default OTLP protocol has been changed from grpc to http/protobuf
in orderto align with the specification.
3. Update tomcat contract test `TOMCAT_THREADS` metric threshold to
`-2`, as the value can be negative by [tomcat
design](https://github.com/apache/tomcat/blob/1afe41491f0e56ec0a776db5ff84607f87ce6640/java/org/apache/tomcat/util/net/AbstractEndpoint.java#L1204).
4. Update the test classes to migrate to the new semantic conventions to
match with [upstream latest
change](open-telemetry/opentelemetry-java-instrumentation@7cd705b):
a. All the change made in
#972:
      `http.url` -> `url.full`
      `http.method` -> `http.request.method`
      `http.status_code` -> `http.response.status_code`
      `net.peer.name` -> `server.address`
      `net.peer.port` -> `server.port`
   b. extra semantic conventions included in contract tests:
      `http.scheme` -> `url.scheme`
      `http.target` -> `url.query`
      `http.url` -> `url.full`
      `net.host.name` -> `server.address`
      `net.host.port` -> `server.port`
      `net.peer.name` -> `server.address`
      `net.peer.port` -> `server.port`
      `net.protocol.name` -> `network.protocol.name`
      `net.protocol.version` -> `network.protocol.version`
      `net.sock.host.addr` -> `network.local.address`
      `net.sock.host.port` -> `network.local.port`
      `net.sock.peer.addr` -> `network.peer.address`
      `net.sock.peer.name` -> no replacement, removed
`messaging.kafka.destination.partition` ->
`messaging.destination.partition.id`
`messaging.message.payload_size_bytes` -> `messaging.message.body.size`
Remove `network.protocol.name` attribute check as it has marked as
Conditionally required if not http and network.protocol.version is set:
https://opentelemetry.io/blog/2023/http-conventions-declared-stable/#summary-of-changes.
Conditionally check `peer.service` for http client. Match with PR:
https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/12083/files
Remove local socket attributes from http server span check as it is not
extracted from HttpServerAttributesExtractor
[code](https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/5ebb81b8a8ac0e5b3c9f2e175b847a3d0b12251f/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpServerAttributesExtractorBuilder.java#L141),
Remove `http.response.header.content_length` as it need an explicit
configuration:
https://opentelemetry.io/docs/specs/semconv/attributes-registry/http/#http-response-header

**D. Smoke tests**
Enable controller telemetry using env variable
`OTEL_INSTRUMENTATION_COMMON_EXPERIMENTAL_CONTROLLER_TELEMETRY_ENABLED`,
which is disabled by default.

https://opentelemetry.io/docs/zero-code/java/agent/disable/#suppressing-controller-andor-view-spans


**Testing:**
1. All contract tests and smoke tests passed.
2. All workflow passed.
3. Tested all E2E tests (This E2E test PR need to be merged after ADOT
Java v2.x released):
- EC2:
https://github.com/aws-observability/aws-application-signals-test-framework/actions/runs/12378967612
- EKS:
https://github.com/aws-observability/aws-application-signals-test-framework/actions/runs/12382416662
- K8S:
https://github.com/aws-observability/aws-application-signals-test-framework/actions/runs/12378972156
- ECS:
https://github.com/aws-observability/aws-application-signals-test-framework/actions/runs/12378968843

4. Audit all upstream v2.0.0 - v2.10.0 [change
logs](https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases)
and manual did E2E test and compared the traces, logs, metrics for Java
v1.x vs Java v2.x:
 
![Screenshot 2024-12-16 at 3 27
02 PM](https://github.com/user-attachments/assets/52bb9106-b4a1-4223-9df5-74f1f1b10089)


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
  • Loading branch information
zzhlogin authored Dec 19, 2024
1 parent 448d680 commit f604e22
Show file tree
Hide file tree
Showing 63 changed files with 2,229 additions and 690 deletions.
2,250 changes: 1,818 additions & 432 deletions .github/patches/opentelemetry-java-instrumentation.patch

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion .github/patches/versions
Original file line number Diff line number Diff line change
@@ -1 +1 @@
OTEL_JAVA_INSTRUMENTATION_VERSION=v1.33.6
OTEL_JAVA_INSTRUMENTATION_VERSION=v2.10.0
13 changes: 13 additions & 0 deletions .github/workflows/docker-build-smoke-tests-fake-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@ jobs:
with:
java-version: 17
distribution: 'temurin'
# cache local patch outputs
- name: Cache local Maven repository
uses: actions/cache@v3
with:
path: |
~/.m2/repository/io/opentelemetry/
key: ${{ runner.os }}-maven-local-${{ hashFiles('.github/patches/opentelemetry-java*.patch') }}

- name: Publish patched dependencies to maven local
uses: ./.github/actions/patch-dependencies
with:
gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }}
gpg_password: ${{ secrets.GPG_PASSPHRASE }}
- uses: gradle/wrapper-validation-action@v1
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v4
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/main-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ jobs:
fetch-depth: 0
- uses: actions/setup-java@v4
with:
java-version: 21
java-version: 23
distribution: 'temurin'
- uses: gradle/wrapper-validation-action@v1

Expand All @@ -219,7 +219,7 @@ jobs:
key: ${{ runner.os }}-maven-local-${{ hashFiles('.github/patches/opentelemetry-java*.patch') }}

- name: Pull base image of Contract Tests Sample Apps
run: docker pull public.ecr.aws/docker/library/amazoncorretto:21-alpine
run: docker pull public.ecr.aws/docker/library/amazoncorretto:23-alpine

- name: Build snapshot with Gradle
uses: gradle/gradle-build-action@v3
Expand Down
18 changes: 16 additions & 2 deletions .github/workflows/nightly-upstream-snapshot-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ jobs:
java-version: 17
distribution: 'temurin'

# cache local patch outputs
- name: Cache local Maven repository
uses: actions/cache@v3
with:
path: |
~/.m2/repository/io/opentelemetry/
key: ${{ runner.os }}-maven-local-${{ hashFiles('.github/patches/opentelemetry-java*.patch') }}

- name: Publish patched dependencies to maven local
uses: ./.github/actions/patch-dependencies
with:
gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }}
gpg_password: ${{ secrets.GPG_PASSPHRASE }}

- uses: gradle/wrapper-validation-action@v1

- name: Configure AWS Credentials
Expand Down Expand Up @@ -120,7 +134,7 @@ jobs:
fetch-depth: 0
- uses: actions/setup-java@v4
with:
java-version: 21
java-version: 23
distribution: 'temurin'
- uses: gradle/wrapper-validation-action@v1

Expand All @@ -136,7 +150,7 @@ jobs:
registry: public.ecr.aws

- name: Pull base image of Contract Tests Sample Apps
run: docker pull public.ecr.aws/docker/library/amazoncorretto:21-alpine
run: docker pull public.ecr.aws/docker/library/amazoncorretto:23-alpine

- name: Build snapshot with Gradle
uses: gradle/gradle-build-action@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/patch-release-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:

env:
AWS_DEFAULT_REGION: us-east-1
TEST_TAG: public.ecr.aws/aws-observability/adot-autoinstrumentation-java:test
TEST_TAG: public.ecr.aws/aws-observability/adot-autoinstrumentation-java:test-v2

permissions:
id-token: write
Expand Down
13 changes: 10 additions & 3 deletions .github/workflows/pr-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:
- main
- "release/v*"
env:
TEST_TAG: public.ecr.aws/aws-observability/adot-autoinstrumentation-java:test
TEST_TAG: public.ecr.aws/aws-observability/adot-autoinstrumentation-java:test-v2

jobs:
testpatch:
Expand Down Expand Up @@ -63,6 +63,13 @@ jobs:

- uses: gradle/wrapper-validation-action@v1

# Cleanup directories before proceeding with setup
- name: Clean up old installations
if: ${{ matrix.os != 'windows-latest' }}
run: |
sudo rm -rf /usr/local/lib/android
sudo rm -rf /usr/share/dotnet
# cache local patch outputs
- name: Cache local Maven repository
uses: actions/cache@v3
Expand All @@ -84,12 +91,12 @@ jobs:
- name: Set up Java version for tests
uses: actions/setup-java@v4
with:
java-version: 21
java-version: 23
distribution: temurin

- name: Pull base image of Contract Tests Sample Apps
if: ${{ matrix.os == 'ubuntu-latest' }}
run: docker pull public.ecr.aws/docker/library/amazoncorretto:21-alpine
run: docker pull public.ecr.aws/docker/library/amazoncorretto:23-alpine

- name: Run contract tests
uses: gradle/gradle-build-action@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
env:
AWS_PUBLIC_ECR_REGION: us-east-1
AWS_PRIVATE_ECR_REGION: us-west-2
TEST_TAG: public.ecr.aws/aws-observability/adot-autoinstrumentation-java:test
TEST_TAG: public.ecr.aws/aws-observability/adot-autoinstrumentation-java:test-v2
PUBLIC_REPOSITORY: public.ecr.aws/aws-observability/adot-autoinstrumentation-java
PRIVATE_REPOSITORY: 020628701572.dkr.ecr.us-west-2.amazonaws.com/adot-autoinstrumentation-java
PRIVATE_REGISTRY: 020628701572.dkr.ecr.us-west-2.amazonaws.com
Expand Down
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ can be exported in a variety of formats. In addition, the agent and exporter can
command line arguments or environment variables. The net result is the ability to gather telemetry
data from a Java application without any code changes.

Note: There are 2.x releases and 1.x releases. The 2.0 release included significant breaking changes from [OpenTelemetry Agent for Java](https://github.com/open-telemetry/opentelemetry-java-instrumentation),
the details of which can be found in the [release notes](https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases).
It is recommended to use the latest 2.x release which will have the latest features and improvements.
1.x will receive security patches for a limited time and will not include other bug fixes and enhancements.

## Getting Started

Check out the [getting started documentation](https://aws-otel.github.io/docs/getting-started/java-sdk/auto-instr).
Expand Down Expand Up @@ -45,4 +50,4 @@ In addition to the sample apps in this repository, there are also a set of [stan
Please note that as per policy, we're providing support via GitHub on a best effort basis. However, if you have AWS Enterprise Support you can create a ticket and we will provide direct support within the respective SLAs.

## Security issue notifications
If you discover a potential security issue in this project we ask that you notify AWS/Amazon Security via our [vulnerability reporting page](http://aws.amazon.com/security/vulnerability-reporting/). Please do **not** create a public github issue.
If you discover a potential security issue in this project we ask that you notify AWS/Amazon Security via our [vulnerability reporting page](http://aws.amazon.com/security/vulnerability-reporting/). Please do **not** create a public github issue.
2 changes: 1 addition & 1 deletion appsignals-tests/contract-tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
java
kotlin("jvm") version "1.8.22"
kotlin("jvm") version "2.1.0-RC2"
}

java {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,19 +263,19 @@ private void assertSemanticConventionsAttributes(
List<KeyValue> attributesList,
String service,
String method,
String peerName,
int peerPort,
String address,
int port,
String url,
int statusCode) {
assertThat(attributesList)
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.RPC_METHOD, method))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.RPC_SERVICE, service))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.RPC_SYSTEM, "aws-api"))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.NET_PEER_NAME, peerName))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.NET_PEER_PORT, peerPort))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.SERVER_ADDRESS, address))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.SERVER_PORT, port))
.satisfiesOnlyOnce(
assertAttribute(SemanticConventionsConstants.HTTP_STATUS_CODE, statusCode))
.satisfiesOnlyOnce(assertAttributeStartsWith(SemanticConventionsConstants.HTTP_URL, url))
assertAttribute(SemanticConventionsConstants.HTTP_RESPONSE_STATUS_CODE, statusCode))
.satisfiesOnlyOnce(assertAttributeStartsWith(SemanticConventionsConstants.URL_FULL, url))
.satisfiesOnlyOnce(assertKeyIsPresent(SemanticConventionsConstants.THREAD_ID));
}

Expand All @@ -284,16 +284,16 @@ private void assertSemanticConventionsSqsConsumerAttributes(
List<KeyValue> attributesList,
String service,
String method,
String peerName,
int peerPort,
String address,
int port,
String url) {
assertThat(attributesList)
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.RPC_METHOD, method))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.RPC_SERVICE, service))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.RPC_SYSTEM, "aws-api"))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.NET_PEER_NAME, peerName))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.NET_PEER_PORT, peerPort))
.satisfiesOnlyOnce(assertAttributeStartsWith(SemanticConventionsConstants.HTTP_URL, url))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.SERVER_ADDRESS, address))
.satisfiesOnlyOnce(assertAttribute(SemanticConventionsConstants.SERVER_PORT, port))
.satisfiesOnlyOnce(assertAttributeStartsWith(SemanticConventionsConstants.URL_FULL, url))
.satisfiesOnlyOnce(assertKeyIsPresent(SemanticConventionsConstants.THREAD_ID));
}

Expand All @@ -308,8 +308,8 @@ private void assertSpanClientAttributes(
String type,
String identifier,
String cloudformationIdentifier,
String peerName,
int peerPort,
String address,
int port,
String url,
int statusCode,
List<ThrowingConsumer<KeyValue>> extraAssertions) {
Expand All @@ -327,8 +327,8 @@ private void assertSpanClientAttributes(
type,
identifier,
cloudformationIdentifier,
peerName,
peerPort,
address,
port,
url,
statusCode,
extraAssertions);
Expand All @@ -345,8 +345,8 @@ private void assertSpanProducerAttributes(
String type,
String identifier,
String cloudformationIdentifier,
String peerName,
int peerPort,
String address,
int port,
String url,
int statusCode,
List<ThrowingConsumer<KeyValue>> extraAssertions) {
Expand All @@ -363,8 +363,8 @@ private void assertSpanProducerAttributes(
type,
identifier,
cloudformationIdentifier,
peerName,
peerPort,
address,
port,
url,
statusCode,
extraAssertions);
Expand All @@ -377,8 +377,8 @@ private void assertSpanConsumerAttributes(
String operation,
String localService,
String method,
String peerName,
int peerPort,
String address,
int port,
String url,
int statusCode,
List<ThrowingConsumer<KeyValue>> extraAssertions) {
Expand All @@ -391,7 +391,7 @@ private void assertSpanConsumerAttributes(
assertThat(span.getKind()).isEqualTo(SpanKind.SPAN_KIND_CONSUMER);
assertThat(span.getName()).isEqualTo(spanName);
assertSemanticConventionsSqsConsumerAttributes(
spanAttributes, rpcService, method, peerName, peerPort, url);
spanAttributes, rpcService, method, address, port, url);
assertSqsConsumerAwsAttributes(span.getAttributesList(), operation);
for (var assertion : extraAssertions) {
assertThat(spanAttributes).satisfiesOnlyOnce(assertion);
Expand All @@ -412,8 +412,8 @@ private void assertSpanAttributes(
String type,
String identifier,
String cloudformationIdentifier,
String peerName,
int peerPort,
String address,
int port,
String url,
int statusCode,
List<ThrowingConsumer<KeyValue>> extraAssertions) {
Expand All @@ -425,9 +425,8 @@ private void assertSpanAttributes(
var spanAttributes = span.getAttributesList();
assertThat(span.getKind()).isEqualTo(spanKind);
assertThat(span.getName()).isEqualTo(spanName);

assertSemanticConventionsAttributes(
spanAttributes, rpcService, method, peerName, peerPort, url, statusCode);
spanAttributes, rpcService, method, address, port, url, statusCode);
assertAwsAttributes(
spanAttributes,
localService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ protected Map<String, String> getApplicationEnvironmentVariables() {
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
COLLECTOR_HTTP_ENDPOINT,
"OTEL_RESOURCE_ATTRIBUTES",
getApplicationOtelResourceAttributes());
getApplicationOtelResourceAttributes(),
"OTEL_EXPORTER_OTLP_PROTOCOL",
"grpc");
}

protected Map<String, String> getApplicationExtraEnvironmentVariables() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ protected long getThreshold(String metricName) {
switch (metricName) {
// If maximum memory size is undefined, then value is -1
// https://docs.oracle.com/en/java/javase/17/docs/api/java.management/java/lang/management/MemoryUsage.html#getMax()
// Thread count can be negative when excutor is null
// https://github.com/apache/tomcat/blob/1afe41491f0e56ec0a776db5ff84607f87ce6640/java/org/apache/tomcat/util/net/AbstractEndpoint.java#L1204
case JMXMetricsConstants.TOMCAT_THREADS:
threshold = -2;
break;
case JMXMetricsConstants.JVM_HEAP_MAX:
case JMXMetricsConstants.JVM_NON_HEAP_MAX:
case JMXMetricsConstants.JVM_POOL_MAX:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,12 @@ protected void assertSemanticConventionsAttributes(
assertThat(attributesList)
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.NET_PEER_NAME);
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.SERVER_ADDRESS);
assertThat(attribute.getValue().getStringValue()).isEqualTo("server");
})
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.NET_PEER_PORT);
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.SERVER_PORT);
assertThat(attribute.getValue().getIntValue()).isEqualTo(50051L);
})
.satisfiesOnlyOnce(
Expand Down Expand Up @@ -266,15 +266,10 @@ protected void assertSemanticConventionsAttributes(
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.RPC_SERVICE);
assertThat(attribute.getValue().getStringValue()).isEqualTo(GRPC_SERVICE_NAME);
})
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.NET_PEER_NAME);
assertThat(attribute.getValue().getStringValue()).isEqualTo("server");
})
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey())
.isEqualTo(SemanticConventionsConstants.NET_SOCK_PEER_ADDR);
.isEqualTo(SemanticConventionsConstants.NETWORK_PEER_ADDRESS);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,20 +271,20 @@ protected void assertSemanticConventionsAttributes(
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey())
.isEqualTo(SemanticConventionsConstants.NET_SOCK_PEER_ADDR);
.isEqualTo(SemanticConventionsConstants.NETWORK_PEER_ADDRESS);
})
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey())
.isEqualTo(SemanticConventionsConstants.NET_SOCK_PEER_PORT);
.isEqualTo(SemanticConventionsConstants.NETWORK_PEER_PORT);
})
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.NET_HOST_PORT);
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.SERVER_PORT);
})
.satisfiesOnlyOnce(
attribute -> {
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.NET_HOST_NAME);
assertThat(attribute.getKey()).isEqualTo(SemanticConventionsConstants.SERVER_ADDRESS);
assertThat(attribute.getValue().getStringValue()).isEqualTo("localhost");
});
}
Expand Down
Loading

0 comments on commit f604e22

Please sign in to comment.