feat(observability): optional tracing instrumentation (off by default, OTel-ready) - #733
Merged
Conversation
…, OTel-ready
Adds an opt-in `tracing` feature that instruments the library with spans for end-to-end visibility into connection, receive/decrypt, send, IQ, app-state, pairing, media and session/crypto flows. Off by default: with the feature disabled there is no `tracing` dependency and zero overhead. The library only emits `tracing` and never installs a subscriber or depends on OpenTelemetry; the application wires that up (see examples/observability.rs). The existing ~835 `log` calls are unchanged and flow into a subscriber via the log to tracing bridge.
Design (modeled on hyper's optional-tracing pattern and the tracing/OpenTelemetry library guidance):
- `tracing = { optional = true, default-features = false, features = ["attributes"] }` in wacore and the main crate; feature `tracing = ["dep:tracing", "wacore/tracing"]`, not in `default`. `default-features = false` keeps the tracing `log` feature OFF so an app-side LogTracer bridge cannot recurse.
- All instrumentation is `#[cfg_attr(feature = "tracing", tracing::instrument(name = "wa.*", level = "debug"/"trace", skip_all, fields(...), err(Debug)))]`, so it compiles to nothing (and links no dependency) when the feature is off. ~115 spans across 28 files, grouped under `wa.{conn,recv,send,appstate,pair,media,session}.*` plus `wa.iq`.
PII handling (first-class for a lib that carries phone numbers):
- `Jid::observe()` renders LID / group / broadcast / newsletter / bot JIDs in full (pseudonymous or non-personal, so the same peer/chat correlates across spans) and replaces phone-number users with a stable `pn#<fnv-hash>` token: traceable without leaking the number, allocation-free, and computed only while an enabled span is being recorded. The `tracing-pii` feature (off by default) renders raw numbers for local debugging only.
- Spans carry redacted correlation fields (chat/sender/peer via observe(), msg_id, ns/kind, reason) and `err(Debug)` on Result paths, so a production error maps to who/where/how/why.
examples/observability.rs shows subscriber + log-bridge wiring and OTLP guidance.
Verified: builds and `clippy --all-targets -- -D warnings` are clean both with and without `--features tracing` (and with `tracing-pii`); `cargo fmt --all -- --check` clean; full test suite green with no behavior change.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds workspace tracing features and an observability example, implements privacy-aware JID observation/redaction, and applies feature-gated ChangesObservability instrumentation with privacy-aware JID redaction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62aa3d8455
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Benchmark Results67 unchanged benchmark(s)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/prekeys.rs (1)
311-313:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop mutating
Devicedirectly in pre-key upload pathThis write bypasses the required command pipeline. Replace
modify_device(...)with aDeviceCommandandprocess_command()so state transitions stay serialized and consistent.Suggested fix
- self.persistence_manager - .modify_device(|d| d.server_has_prekeys = true) - .await; + self.persistence_manager + .process_command(DeviceCommand::SetServerHasPrekeys(true)) + .await;As per coding guidelines: "Never modify Device state directly; always use
DeviceCommand+PersistenceManager::process_command()for state mutations andget_device_snapshot()for reading state".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/prekeys.rs` around lines 311 - 313, The code is directly mutating Device via self.persistence_manager.modify_device(|d| d.server_has_prekeys = true), which violates the rule to only mutate state through DeviceCommand and PersistenceManager::process_command; replace the modify_device call by constructing an appropriate DeviceCommand variant (e.g., SetServerHasPrekeys or similar) that sets server_has_prekeys to true and invoke self.persistence_manager.process_command(command).await (handling the Result/error as other command usages do); when reading state elsewhere use get_device_snapshot() instead of direct mutation/reads.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Line 102: Update the stale example path in the feature documentation inside
Cargo.toml: replace the reference string "examples/otel.rs" with the new example
path "examples/observability.rs" so feature docs correctly point to the
observability example (search for the literal "examples/otel.rs" in Cargo.toml
to locate the doc text to edit).
In `@wacore/binary/src/jid.rs`:
- Around line 875-881: The current Display code in jid.rs uses an unsalted
FNV-1a over jid.user (the loop over jid.user.as_bytes() and write!(f,
"pn#{hash:08x")) which is reversible; replace this with a keyed
pseudonymization: compute an HMAC-SHA256 (or keyed BLAKE3) over
jid.user.as_bytes() using the service secret key (load from config/env),
truncate the hex output to a fixed length (e.g., 8-16 hex chars) and write that
as the pseudonym (keep the "pn#" prefix), and ensure the secret key is obtained
from your existing config loader or init path rather than hardcoding so the
token is not deterministic across deployments without the key.
- Around line 866-885: The redacted branch in fmt::Display for ObservedJid
currently writes "pn#{hash}[:device]`@server`" but omits jid.agent, which
collapses distinct identities for Server::Interop/Server::Messenger; update the
redacted formatting to include the agent portion exactly as the non-redacted
path does (i.e., after optional :device write the agent component when present)
so ObservedJid::fmt preserves .agent in the redacted output for
Server::Interop/Server::Messenger.
---
Outside diff comments:
In `@src/prekeys.rs`:
- Around line 311-313: The code is directly mutating Device via
self.persistence_manager.modify_device(|d| d.server_has_prekeys = true), which
violates the rule to only mutate state through DeviceCommand and
PersistenceManager::process_command; replace the modify_device call by
constructing an appropriate DeviceCommand variant (e.g., SetServerHasPrekeys or
similar) that sets server_has_prekeys to true and invoke
self.persistence_manager.process_command(command).await (handling the
Result/error as other command usages do); when reading state elsewhere use
get_device_snapshot() instead of direct mutation/reads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a38c848b-dd09-4270-b05f-aea7feee8bbd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
Cargo.tomlexamples/observability.rssrc/client/app_state.rssrc/client/device_registry.rssrc/client/lid_pn.rssrc/client/lifecycle.rssrc/client/messaging.rssrc/client/node_io.rssrc/client/sender_keys.rssrc/client/sessions.rssrc/download.rssrc/handshake.rssrc/history_sync.rssrc/keepalive.rssrc/message/dispatch.rssrc/message/msg_secret.rssrc/message/receive.rssrc/message/retry.rssrc/message/special.rssrc/pair.rssrc/pair_code.rssrc/prekeys.rssrc/request.rssrc/send.rssrc/upload.rswacore/Cargo.tomlwacore/binary/Cargo.tomlwacore/binary/src/jid.rswacore/src/appstate_sync.rswacore/src/send/dm.rswacore/src/send/encrypt.rswacore/src/send/group.rswacore/src/send/peer.rs
…eview) Addresses the PR #733 reviews (Codex P1 + CodeRabbit). Jid::observe(): replace the unsalted FNV-1a phone token with a keyed hash (SipHash via a process-lifetime random key). An unkeyed digest of an E.164 number is reversible by precomputation, so it would still leak recoverable numbers in exported traces; the random key lives only in process memory, making tokens non-recoverable while staying stable within a run for correlation (a fresh key each restart is an intentional privacy tradeoff). Preserve `.agent` in the redacted output where the agent is part of the identity (Interop/Messenger), so distinct IDs do not collapse into one token. Fix the stale `examples/otel.rs` reference in the Cargo.toml feature docs; the example is `examples/observability.rs`. Add a unit test asserting observe() never leaks the raw number, preserves device, and renders LID/group JIDs in full. Not changed: CodeRabbit's prekeys.rs `modify_device` note is pre-existing code this PR does not touch (it only added a span there); it is a separate concern.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d8088ab93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Legacy group/broadcast ids have the form "<creator-phone>-<timestamp>", so rendering them in full leaked the creator's phone number in the new span fields. Jid::observe() now redacts the numeric prefix as `pn#<keyed-token>-<timestamp>` for Server::Group/Broadcast while keeping the timestamp (not PII) and group-level correlation. Modern opaque group ids (no numeric "<digits>-" prefix) still render in full. The keyed-token helper is shared with the phone-number path. Extends the unit test with the legacy-group case.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc83a33162
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // | ||
| // Every `wa.*` span is then exported as an OTLP span with its fields | ||
| // (chat/peer/msg_id are already privacy-redacted via `Jid::observe()`). | ||
| .init(); |
There was a problem hiding this comment.
When an app copies this OTLP example with tracing enabled, .init() installs the default tracing-log bridge, so the existing log! messages are exported alongside the new redacted span fields. Those legacy logs still format JIDs with normal Display rather than Jid::observe() (for example src/client/sessions.rs:247/:256 logs jid directly), so a phone-number JID can be emitted as raw PII even though the surrounding wa.* fields are redacted. Disable the log bridge for production tracing or scrub the existing log messages before recommending this wiring.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wacore/binary/src/jid.rs`:
- Around line 982-990: The test currently only checks the redacted ID's shape
but can still leak the creator number; update the test around
Jid::from_str("123456789-1620000000@g.us") / legacy.observe().to_string()
(variables legacy and ls) to assert explicitly that the original numeric prefix
("123456789") does not appear in ls (and/or in legacy.observe().to_string()),
i.e., add a no-leak assertion that the raw creator phone substring is not
contained in the observed/redacted string in addition to the existing shape and
inequality checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 55f45582-f4a7-4847-bd8c-aeea0bb9b2ed
📒 Files selected for processing (1)
wacore/binary/src/jid.rs
… (review pass) Second observability pass on PR #733: resolves the remaining gaps from the self-review and the Codex/CodeRabbit comments. Coverage: instrument the areas the first pass missed - retry/pdo, receipts and the incoming-stanza handlers (notification/receipt/message/presence/call/iq/ib/chatstate), the Bot API, usync, mediaconn, and assorted wacore leaf logic. ~57 new spans (172 total) under the wa.* taxonomy. PII in logs: the existing log!() calls rendered JIDs - and Signal ProtocolAddress names, which embed phone numbers - with raw Display, which would leak through the log->tracing bridge. Wrapped 107 raw-Jid log arguments with Jid::observe(), and added wacore::types::jid::observe_protocol_address (a keyed token reusing the now-public observe_token) to redact the 8 ProtocolAddress log sites. The example documents the bridge/PII tradeoff. Levels: promoted the connection-lifecycle spans (connect/disconnect/reconnect/run/logout) to info so connection state is visible at the default level; failures already surface via err(Debug) at ERROR and via the existing warn/error logs through the bridge. Account attribution: the wa.conn.run session-root span now records our own LID (pseudonymous) so connection traces are attributable per account in multi-account deployments. Hardening (CodeRabbit): the legacy-group redaction test now asserts the exact no-leak invariant (the raw creator number must not appear) plus the fixed-width keyed-token shape. Verified: cargo clippy --all-targets -- -D warnings clean with and without --features tracing; fmt clean; cargo test --workspace --exclude e2e-tests green (1992 passed, 0 failed).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a0fdbb5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// For LID mode, uses `group_info.phone_jid_for_lid_user` to query devices | ||
| /// via PN when available (LID usync is unreliable for own JID), then | ||
| /// converts the result back to LID. Same fallback as `prepare_group_stanza`. | ||
| #[cfg_attr(feature = "tracing", tracing::instrument(name = "wa.send.resolve_skdm_targets", level = "debug", skip_all, fields(group = %group_jid)))] |
There was a problem hiding this comment.
When the tracing feature is enabled and this path handles a legacy group JID like <creator-phone>-<timestamp>@g.us, this span records the raw group_jid string and bypasses the Jid::observe() redaction that now handles those IDs. Fresh evidence for the existing PII concern is that this newly added field uses %group_jid directly, so group sends through resolve_skdm_targets can still export the creator phone number in traces.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pair.rs (1)
279-281:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace direct device mutation with
DeviceCommandflow.This path mutates persisted device state via
modify_device, which breaks the repository rule requiring command-based mutations. Route this through aDeviceCommand(e.g., a dedicatedSetServerHasPrekeys(false)command) andprocess_command()so state transitions stay consistent.As per coding guidelines: "Never modify Device state directly; always use
DeviceCommand+PersistenceManager::process_command()for state mutations andget_device_snapshot()for reading state."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pair.rs` around lines 279 - 281, The code is directly mutating device state via persistence_manager.modify_device; instead, create and dispatch a DeviceCommand (e.g., a new enum variant like SetServerHasPrekeys(false) or existing equivalent) and call persistence_manager.process_command(command). Replace the modify_device(...) call with construction of the proper DeviceCommand and await persistence_manager.process_command(...) to perform the state change, and use persistence_manager.get_device_snapshot(...) when you need to read the device state elsewhere.Source: Coding guidelines
src/handlers/basic.rs (1)
86-105: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueAckHandler has no instrumentation while peer handlers do.
The three handlers above are instrumented but
AckHandleris not. If acks are too frequent to trace without noise, document that decision. Otherwise, add instrumentation for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/handlers/basic.rs` around lines 86 - 105, AckHandler's handle method lacks the tracing/instrumentation used by the other peer handlers; update the AckHandler::handle implementation to add the same instrumentation (e.g., the tracing::instrument or tracing::debug_span usage) around the function body so the call to client.handle_ack_response(node.get()).await is traced, or if you intentionally want to suppress noisy ack traces, add a short comment/docstring on AckHandler explaining that instrumentation was omitted for noise reasons. Ensure you modify the handle method on AckHandler (and keep the tag() method unchanged) so the span name/fields follow the same convention used by the other handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/observability.rs`:
- Around line 27-30: Update the explanatory sentence that currently suggests
dropping `whatsapp_rust`/`wacore` log targets — instead instruct users to drop
the calling application's log targets (the modules that actually emit raw
JIDs/phone numbers) because suppressing only the library bridge doesn't prevent
leaks from application code; replace the phrase "drop the
`whatsapp_rust`/`wacore` `log` targets" with wording that points to the caller's
targets (e.g., "drop your application's or caller module log targets that emit
raw JIDs") and keep the surrounding guidance about using `Jid::observe()` and
the `tracing-pii` feature intact.
In `@wacore/src/types/jid.rs`:
- Around line 136-140: The helper observe_protocol_address always hashes
addr.name() via wacore_binary::jid::observe_token, violating the workspace
contract to show raw phone numbers when the tracing-pii feature is enabled;
update observe_protocol_address to return the raw addr.name() (as a String) when
tracing-pii is enabled (e.g., cfg!(feature = "tracing-pii") or the appropriate
tracing-pii runtime check) and only call
wacore_binary::jid::observe_token(addr.name()) and format the "addr#..." hashed
form when tracing-pii is not enabled, keeping the function signature and using
the ProtocolAddress::name() accessor.
---
Outside diff comments:
In `@src/handlers/basic.rs`:
- Around line 86-105: AckHandler's handle method lacks the
tracing/instrumentation used by the other peer handlers; update the
AckHandler::handle implementation to add the same instrumentation (e.g., the
tracing::instrument or tracing::debug_span usage) around the function body so
the call to client.handle_ack_response(node.get()).await is traced, or if you
intentionally want to suppress noisy ack traces, add a short comment/docstring
on AckHandler explaining that instrumentation was omitted for noise reasons.
Ensure you modify the handle method on AckHandler (and keep the tag() method
unchanged) so the span name/fields follow the same convention used by the other
handlers.
In `@src/pair.rs`:
- Around line 279-281: The code is directly mutating device state via
persistence_manager.modify_device; instead, create and dispatch a DeviceCommand
(e.g., a new enum variant like SetServerHasPrekeys(false) or existing
equivalent) and call persistence_manager.process_command(command). Replace the
modify_device(...) call with construction of the proper DeviceCommand and await
persistence_manager.process_command(...) to perform the state change, and use
persistence_manager.get_device_snapshot(...) when you need to read the device
state elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bfc65640-913d-4e44-b65e-81645a7cfd3b
📒 Files selected for processing (36)
examples/observability.rssrc/bot.rssrc/client/app_state.rssrc/client/device_registry.rssrc/client/lid_pn.rssrc/client/lifecycle.rssrc/client/messaging.rssrc/client/node_io.rssrc/client/offline_resume.rssrc/client/sender_keys.rssrc/client/sessions.rssrc/handlers/basic.rssrc/handlers/call.rssrc/handlers/chatstate.rssrc/handlers/ib.rssrc/handlers/iq.rssrc/handlers/message.rssrc/handlers/notification.rssrc/handlers/presence.rssrc/mediaconn.rssrc/message/msg_secret.rssrc/message/receive.rssrc/message/retry.rssrc/message/special.rssrc/pair.rssrc/pdo.rssrc/prekeys.rssrc/receipt.rssrc/retry.rssrc/send.rssrc/usync.rswacore/binary/src/jid.rswacore/src/send/encrypt.rswacore/src/send/group.rswacore/src/types/jid.rswacore/src/usync.rs
…t group_jid (review) Addresses the latest Codex/CodeRabbit comments on PR #733. - observe_protocol_address now honors the tracing-pii feature (renders the raw address name when enabled), matching Jid::observe(); the feature is forwarded wacore -> wacore-binary. (CodeRabbit) - The wa.send.resolve_skdm_targets span took a raw group_jid: &str and would leak a legacy group's creator phone; it now goes through a new wacore_binary::jid::observe_str helper (parse + Jid::observe, keyed-token fallback, honors tracing-pii). (Codex P2) - examples/observability.rs: clarify that dropping the library log targets only silences the library bridge; the application's own raw-JID logs are a separate leak path the app must scrub. (CodeRabbit) - Raise recursion_limit to 512 in lib.rs and main.rs: instrumenting large async fns (process_sync_task) overflows the default depth limit when the tracing and tracing-pii paths combine. Verified: clippy --all-targets -- -D warnings clean for default, --features tracing, and --features "tracing,tracing-pii"; fmt clean; observe tests pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02a18383b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Error values themselves embedded raw JIDs (e.g. usync's "<device-list> not found for user {jid}"), so err(Debug) on a span - and the existing log::error!("{e}") - would export a raw phone number even with redacted span fields. The two error-message sites that interpolate a JID (wacore/src/usync.rs and src/client/sessions.rs) now use Jid::observe(). A sweep of anyhow!/bail!/format!/.context/thiserror confirmed these were the only two (no thiserror enum embeds a raw JID).
|
Docs PR opened: oxidezap/whatsapp-rust-docs#269 Added an Observability page, two feature-flag rows, and a changelog entry covering the optional tracing instrumentation feature. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an opt-in
tracingfeature that instruments the library for end-to-end observability across connection, receive/decrypt, send, IQ, app-state, pairing, media, receipts, retry, notifications and session/crypto flows. The goal is that a production error can be mapped to who / where / how / why.Off by default: with the feature disabled there is no
tracingdependency and zero overhead. The library only emitstracingand never installs a subscriber or depends on OpenTelemetry; the application wires that up (seeexamples/observability.rs). The existinglogcalls are unchanged and flow into a subscriber via the log to tracing bridge, so you get span-correlated structured logs even before adopting any new span.~172 spans across 30+ files, grouped under a
wa.{conn,recv,send,iq,appstate,pair,media,receipt,retry,pdo,notif,session,bot}.*taxonomy.Design
Modeled on hyper's optional-tracing pattern and the tracing/OpenTelemetry guidance for libraries.
tracing = { optional = true, default-features = false, features = ["attributes"] }inwacoreand the main crate; featuretracing = ["dep:tracing", "wacore/tracing"], deliberately not indefault.default-features = falsekeeps the tracinglogfeature OFF so an app-sideLogTracerbridge cannot recurse.#[cfg_attr(feature = "tracing", tracing::instrument(name = "wa.*", level = ..., skip_all, fields(...), err(Debug)))], so it compiles to nothing (and links no dependency) when the feature is off.debug/trace, except the connection-lifecycle spans (connect/disconnect/reconnect/run/logout) which areinfoso connection state is visible at the default level. Failures surface at ERROR viaerr(Debug), and the existing warn/error logs surface through the bridge.wa.conn.runsession-root span records our own LID (pseudonymous) so connection traces are attributable per account in multi-account deployments.PII (first-class, since the library carries phone numbers)
Jid::observe()renders LID / group / broadcast / newsletter / bot JIDs in full (pseudonymous or non-personal, so the same peer/chat correlates across spans) and replaces phone-number users with apn#<token>. The token is a keyed hash (SipHash with a process-lifetime random key), not a plain digest: an unkeyed hash of an E.164 number is reversible by precomputation, while the random key (kept only in memory) makes exported tokens non-recoverable. Legacy group ids<creator-phone>-<timestamp>get only the numeric prefix redacted.observe_protocol_address()applies the same keyed scheme to SignalProtocolAddressnames (which embed a phone number) used in logs.log!calls now render JIDs and addresses through these helpers (raw-Jid args wrapped withobserve(), address logs withobserve_protocol_address()), so the bridged logs carry the same redaction as the span fields.tracing-piifeature (off by default) renders raw numbers for local debugging only.Overhead
#[cfg_attr]attributes vanish, zero cost.debug/trace/info, so a downstream binary can statically strip lower levels withrelease_max_level_info/warn.Verification
cargo clippy --all-targets -- -D warningsclean both with and without--features tracing(and withtracing-pii).cargo fmt --all -- --checkclean.cargo test --workspace --exclude e2e-tests: green (1992 passed, 0 failed). The instrumentation is additive and cfg-gated, so no behavior change.Usage
Review
Addresses the Codex and CodeRabbit comments: keyed (non-enumerable) phone token; legacy-group creator-phone redaction with an exact no-leak test; agent preserved in redacted output; the log->tracing bridge PII gap (raw JIDs/addresses in legacy logs are now redacted); and the example path/wiring docs.
Out of scope (deliberate follow-ups): a metrics layer (counters/histograms for rates and percentiles) is a separate capability from tracing spans; and the pre-existing
modify_devicenote inprekeys.rsis unrelated to observability.