-
Notifications
You must be signed in to change notification settings - Fork 18
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
Update semantic convention requirements #165
Conversation
pom.xml
Outdated
@@ -32,6 +32,7 @@ | |||
<properties> | |||
<inceptionYear>2022</inceptionYear> | |||
<opentelemetry.java.version>1.32.0</opentelemetry.java.version> | |||
<opentelemetry.semconv.version>1.24.0</opentelemetry.semconv.version> |
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.
Shouldn't we use the same version as used inside:
<groupId>io.opentelemetry.instrumentation</groupId>
<artifactId>opentelemetry-instrumentation-api-semconv</artifactId>
... they have 1.21.0-alpha
in there.
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.
I'm unable to find reference to this artifact in spec or the current TCKs.
Tracing TCK depends on io.opentelemetry.semconv:opentelemetry-semconv:jar:1.23.1-alpha
, which is the most recent version "official" semconv package from https://github.com/open-telemetry/semantic-conventions-java.
No instrumentation artifacts are referenced outside of io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations:jar:1.32.0
, which only depends on opentelemetry-api
.
My argument is that while these APIs can be useful for implementations, they are too fragile to bind them to a spec. It is ok to use them, but not to make it available to end users.
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 cannot use io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations
as an alternative to io.opentelemetry.semconv:opentelemetry-semconv
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.
@yasmin-aumeeruddy what will you suggest to update in @pdudits 's PR?
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.
I think io.opentelemetry.semconv:opentelemetry-semconv:jar:1.23.1-alpha
would be the right articact to use
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.
From today's meeting, @pdudits will update the version to 1.25
and tck dependency to 1.25.0-alpha
Partially addresses #150.
The last one may be a bit surprising one, but due to following reasons: