Add console_without_tokio_unstable to make tokio_unstable assertion optional

[ldm0]

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_unstable cfg. Removing this assertion allows us to stop using a forked console-subscriber.

[hds]

@ldm0 I'm concerned that removing this assertion would make the first user experience of using tokio-console and console-subscriber potentially worse (and by a lot). If a new user forgets to enable tokio_unstable or doesn't add it in the right place, then console-subscriber will "fail silently".

Is the reason that you don't wish to build with tokio_unstable that you don't want to enable other unstable APIs in tokio?

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 console_without_tokio_unstable, which would avoid this assertion, but otherwise do nothing - the expectation being that anyone using this flag really know what they're doing.

[ldm0]

@hds

Is the reason that you don't wish to build with tokio_unstable that you don't want to enable other unstable APIs in tokio?

Exactly, that's one of the reasons. We use a cargo-feature to gate the instrumentations in our runtime, thus enabling tokio_unstable cfg is not necessary.

Introducing less cfg is also important to maintain compilation speed, since adding or removing cfg invalidates all compilation cache.

I'm concerned that removing this assertion would make the first user experience of using tokio-console and console-subscriber potentially worse (and by a lot). If a new user forgets to enable tokio_unstable or doesn't add it in the right place, then console-subscriber will "fail silently".

And yes, first user experience is important.
Would you mind my adding a default cargo-feature like tokio_unstable_assertion: When it's disabled, the assertion check is turned off.
In this way, the tokio_unstable cfg check is on by default, and developers can turn off the assertion manually by turning off the feature.

[hds]

@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.

[ldm0]

@hds Okay, cfg is also appropriate though. At least now there is a way to turn off the assertion. :D

[hds]

Sorry for the delay, I hadn't seen that you pushed changes already. Let's combine the 2 cfg checks.

[hds]

@hawkw What do you think about this approach to allow people to use console-subscriber without the tokio_unstable cfg flag (because their instrumentation is coming from elsewhere)?

[hawkw]

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]

let's make sure to add this to the docs, and then i'll be happy to approve it!

[ldm0]

Documentation is updated.

[hds]

Thank you for your change! This looks good to me.