tracing: Consider Switching compile-time filters from Cargo features to `RUSTFLAGS`

[davidbarsky]

Feature Request

Crates

• tracing

Motivation

Cargo's feature flags are global across the crate graph and are additive. This means if tracing's compile time filters are enabled in a dependent crate (often times, by accident!), all crates in the dependency graph will transitively enable the compile-time filter with no mechanism of disabling the feature in a consuming crate. This is not a good experience and introduces an ecosystem-wide papercuts.

Proposal

tracing 0.2 should remove the Cargo-based compile time filters from tracing and replace them with a RUSTFLAGS-based approach, similar to how Tokio enables unstable features. This means that only the root of the dependency of crate graph (a binary that takes a dependency on tracing) can enable a compile time filter. That filter cannot not be transitively added to other libraries or binaries since RUSTFLAGS are not part of a registry's crate metadata.

Alternatives

Don't do this and keep compile-time filters as is. Unfortunately, this is a papercut with ecosystem-wide risk.

cc: @oli-obk (impacts rustc's builds)

[oli-obk]

Hmm... can we figure out a transition path so we can incrementally start using the new system without having to switch to 0.2 immediately?

[davidbarsky]

Hmm... can we figure out a transition path so we can incrementally start using the new system without having to switch to 0.2 immediately?

I mean, possibly? I'm not sure how to cut the features over incrementally, especially in cases where RUSTFLAGS conflicts with a Cargo-based filter. I think we can make it possible for a tracing 0.1-emitting crate to be consumed by a tracing 0.2 Collect (née Subscriber) through the semver trick. I can also update rustc's tracing dependencies (e.g., tracing-tree) to tracing 0.2 prior to 0.2's release.

What are your specific concerns here, by the way? What can we/I do to address them?

[hawkw]

Hmm... can we figure out a transition path so we can incrementally start using the new system without having to switch to 0.2 immediately?

New RUSTFLAGS-based compile-time filtering could certainly be added in v0.1.x without removing the feature-flag-based compile-time filtering. The primary benefit of the change wouldn't be from adding the cfg-based filtering, though, but from removing the feature-flag-based filtering, but we could add the new system now so that users can migrate to it prior to the breaking change.

We may also want to consider having the feature-flag-based compile-time filters generate a deprecation warning in v0.1.x, after the cfg-based filters have been added?

[jyn514]

I would rather not do this. RUSTFLAGS are global and are often set by .cargo/config; I don't know of a way to merge .cargo/config with a process-local variable. They're also harder to experiment with because any change means you need to recompile your entire dependency tree (and cargo will actually wipe the previous cache, so changing back has the same impact too).

[oli-obk]

These don't really need experimentation, and changing the features will rebuild most of your dependency tree anyway.

This is more something you change once for your entire project and then forget about it. I would prefer other alternatives, but something global like an env var or specifically like RUSTFLAGS seems best to me.

We could pick another env var by abusing build scripts, that would limit the rebuilds to everything using tracing.

[davidbarsky]

They're also harder to experiment with because any change means you need to recompile your entire dependency tree (and cargo will actually wipe the previous cache, so changing back has the same impact too).

+1 to what Oli said. We don't expect the compile-time filter to be experimented with—if any experimentation is going on, it'd be at the tail end of changes after the necessary information has been determined through other, lighter-weight mechanisms.

We could pick another env var by abusing build scripts, that would limit the rebuilds to everything using tracing.

Given the prevalence of tracing in people's dependency graphs, I think it's it's fair to while rebuilds will technically be limited to crates that use tracing, it will be still be a substantial fraction of crates. I'm also weary—and this one is kinda irrational—about introducing additional build scripts, especially in a crate like tracing.

[hawkw]

Given the prevalence of tracing in people's dependency graphs, I think it's it's fair to while rebuilds will technically be limited to crates that use tracing, it will be still be a substantial fraction of crates. I'm also weary—and this one is kinda irrational—about introducing additional build scripts, especially in a crate like tracing.

This is incorrect. Changing the RUSTFLAGS environment variable will always rebuild everything, not just crates that depend on tracing, the flags may affect any number of aspects of how the crate is compiled.

[jyn514]

@hawkw I think @davidbarsky meant that if we use a new env variable read by a build script, it will still be a significant fraction of crates because tracing is widely used.

(I disagree that the savings are negligible; if nothing else this saves you rebuilding build scripts and integration tests.)

[hawkw]

Oh, got it, I misinterpreted that post. Thanks!

[oli-obk]

if nothing else this saves you rebuilding build scripts

Can you elaborate on this? How would a custom env var cause build scripts and integration from getting rebuilt?

[jyn514]

if nothing else this saves you rebuilding build scripts

Can you elaborate on this? How would a custom env var cause build scripts and integration from getting rebuilt?

Other way around. There are three options:

1. Use cargo feature flags like we do today. This rebuilds tracing when you change the feature flag, and also if you add a dependency that enables the feature flag (because of feature unification).
2. Use RUSTFLAGS. This rebuilds everything unconditionally on any change, including build scripts and dev dependencies.
3. Use a custom env variable read by tracing's build script, with rerun-on-env-changed. This rebuilds tracing when you change the env variable.

My point was that 3 has a significant time savings over 2.

( I am still not sure why 1 is a bad option though? That's the normal way to do feature flags.)

[oli-obk]

😆 actually, I don't have an opinion here. I can work with either. The env var has the advantage that you don't have 10 features to choose from, but a single env var for which you set only a single value. The disadvantages have already been mentioned.

Independently of that: adding a build script/env var is helpful to get rid of the feature shuffling in https://github.com/tokio-rs/tracing/blob/master/tracing/src/level_filters.rs and could make #[instrument] truly be a nop in case its level is disabled, instead of always generating a wrapper function only to let optimizations figure out it's a nop wrapper.

[jyn514]

Independently of that: adding a build script/env var is helpful to get rid of the feature shuffling in https://github.com/tokio-rs/tracing/blob/master/tracing/src/level_filters.rs and could make #[instrument] truly be a nop in case its level is disabled, instead of always generating a wrapper function only to let optimizations figure out it's a nop wrapper.

hmm, I'm not sure why the build script would help there? you can already use those cfgs in tracing's normal library code, if it's a pain to do the #[cfg(all(not(debug_assertions), feature = "release_max_level_off"))] dance you could add a helper macro.

[oli-obk]

if it's a pain

That's not it, it's just that what is supposed to be an enum with 5 variants is a bitset with 5 bits and you check each bit starting at the highest one. It's weird that you can set multiple different levels in your dependency tree.

[davidbarsky]

Oh, got it, I misinterpreted that post. Thanks!

Sorry! I was being oblique about the fact that tracing is now a "big deal".

I am still not sure why 1 is a bad option though? That's the normal way to do feature flags.

While this is the normal way to do feature flags, there are two motivating issue:

• these aren't traditional feature flags, as they're not additive in behavior. Just before I opened this issue, someone accidentally took a dependency on tracing and set the filter to trace, iirc. This took a bit of effort to find and debug, and while the person fixed it very quickly once it was reported, it'd be nice to get rid of footguns that can infect dependency graphs with spooky-action-at-a-distance.
• At $WORK, we have a big monorepo where crates are imported into a third-party/ folder. As part of that vendoring process, every Cargo feature is enabled (except for those that would break consumers, such as these feature flags). I don't think my employer's monorepo is unique in this regard. Switching to RUSTFLAGS or a build.rs would allow people to use these compile-time filters.

[utkarshgupta137]

Another use case: We would like to use release_max_level_info when deploying to production but use release_max_level_debug when deploying to staging. Since our code base is spread across multiple repos, each with 10-50 crates, it is not easy to do this via feature flags.