chore: Upgrade io.opentelemetry version to 1.58.0#26644
chore: Upgrade io.opentelemetry version to 1.58.0#26644imjalpreet merged 1 commit intoprestodb:masterfrom
Conversation
9e4c1ff to
6fb33bb
Compare
imjalpreet
left a comment
There was a problem hiding this comment.
Thanks, @sumi-mathew.
I don't see any tests present in the open-telemetry plugin. It would be good to at least manually test out the plugin to ensure that everything is working since expected since we are moving from 1.19.0(released in 2022) to 1.56.0(released last month).
pom.xml
Outdated
| <dependency> | ||
| <groupId>io.opentelemetry</groupId> | ||
| <artifactId>opentelemetry-semconv</artifactId> | ||
| <version>1.19.0-alpha</version> | ||
| <version>1.30.1-alpha</version> |
There was a problem hiding this comment.
It looks like this dependency has moved to https://mvnrepository.com/artifact/io.opentelemetry.semconv/opentelemetry-semconv, and there are more recent versions released.
bc77898 to
81b04da
Compare
|
@imjalpreet Could you please re-review the PR |
agrawalreetika
left a comment
There was a problem hiding this comment.
Thanks for the changes. I have just one comment
pom.xml
Outdated
| <dep.fastutil.version>8.5.2</dep.fastutil.version> | ||
| <dep.datasketches-memory.version>2.2.0</dep.datasketches-memory.version> | ||
| <dep.datasketches-java.version>5.0.1</dep.datasketches-java.version> | ||
| <dep.io.opentelemetry.version>1.56.0</dep.io.opentelemetry.version> |
There was a problem hiding this comment.
Why don't we upgrade to the latest 1.58.0 version?
There was a problem hiding this comment.
At the time of raising the PR, the latest version was 1.56.0. I’ve now updated it to 1.58.0.
81b04da to
15f2f3a
Compare
imjalpreet
left a comment
There was a problem hiding this comment.
Thanks, @sumi-mathew.
LGTM % one small recommendation.
| { | ||
| Resource resource = Resource.getDefault() | ||
| .merge(Resource.create(Attributes.of(ResourceAttributes.SERVICE_NAME, "presto"))); | ||
| .merge(Resource.create(Attributes.of(AttributeKey.stringKey("service.name"), "presto"))); |
There was a problem hiding this comment.
ResourceAttributes was deprecated and removed, but SERVICE_NAME was moved to ServiceAttributes: https://github.com/open-telemetry/semantic-conventions-java/blob/release/v1.37.0/semconv/src/main/java/io/opentelemetry/semconv/ServiceAttributes.java
Let's migrate to this rather than using a hardcoded value in Presto.
There was a problem hiding this comment.
Thanks for pointing this out. I’ve migrated to ServiceAttributes as suggested and removed the hardcoded value from Presto.
15f2f3a to
e4d76be
Compare
imjalpreet
left a comment
There was a problem hiding this comment.
LGTM. Thanks, @sumi-mathew.

Description
Upgrade io.opentelemetry version to 1.58.0
Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and ensures we aren't relying on outdated or unsupported code.
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.