Skip to content
57 changes: 55 additions & 2 deletions agent_docs/observability.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ must follow.
- **No PII.** Snapshots and reports carry numbers only, never JIDs/phone
numbers, matching the `wacore::telemetry` label rules.

## The three surfaces
## The four surfaces

### 1. `Client::stats()` — wire I/O counters (always on)

Expand Down Expand Up @@ -104,7 +104,60 @@ resource teardown panics, publication failures, and queue drops mark only the
responsible plugin as degraded. Concurrent snapshots are intentionally
approximate, and carry no message content, JIDs, or phone numbers.

### 3. `BotBuilder::with_task_instrument` — CPU / custom attribution (opt-in)
### 3. `Client::device_memo_stats()` — group-path memo outcomes (always on)

`DeviceMemoStats`: per-term hit/miss counts for the two device-list memos a
group send depends on, `resolve_group_devices_memoized` and
`resolve_skdm_targets_memoized`. Cumulative for the client's lifetime;
`DeviceMemoStats::since` subtracts an earlier snapshot to scope a workload.

The reason it is per-term rather than a hit/miss pair: the group memo has three
validity terms (entry present, `GroupInfo` `Arc` identity, topology generation
— with a scoped re-stamp between the last two) and the SKDM memo has four
(device `Arc`, sender-key-map `Arc`, map generation, sending identity). An
aggregate "N misses" cannot separate an in-place cold flip from a metadata
refresh from a memo that was never stored, and those have different fixes. It
also cannot separate cause from consequence: the SKDM memo compares the `Arc`
that the group memo returned, so **a group-memo recompute forces
`skdm_targets.miss_devices` no matter what**. Read the group half first.

Two counters do not fit the "one per call" shape and are documented as such:
`restamps` (served like a hit, but paid the `unchanged_for` scan first) and
`not_stored` (a resolution whose target set was neither empty nor
own-devices-only, so nothing was memoized). `not_stored` guarantees the next
call **cannot hit** — not that it reports `miss_absent`. A stale entry that was
already there is deliberately left in place, because it can never become valid
again (the sender-key map generation only moves forward, the map `Arc` is
replaced wholesale on a rebuild, and the device-set `Weak` keeps the old
allocation alive so no `ptr::eq` can spuriously match), so the next call
reports whichever term is still failing. Reading a run of `not_stored` as
eviction pressure is therefore the wrong conclusion: it means the group is not
settling into the warm steady state at all.

Every other SKDM outcome is exactly one per call, including `resolve_failed`,
which covers the calls that never reached a memo term because the device
resolution they depend on errored. Without it `hit_rate()` would look healthy
over a denominator that quietly shrank as sends started failing.

Why always-on rather than `#[cfg(test)]` like `dm_devices_memo_recomputes`: a
test counter answers the question in a fixture, and the question here is what a
*deployed* client gets — an embedder whose registry writes are noisier than any
fixture's would have no way to see its own hit rate. It costs one indexed
relaxed `fetch_add` per resolver call, twice per group send. Measured against
the tightest thing the counters sit inside (SKDM target resolution on the
memo-hit path, callgrind, min of 3, K=10001 so the fixture's setup jitter
divides away): **+16 Ir per resolve at 8 members, +25 at 512**, against 4,419
and 4,408 without them. At whole-send scale it is under the fixture's own
run-to-run spread.

Record the outcome **on the branch that decided it**. An earlier revision
classified into an enum and then matched on it again to act; that second
dispatch, plus moving the SKDM memo entry (a five-field tuple carrying a `Jid`
and a `Vec<Jid>`) into a temporary to classify it, cost 126 Ir per resolve
instead of 16. A counter meant to be free on the hit path has to be written
that way.

### 4. `BotBuilder::with_task_instrument` — CPU / custom attribution (opt-in)

