Add console_without_tokio_unstable to make tokio_unstable assertion optional#446
Add console_without_tokio_unstable to make tokio_unstable assertion optional#446hawkw merged 2 commits intotokio-rs:mainfrom
console_without_tokio_unstable to make tokio_unstable assertion optional#446Conversation
89d3358 to
3ba6d9e
Compare
|
@ldm0 I'm concerned that removing this assertion would make the first user experience of using Is the reason that you don't wish to build with Would adding another cfg flag which you could use instead (and that would do nothing else) also solve your issue? For example, we could add a cfg flag like |
Exactly, that's one of the reasons. We use a cargo-feature to gate the instrumentations in our runtime, thus enabling Introducing less cfg is also important to maintain compilation speed, since adding or removing cfg invalidates all compilation cache.
And yes, first user experience is important. |
3ba6d9e to
0e6d4cf
Compare
tokio_unstable in console-subscribertokio_unstable_assertion to make cfg assertion optional
|
@ldm0 There are additional downsides to using a feature for this instead of a cfg flag. Some other crate in your dependency graph could enable the feature and you'd miss the warning. Also, once we add a feature, it can't be removed, even if it effectively does nothing in the future, without being a breaking change. Whereas even if we remove this assertion from the code in the future, it's not a breaking change if the code still compiles and runs as before. I'm not sure I understand your concern with the cfg flag. Changing a flag does force a recompile, that's true, but it is a one time operation (I don't see a use case where someone would need to regularly turn this flag on and off). From my point of view, that outweighs the negatives of introducing a feature flag for this case. |
0e6d4cf to
65564d1
Compare
tokio_unstable_assertion to make cfg assertion optionalconsole_without_tokio_unstable to make tokio_unstable assertion optional
|
@hds Okay, cfg is also appropriate though. At least now there is a way to turn off the assertion. :D |
e9fbd47 to
531f2ea
Compare
hds
left a comment
There was a problem hiding this comment.
Sorry for the delay, I hadn't seen that you pushed changes already. Let's combine the 2 cfg checks.
531f2ea to
bc754ef
Compare
|
@hawkw What do you think about this approach to allow people to use |
hawkw
left a comment
There was a problem hiding this comment.
I'm fine with this approach. However, I think that if we're adding this flag, we should also make sure to add it to the documentation as well.
We discuss enabling Tokio's instrumentation in the docs here: https://github.com/tokio-rs/console/blob/main/console-subscriber/README.md#enabling-tokio-instrumentation. We should probably add a new section on using other runtimes that mentions the new cfg option.
hawkw
left a comment
There was a problem hiding this comment.
let's make sure to add this to the docs, and then i'll be happy to approve it!
bc754ef to
4c28842
Compare
|
Documentation is updated. |
4c28842 to
130d4af
Compare
hds
left a comment
There was a problem hiding this comment.
Thank you for your change! This looks good to me.
Add cfg `console_without_tokio_unstable` for developers to turn off the assertion on `tokio_unstable`. This is useful for non-tokio runtimes which has `tokio-console` support.
1a38943 to
ad44459
Compare
Add cfg `console_without_tokio_unstable` for developers to turn off the assertion on `tokio_unstable`. This is useful for non-tokio runtimes which has `tokio-console` support.
# Changelog All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## console-subscriber-v0.2.0 - (2023-09-29) [0b0c1af](https://github.com/tokio-rs/console/commit/0b0c1aff18c3260d3a45a78f6c0d6f4206af1cbb)...[0b0c1af](https://github.com/tokio-rs/console/commit/0b0c1aff18c3260d3a45a78f6c0d6f4206af1cbb) ### <a id = "console-subscriber-v0.2.0-breaking"></a>Breaking Changes - **Update Tonic and Prost dependencies ([#364](#364 ([f9b8e03](https://github.com/tokio-rs/console/commit/f9b8e03bd7ee1d0edb441c94a93a350d5b06ed3b))<br />This commit updates the public dependencies `prost` and `tonic` to semver-incompatible versions (v0.11.0 and v0.8.0, respectively). This is a breaking change for users who are integrating the `console-api` protos with their own `tonic` servers or clients. - **Update `tonic` to v0.10 and increase MSRV to 1.64 ([#464](#464 ([96e62c8](https://github.com/tokio-rs/console/commit/96e62c83ef959569bb062dc8fee98fa2b2461e8d))<br />This is a breaking change for users of `console-api` and `console-subscriber`, as it changes the public `tonic` dependency to a semver-incompatible version. This breaks compatibility with `tonic` 0.9.x and `prost` 0.11.x. ### Added - [**breaking**](#console-subscriber-v0.2.0-breaking) Update Tonic and Prost dependencies ([#364](#364)) ([f9b8e03](f9b8e03)) - Add support for Unix domain sockets ([#388](#388)) ([a944dbc](a944dbc), closes [#296](#296)) - Add scheduled time per task ([#406](#406)) ([f280df9](f280df9)) - Add task scheduled times histogram ([#409](#409)) ([d92a399](d92a399)) - Update `tonic` to 0.9 ([#420](#420)) ([48af1ee](48af1ee)) - Update MSRV to Rust 1.60.0 ([b18ee47](b18ee47)) - Expose server parts ([#451](#451)) ([e51ac5a](e51ac5a)) - Add cfg `console_without_tokio_unstable` ([#446](#446)) ([7ed6673](7ed6673)) - Add warning for tasks that never yield ([#439](#439)) ([d05fa9e](d05fa9e)) - [**breaking**](#console-subscriber-v0.2.0-breaking) Update `tonic` to v0.10 and increase MSRV to 1.64 ([#464](#464)) ([96e62c8](96e62c8)) ### Documented - Fix unclosed code block ([#463](#463)) ([362bdbe](362bdbe)) - Update MSRV version docs to 1.64 ([#467](#467)) ([94a5a51](94a5a51)) ### Fixed - Fix build on tokio 1.21.0 ([#374](#374)) ([c34ac2d](c34ac2d)) - Fix off-by-one indexing for `callsites` ([#391](#391)) ([43891ab](43891ab)) - Bump minimum Tokio version ([#397](#397)) ([bbb8f25](bbb8f25), fixes [#386](#386)) - Fix self wakes count ([#430](#430)) ([d308935](d308935)) - Remove clock skew warning in `start_poll` ([#434](#434)) ([4a88b28](4a88b28)) - Do not report excessive polling ([#378](#378)) ([#440](#440)) ([8b483bf](8b483bf), closes [#378](#378)) - Correct retain logic ([#447](#447)) ([36ffc51](36ffc51)) Signed-off-by: Eliza Weisman <eliza@buoyant.io>
We add the same trace span in the runtime of our company project as tokio to support tokio-console, but we don't enable the
tokio_unstablecfg. Removing this assertion allows us to stop using a forked console-subscriber.