Skip to content
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

Support new semantice conventions. #972

Merged
merged 3 commits into from
Dec 11, 2024

Conversation

zzhlogin
Copy link
Contributor

@zzhlogin zzhlogin commented Dec 10, 2024

Description of changes:
This PR adding support of new semantic conventions in latest version from otel upstream:
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

Testing are done for both old upstream ve
Screenshot 2024-12-10 at 3 42 53 PM
rsion v1.32.1 and new upstream version v2.10.0.

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

@zzhlogin zzhlogin requested a review from a team as a code owner December 10, 2024 23:19
Copy link
Member

@mxiamxia mxiamxia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zzhlogin zzhlogin merged commit 448d680 into aws-observability:main Dec 11, 2024
4 checks passed
zzhlogin added a commit that referenced this pull request Dec 19, 2024
*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.
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.

2 participants