[breaking] deps(udp): make tracing optional and add optional log

[mxinden]

This commit makes the tracing dependency in quinn-udp optional, but enabled by default. If disabled, log is used. By adding it to the default feature set, this is not a breaking change.

---

Context:

• I am working on integrating quinn-udp into Firefox. See mozilla-central pull request.

• Each Rust dependency in Firefox needs to be audited via cargo vet.

  ➜  mozilla-unified git:(quic-rust-udp-io) ✗ ./mach cargo vet 
  Vetting Failed!
  
  3 unvetted dependencies:
    tracing:0.1.37 missing ["safe-to-deploy"]
    tracing-attributes:0.1.24 missing ["safe-to-deploy"]
    tracing-core:0.1.30 missing ["safe-to-deploy"]
  
  recommended audits for safe-to-deploy:
      Command                                      Publisher  Used By                                   Audit Size
      cargo vet inspect tracing-attributes 0.1.24  hawkw      tracing                                   5025 lines
      cargo vet inspect tracing-core 0.1.30        hawkw      tracing                                   7032 lines
      cargo vet inspect tracing 0.1.34             hawkw      h2, warp, hyper, quinn-udp, and 2 others  11016 lines
  
  estimated audit backlog: 23073 lines
  

• quinn-udp optionally not depending on tracing will safe Mozilla a lot of work auditing all of tracing.

Questions:

• Would you be open to making tracing optional in quinn-udp?
• If yes, how would you want to proceed with the log feature? See inline comment below.

Follow-up to #1473 and #1903

[djc]

Definitely unexcited about this change, but I suppose I can live with it.

Does the cargo vet output imply that mozilla-central already depends on h2 and warp, or just that those are the most popular crates.io crates that depend on tracing?

[mxinden]

Does the cargo vet output imply that mozilla-central already depends on h2 and warp, or just that those are the most popular crates.io crates that depend on tracing?

mozilla-central depends on h2 and warp as dev-dependencies. warp is used to test the crashreporter, h2 is, I assume, used in warp through hyper only.

quinn-udp will be used not as a dev-dependency, but as a dependency. Thus it requires a stronger audit, namely a safe-to-deploy audit instead of a safe-to-run audit.

[mxinden]

I don't think the freebsd CI check failure is related:

  thread 'tests::key_update_simple' panicked at quinn-proto/src/tests/mod.rs:996:5:
  assertion failed: `None` does not match `Some(Event::Stream(StreamEvent::Readable { id })) if id == s`
  
  
  failures:
      tests::key_update_simple
  
  test result: FAILED. 232 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.94s
  
  error: test failed, to rerun pass `-p quinn-proto --lib`

[djc]

Thanks, this looks good to me.

[Ralith]

Thanks!

[Ralith]

Note that quinn-udp is presently a public dependency of quinn by direct reexport. I don't recall offhand if there's a very good reason for this, but it does mean we can't update quinn to use quinn-udp 0.6 without a breaking change.

I think we could avoid the breaking change in quinn-udp in this change. For example, we could have a feature native-log = ["dep:log"] and retain the existing log feature. That may or may not be worth the trouble/weirdness depending on whether we expect other breaking quinn-udp changes soon.

[djc]

I think we could avoid the breaking change in quinn-udp in this change. For example, we could have a feature native-log = ["dep:log"] and retain the existing log feature. That may or may not be worth the trouble/weirdness depending on whether we expect other breaking quinn-udp changes soon.

I think that makes sense as a temporary solution until the next breaking change? I think the only downside is the slightly weird feature name, which seems manageable if that lets us retain semver compatibility.

[mxinden]

Good catch @Ralith. Thanks.

I created #1923 implementing your suggestion above.

That may or may not be worth the trouble/weirdness depending on whether we expect other breaking quinn-udp changes soon.

Happy to prepare an additional clean-up pull request that can be merged once a breaking change release is in sight.

What do you think @djc and @Ralith?

[Ralith]

LGTM! Merged #1923.