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

Adopt new io.opentelemetry.semconv:opentelemetry-semconv lib #1059

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cyrille-leclerc
Copy link
Member

Description:

Adopt new io.opentelemetry.semconv:opentelemetry-semconv lib without yet replacing http.url and http.method by url.full and http.request.method as we will want to align with the OTel Java Auto Instrumentation and support -Dotel.semconv-stability.opt-in=http... to embrace the new semantic conventions

Existing Issue(s):

Testing:

None as there are no change in the behavior of the instrumentation

Documentation:

None as there are no change in the behavior of the instrumentation

Outstanding items:

Not really outstanding but we will have to add support for the new HTTP Semantic Conventions, url.full and http.request.method and we want to do it the same way the Otel Java Auto Instr implementing -Dotel.semconv-stability.opt-in=http... as described in https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v1.27.0

@@ -29,6 +32,7 @@ public List<MavenGoal> getSupportedGoals() {
MavenGoal.create("com.google.cloud.tools", "jib-maven-plugin", "build"));
}

@SuppressWarnings("deprecation") // until old http semconv are dropped in 2.0
Copy link
Member

Choose a reason for hiding this comment

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

@mateuszrzeszutek @jack-berg @laurit we haven't discussed bumping the contrib repo to 2.0 when we bump the instrumentation repo, but it could make sense to keep the -instrumentation and -contrib repos version aligned

@srprash @wangzlei would you want to follow this approach instead of the clean break that was merged in #1050? (we have time before the next release to revert that change if you want)

@SylvainJuge
Copy link
Contributor

Hi @cyrille-leclerc do you think there are still changes in this PR that are relevant to include ?

Some of the changes were included in #1299 and there are now a few conflicts, so it might need a bit of time (and work) to update and fix it.

@trask trask added the needs author feedback Waiting for additional feedback from the author label Sep 16, 2024
Copy link
Contributor

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@github-actions github-actions bot added the Stale label Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs author feedback Waiting for additional feedback from the author Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants