-
Notifications
You must be signed in to change notification settings - Fork 873
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 the OpenTelemetry SDK version to 1.37.0 #11066
Conversation
@@ -4,7 +4,7 @@ plugins { | |||
|
|||
dependencies { | |||
implementation(project(":instrumentation-api")) | |||
implementation("io.opentelemetry:opentelemetry-extension-incubator") | |||
implementation("io.opentelemetry:opentelemetry-api-incubator") |
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.
is this dependency needed at all?
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.
yes, for EventLogger
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.
this module does not use EventLogger
as far as I can tell
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.
right, mixed that up with another module - removed
@Override | ||
@CanIgnoreReturnValue | ||
public Span addLink(SpanContext spanContext) { | ||
agentSpan.addLink(Bridging.toAgent(spanContext)); | ||
return this; | ||
} | ||
|
||
@Override | ||
@CanIgnoreReturnValue | ||
public Span addLink(SpanContext spanContext, Attributes attributes) { | ||
agentSpan.addLink(Bridging.toAgent(spanContext), Bridging.toAgent(attributes)); | ||
return this; | ||
} | ||
|
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.
this deviates a bit from our usual approach of adding bridging for new methods in a subclass, but here it is much easier to do it the way it is so I think it is fine.
configurations.configureEach { | ||
if (name == "testRuntimeClasspath" || name == "testCompileClasspath") { | ||
resolutionStrategy { | ||
force("io.opentelemetry:opentelemetry-api:1.31.0") |
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.
@laurit should we add something similar in all of the opentelemetry-api-* javaagent modules?
testing { | ||
suites { | ||
val incubatorTest by registering(JvmTestSuite::class) { | ||
dependencies { | ||
implementation("io.opentelemetry:opentelemetry-api-incubator:1.37.0-alpha") | ||
} | ||
} | ||
} | ||
} |
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.
it might be worth adding an oldAndNewIncubatorTest
that pulls in both the -extension-incubator
and -api-incubator
artifacts
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.
...c/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzer.java
Show resolved
Hide resolved
Co-authored-by: opentelemetrybot <[email protected]> Co-authored-by: Trask Stalnaker <[email protected]> Co-authored-by: Lauri Tulmin <[email protected]>
No description provided.