feat: Makes the tracing dependency optional.

[keskalukasfri]

No description provided.

[keskalukasfri]

I've made it off by default now, but that can be changed if preferred.

[Copilot]

Pull request overview

This PR makes the tracing dependency optional for the httpsig crate by removing the internal trace re-export module and gating all logging usage behind a new tracing feature.

Changes:

• Introduces a tracing Cargo feature and marks the tracing dependency as optional.
• Removes httpsig::trace module usage and switches call sites to conditional use tracing::{...} imports.
• Wraps logging calls with #[cfg(feature = "tracing")] to allow building without tracing.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
httpsig/src/trace.rs	Removes the internal re-export layer for tracing macros.
httpsig/src/signature_params.rs	Drops trace::* and conditionally logs invalid params via tracing::error.
httpsig/src/message_component/component.rs	Drops trace::* and conditionally logs req parameter handling via tracing::debug.
httpsig/src/lib.rs	Removes the mod trace; module declaration.
httpsig/src/crypto/symmetric.rs	Drops trace::* and conditionally logs symmetric signing/verification steps.
httpsig/src/crypto/asymmetric.rs	Drops trace::* and conditionally logs asymmetric key parsing/signing/verification (some RSA branches need fixes).
httpsig/Cargo.toml	Adds tracing feature and makes the tracing dependency optional.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[junkurihara]

omg. it must be unacceptable.

[junkurihara]

Thanks for the PR! Making tracing optional is acceptable, but I'd like to request two changes before merging.

1. Keep the feature enabled by default (commented inline)
2. Keep the trace.rs facade instead of scattering #[cfg] attributes (commented inline)
3. Rebase needed
   The branch currently conflicts with develop (httpsig/Cargo.toml and src/signature_params.rs) after the recent dependency updates, so please rebase as well.

[junkurihara]

Log output of existing users should not silently disappear, so the current behavior must be preserved.

[features]
default = ["tracing"]
tracing = ["dep:tracing"]

[junkurihara]

Adding #[cfg(feature = "tracing")] to ~20 individual call sites is hard to maintain: every future log line would need to remember the guard, and a single forgotten one breaks the --no-default-features build. Please keep src/trace.rs and make it provide no-op macros when the feature is disabled, so that all call sites stay untouched:

#[cfg(feature = "tracing")]
#[allow(unused)]
pub(crate) use tracing::{debug, error, info, trace, warn};

/// No-op macros used when the `tracing` feature is disabled.
/// They expand to `()` so that they are usable both in statement and
/// expression positions, like the original `tracing` macros.
/// `warn` is defined as `warn_` and re-exported, since a macro named
/// `warn` conflicts with the built-in `warn` attribute (E0659).
#[cfg(not(feature = "tracing"))]
mod noop {
  #![allow(unused_macros)]
  macro_rules! debug { ($($arg:tt)*) => { () }; }
  macro_rules! error { ($($arg:tt)*) => { () }; }
  macro_rules! info { ($($arg:tt)*) => { () }; }
  macro_rules! trace { ($($arg:tt)*) => { () }; }
  macro_rules! warn_ { ($($arg:tt)*) => { () }; }
  #[allow(unused_imports)]
  pub(crate) use {debug, error, info, trace, warn_ as warn};
}

#[cfg(not(feature = "tracing"))]
#[allow(unused)]
pub(crate) use noop::*;

I have verified locally that this compiles with the feature both on and off. Two gotchas encoded in the snippet:

• a macro_rules! macro named warn cannot be re-exported because it collides with the built-in warn attribute (E0659), hence the warn_ definition re-exported under the name warn;
• the no-op macros must expand to () rather than to nothing, because some call sites use them in expression position (e.g. the _ => { error!(...) } arm in signature_params.rs).

With this approach the diff shrinks to Cargo.toml and trace.rs only. Please revert the changes to the other five files.

[junkurihara]

Note that Copilot's four comments above are all valid. I confirmed that building with --features rsa-signature alone fails to compile with 7 errors on this branch. They are exactly the class of bug the no-op macro facade eliminates.

Since fixing this requires rewriting the diff almost entirely, I have debugged and reimplemented the change on my side using the facade approach, and verified it across the full feature matrix (default / --no-default-features / rsa-signature with and without tracing, plus tests). I'll close this PR and push the reworked change to develop directly. Thanks for raising this and for the initial implementation!