util: Make tracing optional when codec is enabled

[Gelbpunkt]

Motivation

I maintain a library that implements the websocket protocol using tokio-util's FramedRead for decoding frames. I conduct testing using cargo-minvers in CI to ensure that my minimum dependency versions are correct and working as they are intended to.

proc-macro2 is a crate that very, very often breaks with newer rustc versions (most recently, a rustc update broke versions prior to 1.0.60, see this issue). tracing-attributes depends on proc-macro2 and regularly has to bump the minimum version to fix their own cargo-minvers checks (see this commit bumping the dependency).

My issue lies exactly here. tokio-util's codec module depends on tracing, which in turn breaks my minvers check every once in a while due to the tracing version required by tokio-util being too low with the recent rustc I run my checks with. Of course bumping the dependency in tokio-util makes little sense, so my current "fix" is to pin the minimum version of tracing in my crate myself (like so).

But that is just tedious and seems wrong for a dependency that is only used for trace! macro calls. A lot of headaches and needless CI failures could just be fixed by not depending on tracing.

Solution

This PR makes the tracing usage in the codec module optional, allowing people to opt-in to the events or out of the regular minvers breakage.

[mox692]

The change itself looks good to me, but I guess users already using the codec feature's tracing logs will now need to explicitly opt-in the tracing feature.

@Darksonn @hawkw
What do you think about this change?

[taiki-e]

This seems reasonable to me, but I have two questions.

• tracing feature does not seem to be documented anywhere in the tokio-util.
• Should the full feature of tokio-util enable tracing feature?

[hawkw]

I think making the tracing dependency in tokio-util/codec optional is probably fine.

However, I do want to offer an alternative solution, which would also benefit tokio-util users who do consume the codec tracing events: we could add default-features = false to the tracing dependency. This would disable the tracing/attributes feature flag, which is where the proc-macro2 dep comes from. The codec module only uses tracing's function-like macros, and does not use the #[tracing::instrument] attribute, so we don't need the proc-macro deps that are only necessary for attributes.

This way, we remove the proc-macro2 dep without forcing existing users to have to opt-in to tracing.

[taiki-e]

@hawkw

However, I do want to offer an alternative solution, which would also benefit tokio-util users who do consume the codec tracing events: we could add default-features = false to the tracing dependency. This would disable the tracing/attributes feature flag, which is where the proc-macro2 dep comes from. The codec module only uses tracing's function-like macros, and does not use the #[tracing::instrument] attribute, so we don't need the proc-macro deps that are only necessary for attributes.

tokio-util already has disabled the attribute, and tokio-websockets does too. I think there is a crate somewhere in tokio-websockets's dependency tree that has the attribute enabled, but I'm not sure how easy it is to find it.

tokio/tokio-util/Cargo.toml

Line 45 in 9c337ca

tracing = { version = "0.1.25", default-features = false, features = ["std"], optional = true }

https://github.com/Gelbpunkt/tokio-websockets/pull/48/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542L19

[Gelbpunkt]

@hawkw

However, I do want to offer an alternative solution, which would also benefit tokio-util users who do consume the codec tracing events: we could add default-features = false to the tracing dependency. This would disable the tracing/attributes feature flag, which is where the proc-macro2 dep comes from. The codec module only uses tracing's function-like macros, and does not use the #[tracing::instrument] attribute, so we don't need the proc-macro deps that are only necessary for attributes.

tokio-util already has disabled the attribute, and tokio-websockets does too. I think there is a crate somewhere in tokio-websockets's dependency tree that has the attribute enabled, but I'm not sure how easy it is to find it.

tokio/tokio-util/Cargo.toml

Line 45 in 9c337ca

tracing = { version = "0.1.25", default-features = false, features = ["std"], optional = true }

https://github.com/Gelbpunkt/tokio-websockets/pull/48/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542L19

Good catch! I found the issue: hyper-util depends on tower > 0.4.0, which will use tracing with default features... See tower-rs/tower@7f004da

That is an easy fix since I wanted to rip out the hyper specific stuff anyways, but I think that this PR has some added value nonetheless. Consumers of this crate that either don't need tracing or don't want trace logs from this library (very likely) won't need the tracing dependency in this crate.

EDIT: I don't get it...

$ cargo update -Z minimal-versions
$ cargo tree --all-features | grep tracing -B 25
│   │   │   │   │   └── cc v1.0.72
│   │   │   │   ├── dunce v1.0.1
│   │   │   │   └── fs_extra v1.3.0
│   │   │   ├── mirai-annotations v1.12.0
│   │   │   ├── paste v1.0.11 (proc-macro)
│   │   │   └── zeroize v1.7.0
│   │   ├── log v0.4.8 (*)
│   │   ├── once_cell v1.16.0
│   │   ├── ring v0.17.0 (*)
│   │   ├── rustls-pki-types v1.2.0
│   │   ├── rustls-webpki v0.102.2
│   │   │   ├── aws-lc-rs v1.6.0 (*)
│   │   │   ├── ring v0.17.0 (*)
│   │   │   ├── rustls-pki-types v1.2.0
│   │   │   └── untrusted v0.9.0
│   │   ├── subtle v2.5.0
│   │   └── zeroize v1.7.0
│   ├── rustls-pki-types v1.2.0
│   └── tokio v1.6.1 (*)
├── tokio-util v0.7.1
│   ├── bytes v1.0.0
│   ├── futures-core v0.3.14
│   ├── futures-sink v0.3.14
│   ├── pin-project-lite v0.2.5
│   ├── tokio v1.6.1 (*)
│   └── tracing v0.1.25
│       ├── cfg-if v1.0.0
│       ├── pin-project-lite v0.2.5
│       ├── tracing-attributes v0.1.13 (proc-macro)
│       │   ├── proc-macro2 v1.0.55 (*)
│       │   ├── quote v1.0.26 (*)
│       │   └── syn v1.0.3 (*)
│       └── tracing-core v0.1.17

Seems like tokio-util is the only thing depending on tracing, and tracing-attributes are pulled in.

EDIT 2: tokio-util 0.7.3 is the first without default features on tracing, that should fix it.

[Darksonn]

I guess it's probably fine to disable the feature by default.

[hawkw]

I'm fine with disabling the feature by default, as I agree most users probably don't consume these traces.

[Darksonn]

Thanks.