-
Notifications
You must be signed in to change notification settings - Fork 86
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
Upgrade to opentelemetry 0.25 #164
Conversation
@@ -702,3 +675,16 @@ where | |||
Ok(()) | |||
} | |||
} | |||
|
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 don’t understand the issue yet, but the test failed because the order of attributes in the exported metrics differed each time. Therefore, I added a sorting process during the assertion.
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.
Thanks for working on this! I think this is mostly looking good, but I'd like to have a cleaner commit history where basically each commit could pass CI (though I won't ask you to actually test this). As such:
- Please have reorder the commits to put the clippy commit first, as I assume this is independent of the opentelemtry upgrade
- Please squash the commit with the dependency bump with the commit that fixes the tests, also include the commit for fixing the warnings from deprecations
- Add a version bump for this crate into the commit that updates the CHANGELOG
Thank you for your review. |
|
@djc
but it should have been:
Should I create a PR to address this now? https://github.com/tokio-rs/tracing-opentelemetry/blob/v0.26.0/CHANGELOG.md |
Yes, please. |
Motivation
Update opentelemetry depencencies to 0.25
https://github.com/open-telemetry/opentelemetry-rust/releases/tag/opentelemetry-0.25.0
From version 0.25, OpenTelemetry started using a unified version across all crates.
open-telemetry/opentelemetry-rust#2084
Solution
AttributeSet
from testsdeployment.environment
semantic conv todeployment.environment.name