RFC: Default feature policy

[Gelbpunkt]

Currently, there is no clear and defined default feature policy. Overall, currently the default feature set from crate to crate varies: http has decompression enabled by default, but gateway-queue does not.

So, the question I am raising is: What makes a feature a default feature?

Overall, there are several different approaches to default feature sets in the rust ecosystem:

• no default features (like tokio or hyper)
• only a default feature on std (like serde)
• minimal functionality features (like rand)
• commonly used features (like axum)
• or all features, but I couldn't find an example off of my head

Twilight has no set policy for this. For example, tracing is a default feature, but isn't required for functionality whatsoever, just like compression/decompression features. They do not extend the API visible to the user, so they do not fall into the "commonly used features" category.

The cargo docs say:

Note: Be careful about choosing the default feature set. The default features are a convenience that make it easier to use a package without forcing the user to carefully select which features to enable for common use, but there are some drawbacks. Dependencies automatically enable default features unless default-features = false is specified. This can make it difficult to ensure that the default features are not enabled, especially for a dependency that appears multiple times in the dependency graph. Every package must ensure that default-features = false is specified to avoid enabling them.

I propose a new policy based on this: The default feature set should be the most broadly compatible yet minimal feature set possible required for common APIs to be accessible. This would mean that TLS features remain a default, due to being required for the APIs to be accessible/working, but things like compression and tracing would no longer be a default because they are not required for the APIs to be accessible.

Twilight is highly configurable via feature flags and the users who do not put in the time to read the documentation on them should be given the features for APIs to work, but maybe not perform as good as they might. Disabling compression might hurt network performance a tiny bit - but for someone running a tiny bot with default feature flags, it does not matter. If they cared about every single possible performance improvement, they would have enabled features like simd-json, simd-zlib or trust-dns previously already.

The approach I mentioned - "the most broadly compatible [...] feature set" - also would mean changing the default TLS backend. Currently, we are using rustls-native-roots instead of native-tls in order to be less dependent on the surrounding OS, because native-tls is highly dependent on available system libraries. However, native-roots still depends on the OS: It tries to use the OS certificate storage. This can cause problems on unsupported operating systems or fully static binaries in scratch docker containers. webpki-roots instead ships the mozilla certificate list and does not depend on the OS to provide certificates. There is no situation where webpki-roots would not work while native-roots would, which is the reason why this would be a logical change based on this RFC.

[vilgotf]

I agree with the rest of the post, but I think tracing should also be enabled by default. Other feature flags fit broadly into three categories: niche (like metrics), API extensions or performance enhancers. Tracing, logging, differs from this because it's essential for diagnostics when something (eventually) goes wrong. As such it's nearly always something one wants, from cargo docs:

select which features to enable for common use

Moreover, it's currently a challenge to fully enabled it, requiring one to add private twilight dependencies due to it being disabled by default.

[7596ff]

the most broadly compatible yet minimal feature set

I would have to do a survey to find this out for sure, but I feel that if we wish to respect the "minimal" aspect of this approach, we would just need to use a feature that relies on the OS feature storage, since using a scratch Docker container, by my own intuition, is just not a common enough use case. So I would still prefer to stick with the rustls-native-roots feature as the default in this case.

Regarding the rest of your RFC, I think it's good, and we should definitely make our default features across our crates consistent. I also tend to agree with enabling tracing by default since disabling it for performance is a specific use case as well.