Skip to content

Upgrade to otel 0.30#205

Merged
mladedav merged 2 commits intotokio-rs:v0.1.xfrom
BSteffaniak:upgrade-otel-to-0.30
May 31, 2025
Merged

Upgrade to otel 0.30#205
mladedav merged 2 commits intotokio-rs:v0.1.xfrom
BSteffaniak:upgrade-otel-to-0.30

Conversation

@BSteffaniak
Copy link
Copy Markdown
Contributor

@BSteffaniak BSteffaniak commented May 23, 2025

Hi, I upgraded to the newest opentelemetry 0.30 and it broke compatibility with this crate. This should hopefully fix those incompatibility issues.

Motivation

There were some breaking changes with the upgrade to otel 0.30: https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-sdk/CHANGELOG.md#0300

I was able to fix upgrade them without much of an issue. The only compilation issues were in the metrics_publishing test. A few of the types used in that test now require the experimental_metrics_custom_reader feature to be enabled in the opentelemetry-sdk package.

Solution

Bump the versions & fix the one test file that had compilation errors.

Please let me know if I'm missing anything. Thanks!

@BSteffaniak
Copy link
Copy Markdown
Contributor Author

BSteffaniak commented May 23, 2025

There were definitely some weird things with the downcasting that are now potentially problematic with this update. I was able to manually impl AsAny, but after testing (with the metrics_gauge_unstable feature enabled) a bit more it seems like that might not be fully working. I'm not super experienced with std::any::Any (I try to stay away from that), so I might be missing something silly.

@BSteffaniak BSteffaniak force-pushed the upgrade-otel-to-0.30 branch from e6102bb to 037db4e Compare May 23, 2025 20:55
@BSteffaniak
Copy link
Copy Markdown
Contributor Author

Ok, I think I was able to fix the rest of the AsAny errors. I just needed to fill out the rest of the match arms. It still feels a bit off... but again, I don't really write much AsAny, so I could have done that all wrong.

@djc
Copy link
Copy Markdown
Collaborator

djc commented May 27, 2025

Okay, which organization(s) want to actually support work on this crate? Reviewing the regular treadmill that upstream seems to like is not fun for me, I have no direct (paid) usage for this crate, whereas I have a bunch of other paid work I could spend time on.

@markcda
Copy link
Copy Markdown

markcda commented May 27, 2025

Okay, which organization(s) want to actually support work on this crate? Reviewing the regular treadmill that upstream seems to like is not fun for me, I have no direct (paid) usage for this crate, whereas I have a bunch of other paid work I could spend time on.

I can assist you in this work.

@djc
Copy link
Copy Markdown
Collaborator

djc commented May 27, 2025

Okay, which organization(s) want to actually support work on this crate? Reviewing the regular treadmill that upstream seems to like is not fun for me, I have no direct (paid) usage for this crate, whereas I have a bunch of other paid work I could spend time on.

I can assist you in this work.

It's kind of you to offer, but I'm looking for financial support. Even if someone wanted to be a maintainer, it would have to be someone I can trust with what ends up being a fairly widely used crate so it's not easy to just send out invites.

@BSteffaniak
Copy link
Copy Markdown
Contributor Author

@djc, you're probably already very much aware of this, but I've noticed a lot of churn with updates from otel crates -- the ecosystem seems pretty fragmented. However, from what I can tell it looks like they might be trying to bring some of these straggler crates into under a 'contrib' repo: https://github.com/open-telemetry/opentelemetry-rust-contrib. Do you think it would make sense to move this crate definition into that repo?

@hdost
Copy link
Copy Markdown
Contributor

hdost commented May 27, 2025

@djc, you're probably already very much aware of this, but I've noticed a lot of churn with updates from otel crates -- the ecosystem seems pretty fragmented. However, from what I can tell it looks like they might be trying to bring some of these straggler crates into under a 'contrib' repo: https://github.com/open-telemetry/opentelemetry-rust-contrib. Do you think it would make sense to move this crate definition into that repo?

We were just talking about this as being a possibility so that we (the OTel contributors) have the ability to maintain it directly.

Comment thread CHANGELOG.md Outdated
[changelog](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-sdk/CHANGELOG.md#0300)
for more information.

# 0.31.0 (May 23, 2025)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You probably wanted to put this at the top, before the new change (assuming the intent is to have the change released).

But usually it's better to not bump versions and creating releases in PRs like this since the date gets out of sync if the review (or release) takes longer than a day, multiple PRs may independently bump versions, skipping over some, etc.

So keep the change log and delete just this line, revert the change bumping the version in Cargo.toml (of this package, not the dependencies).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed version bump in Cargo.toml & removed this line.

Comment on lines +580 to +582
trait AsAny {
fn as_any(&self) -> &dyn std::any::Any;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is fine for now, but basically what you're doing is extracting heterogenous data from an enum to then convert it to a type you think it was. You could have just added methods like unwrap_sum and similar and it'd be a bit clearer without the need for these Any shenanigans.

But this is rather something that the upstream API should expose I think, so in the meantime I think this is fine.

@BSteffaniak BSteffaniak force-pushed the upgrade-otel-to-0.30 branch from 6cd59e7 to 582d4ba Compare May 31, 2025 12:57
Copy link
Copy Markdown
Collaborator

@mladedav mladedav left a comment

Choose a reason for hiding this comment

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

Thanks for the update

@mladedav mladedav merged commit 8905ee7 into tokio-rs:v0.1.x May 31, 2025
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.

5 participants