opentelemetry-rust
opentelemetry-rust copied to clipboard
Upgrade to hyper/http v1.0
Fixes #1427
Blocked by:
- https://github.com/hyperium/tonic/pull/1670
- https://github.com/tokio-rs/axum/pull/2693
Changes
- Update all hyper and http dependencies to v1.x
- Update reqwest to v0.12
- Remove isahc since it's still stuck on http v0.2
Merge requirement checklist
- [x] CONTRIBUTING guidelines followed
- [x] Unit tests added/updated (if applicable)
- [x] Appropriate
CHANGELOG.mdfiles updated for non-trivial, user-facing changes - [ ] Changes in public API reviewed (if applicable)
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: aumetra / name: Aumetra Weisman (c93e43f9ae12868a01ebbb7dda2c958b733209b4, 25f9213ab87eeb080df88242aba333c37bbc82ec, 1a3d711b4d983c9d9b17ff7c5547043d0b5069a5, e22bb0b063fc42fa0813f537df907bd84b3eecba, f5c73eb240aec3934af0e2fe62a41ce1ca18ee22, 75b325927004ccc9f1d0dbba5be98620a12ef000, 7d1c3a1635d95a01fd966fb99e7ef14f3d9b377c, 95608c5759d34fa825767e375fc66738b0007b56, 318bb62b530cdb043191d24998eb6a4d369ee5ff, 88ad41698a4e36269fa8668006c9e661d7c9dcfa, 19ddbd8ae6066feb41205f9918dee185d9d99ebd, 8ad764ac9ea5fef69788f083405560ad0d8b1149, 6f34ee354a31ea1822470c70480047c0283c80f2, 40fa5766e3cc2786c1010764e7b841c96cd60c6a, adda11d6d3a55144c74ab850e45a02b77b99c2b2, 70a9bc7c6cb215e191f9a57577fdfdd8b937c841, a87360bb186130e87eb44f37a1baa93fd6a5131a, 6c75e7c5d8c98ede77c14bece26bb33810ddadc6, 45903d803b529eaed84cf2bfdae676ec6b0d846a, e52ba2c29618767a0616bdb8c0d3660e2483f816, af97e0ed5a6de7748c78b99f0e59c110934e2a0b, eb2ae3295d3f8c5f1670d21a4c09edb71650bae9, 650e688f7f4f6e59e37aa7e83d2b4d5e104da00b, 8e6c5d2a9e7c8839213832b4c62ee7ca9c0b73db, 0664b648d9dda61739f86dd433c66f3751b42da3, 23c09c781e478dbad5992c3df309b8997e980d54, 5944e90d64cd9fc397ded0b18f13b6550bebb7db, 16783065f157ba507670057325938f3a4ff5b1b9, 832b1cb3496be28fc303671f1c9375327464aeab, c8409787d4923d1efc557a82fd458adcdaecafaa, 1d1cef9c6f1d481830ea1448300080abe4e63b56, ab7a3a4154bae41780362678e1f697f00ffb6f63)
- :white_check_mark: login: lalitb / name: Lalit Kumar Bhasin (7d9670f2c19a0f6250e2e5f494cc84459717805b)
Lint needs to have a feature enabled that's transitively enabled if the full workspace is checked, will do that later.
The external types check is broken because the nightly used is too old and axum unconditionally enables the diagnostic attribute on nightly without the feature flag because they assume everyone is at least on a nightly that is 1.68
Sidenote: The some tests in this repository seem flakey? Locally I sometimes have to rerun the tests a few times before they pass. Weird.
The some tests in this repository seem flakey
Yeah feel free to report those test in https://github.com/open-telemetry/opentelemetry-rust/issues/1559. Most of them are because global setup / env vars and we are working on addressing them as they pop up
Codecov Report
Attention: Patch coverage is 10.00000% with 18 lines in your changes missing coverage. Please review.
Project coverage is 75.0%. Comparing base (
df815bd) to head (7d9670f). Report is 1 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| opentelemetry-http/src/lib.rs | 0.0% | 18 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #1674 +/- ##
=======================================
- Coverage 75.0% 75.0% -0.1%
=======================================
Files 122 122
Lines 20276 20290 +14
=======================================
Hits 15222 15222
- Misses 5054 5068 +14
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
(missed that one during the last rebase)
I was wondering if this is planned to be merged anytime soon?
I was wondering if this is planned to be merged anytime soon?
I'm intending to get this over the finish line eventually, issue is just that the tonic PR is still pending
@aumetra The tonic PR is merged. do you think we can now remove the patch from this PR, and make it ready to review/merge.
Edit - Or probably better to wait for tonic release with merged changes.
As soon as it's released, sure!
Seems like tonic is close to releasing v0.12 with hyper v1 support.
After they publish it, I'll rebase and remove the patches
https://github.com/hyperium/tonic/pull/1740
@aumetra as https://github.com/hyperium/tonic/pull/1740 is merged, can you resolve the conflicts in this PR, and fixing the patch dependency, so that it can be reviewed and merged ?
Sorry, had to set my PC up again after moving :P Gonna rebase it real quick and then address the review comments.
This ::prost::Enumeration issue (https://github.com/tokio-rs/prost/issues/1097) is a bit annoying. I silenced it for now because it doesn't seem like there is an actual solution to the problem yet.
prost v0.13.0 got yanked so we gotta deal with the broken CI for a little bit
0.13.1 has been released.
Well then, let's see how far it gets now.
Looking pretty good. Just a question to the code owners if they'd prefer to split the isahc removal into a separate PR. I'm fine with doing either.
Looking pretty good. Just a question to the code owners if they'd prefer to split the isahc removal into a separate PR. I'm fine with doing either.
Thanks @aumetra . As @djc suggested, good to split as separate PR.
Looking pretty good. Just a question to the code owners if they'd prefer to split the isahc removal into a separate PR. I'm fine with doing either.
Thanks @aumetra . As @djc suggested, good to split as separate PR.
Unless you think it would be difficult to split, as isahc client depends on http 0.2.1.
I would split it up like this:
- create PR to delete that client
- get that merged
- rebase on main after merge
- merge this
+1 on split the client clean up to another PR to control the scope
Removed isahc support in https://github.com/open-telemetry/opentelemetry-rust/pull/1924, gonna rebase this on that branch so we can move ahead.
Just rebased it real quick to make the eventual rebase on main based on https://github.com/open-telemetry/opentelemetry-rust/pull/1924 easier.
Waiting for that PR to merge for now.
@aumetra Is the upgrade to opentelemetry-proto submodule intended/required? If not, can we remove it from this PR, to be handled separately.
@aumetra Is the upgrade to
opentelemetry-protosubmodule intended/required? If not, can we remove it from this PR, to be handled separately.
Not intended. Probably mistyped somewhere when attempting to reset the git submodule. Gonna revert that
@lalitb are you planning a release? Need help with that?
@lalitb are you planning a release? Need help with that?
@djc yes I believe @cijothomas plans to do a release today if there are no issues.
Great, looking forward to it!