`wacore::stats::TaskInstrument` is an object-safe enter/exit hook called
around every poll of the client's internal tasks and around its blocking
Expand Down
14 changes: 10 additions & 4 deletions benches/client_group_send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@
//! - **LID addressing.** The fixture is PN-addressed, so
//! `GroupInfo::phone_jid_for_lid_user` is never reached. A LID sweep needs a
//! registry seeded with LID↔PN mappings, which is a different fixture rather
//! than a parameter on this one.
//! than a parameter on this one. The *hit-rate* question is covered for LID
//! groups by `repeat_lid_group_sends_hit_both_device_memos_on_every_send`
//! in `src/send/mod.rs`, which is what settles whether that function is on
//! a warm send's bill at all — it runs only inside the uncached resolve.
//! - **The SQLite backend.** The fixture stores through `InMemoryBackend`, so
//! the storage engine's own per-send cost is excluded by construction.
//! - **First contact.** The fixture reaches its steady state before measuring,
Expand Down Expand Up @@ -120,9 +123,12 @@ fn skdm_target_resolution_warm(bencher: divan::Bencher, group_size: usize) {
/// `skdm_target_resolution_warm` is what the memo is worth per send. Measuring
/// both is what separates "the filter is expensive" from "the filter runs when
/// it should not" — only the second is a bug, and only a hit-rate observation
/// can tell them apart. That the steady state takes the memoized path is pinned
/// as a test (`skdm_warm_memo_hits_on_every_repeat_send`), not asserted here: a
/// benchmark measures how much it costs, not how often it happens.
/// can tell them apart. Which one the steady state takes is not asserted here
/// (a benchmark measures how much an outcome costs, not how often it happens):
/// it is pinned by `skdm_warm_memo_hits_on_every_repeat_send` and, per term,
/// by `repeat_group_sends_hit_both_device_memos_on_every_send`. At group sizes
/// no unit test builds, read `GroupSendHarness::memo_stats` after a run of
/// `warm_send`.
#[divan::bench(args = GROUP_SIZES, sample_count = SAMPLE_COUNT, sample_size = SAMPLE_SIZE)]
fn skdm_target_resolution_memo_cold(bencher: divan::Bencher, group_size: usize) {
let harness = shared("skdm_target_resolution_memo_cold", group_size);
Expand Down
11 changes: 11 additions & 0 deletions src/bench_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,17 @@ impl GroupSendHarness {
.expect("target resolution must succeed offline")
}

/// Per-term outcomes of the two device memos so far.
///
/// The benchmarks below force their memo outcome, which is what makes them
/// measurable — and what makes them unable to say which outcome a client
/// takes once the group is in its steady state. Reading the counters after
/// a run of [`Self::warm_send`] is how the sweep answers that at group
/// sizes the unit tests do not reach.
pub fn memo_stats(&self) -> crate::client::DeviceMemoStats {
self.client.device_memo_stats()
}

/// A signal-cache flush over the cached session set this group produced.
///
/// Repeated calls flush an already-clean cache, which is the interesting
Expand Down
12 changes: 12 additions & 0 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub(crate) use app_state::{BatchedSyncOutcome, BatchedSyncRequest, CriticalSyncP
pub(crate) use app_state::{SyncHolder, batched_sync_outcome_tests::batch_result};
mod builder;
mod context_impl;
mod device_memo_stats;
mod device_registry;
pub(crate) mod device_topology;
#[cfg(feature = "client-lifecycle")]
Expand All @@ -22,6 +23,10 @@ mod sessions;
mod voip;
use builder::{ClientAssembly, ClientExtensions};
pub use builder::{ClientBuild, ClientBuilder, ClientBuilderError};
pub(crate) use device_memo_stats::{
DeviceMemoCounters, GroupDevicesMemoOutcome, SkdmTargetsMemoOutcome,
};
pub use device_memo_stats::{DeviceMemoStats, GroupDevicesMemoStats, SkdmTargetsMemoStats};
#[cfg(feature = "client-lifecycle")]
use extension_lifecycle::LifecycleRegistration;
#[cfg(feature = "client-lifecycle")]
Expand Down Expand Up @@ -1566,6 +1571,13 @@ pub struct Client {
#[cfg(test)]
pub(crate) dm_devices_memo_recomputes: AtomicU64,

/// Per-term hit/miss counts for the two group-path device memos above,
/// read through [`Client::device_memo_stats`]. Which term invalidated is
/// the only thing that distinguishes "this memo is doing its job" from
/// "this memo has never hit", and no benchmark can observe it: a fixture
/// that forces the outcome measures the cost of the outcome it forced.
pub(crate) device_memo_counters: DeviceMemoCounters,

/// Single-flight for cold SKDM distribution, keyed per group. Concurrent
/// cold sends each re-ran the full per-member fan-out before any of them
/// marked the devices warm; the loser now waits here and re-resolves,
Expand Down
Loading
Loading