Repository navigation
Conversation
pblazej
added
the
internal
622c20e to
fe4f566
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 4 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| .with_attribute( | ||
| "lk.device.battery.level", | ||
| self.battery_level.unwrap_or(0) as i64, | ||
| ) | ||
| .with_attribute("lk.device.battery.charging", self.battery_charging), |
There was a problem hiding this comment.
🔴 Battery events omit their declared attributes
When battery state changes, change_events emits a percent integer and boolean instead of the declared charge fraction and state. Consumers of hw.battery.charge and hw.battery.state cannot read the battery event.
| .with_attribute( | |
| "lk.device.battery.level", | |
| self.battery_level.unwrap_or(0) as i64, | |
| ) | |
| .with_attribute("lk.device.battery.charging", self.battery_charging), | |
| .with_attribute( | |
| "hw.battery.charge", | |
| self.battery_level.unwrap_or(0) as f64 / 100.0, | |
| ) | |
| .with_attribute( | |
| "hw.battery.state", | |
| if self.battery_charging { "charging" } else { "discharging" }, | |
| ), |
Was this helpful? React with 👍 or 👎 to provide feedback.
| let nominated = | ||
| matches!(stat.members.get("nominated"), Some(AttributeValue::Bool(true))) | ||
| || text(stat, "state").as_deref() == Some("succeeded"); | ||
| if nominated { | ||
| index.pair_rtt_ms = num(stat, "currentRoundTripTime").map(|s| s * 1000.0); |
There was a problem hiding this comment.
🟡 Inbound RTT comes from an inactive pair
If an old succeeded candidate pair precedes the nominated pair, Index::new selects its RTT. Every inbound sample then reports the old path's RTT.
Learn more
A WebRTC report can contain multiple successful candidate pairs after connectivity checks or an ICE switch. state = succeeded does not mean that the pair carries traffic. Because the index takes the first qualifying entry, the RTT depends on report ordering rather than the active path.
Example: Pair A is succeeded with RTT 300 ms and pair B is nominated with RTT 40 ms. If A appears first, inbound samples carry 300 ms.
Recommended fix: Resolve the selected pair from transport's selectedCandidatePairId when present; otherwise prioritize nominated pairs and use a succeeded pair only as a documented fallback.
Was this helpful? React with 👍 or 👎 to provide feedback.
| .fold(None, |m, v| Some(m.map_or(v, |m: f64| m.max(v)))); | ||
| sample.quality_limitation_bandwidth_ms = | ||
| max_u64(layers.values().map(|l| l.limitation_bandwidth_ms)); | ||
| sample.quality_limitation_cpu_ms = max_u64(layers.values().map(|l| l.limitation_cpu_ms)); |
There was a problem hiding this comment.
🟡 Simulcast CPU pressure goes undetected
When a smaller layer's CPU-limitation counter grows below another layer's value, fold_layers keeps the unchanged maximum. track_limitation sees no increase, so the cadence never stretches for that CPU pressure.
Learn more
Each layer has its own cumulative CPU-limitation duration. Taking the maximum discards growth on any layer whose counter remains below that maximum. The cadence detector compares successive folded values, so it cannot detect this growth.
Example: One layer remains at 1000 ms while a second grows from 100 to 200 ms. The folded value remains 1000 ms, and CPU pressure is missed.
Recommended fix: Detect growth per layer before folding, or combine counters in a way that preserves each layer's positive deltas while handling resets and vanished layers.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #[derive(Default)] | ||
| pub(crate) struct StatsWindows { |
There was a problem hiding this comment.
🔴 RTC samples cannot enter telemetry
SDKs can construct RtcStatsSample, but StatsWindows is private and no public API accepts samples. SDK-provided stats cannot become telemetry events.
Learn more
The accumulator is not exposed to callers, and neither a pipeline nor a scope ingestion method exists in the current crate. Thus RtcStatsSample is only a data type in the public API; external users cannot record its readings or obtain windowed events.
Example: An SDK creates an inbound audio sample with bytes = Some(1000). It cannot submit that sample to a public telemetry object to produce lk.rtc.stats.sample.
Recommended fix: Provide a public ingestion method that routes the sample to the owning session's StatsWindows, and wire closing and retirement into the pipeline lifecycle.
Was this helpful? React with 👍 or 👎 to provide feedback.
`DeviceState` is what the host observes (thermal, power, memory, battery, network, lifecycle); the core derives the `lk.device.*.changed` events, a cadence factor of up to 4× under pressure, and soft and hard upload holds. `DeviceEvent` files audio route changes, interruptions and capture failures with shared enums. RTC stats map one raw `getStats()` report (per track or per peer connection) to samples, fold simulcast layers into one series per track, and window them on device into `lk.rtc.stats.sample`, per track, direction and session.
fe4f566 to
ec765e3
Compare
This branch has not been deployed
1 participant
Summary
What the host reports and what the core does with it. The device contract, row by row with its tests, is in #1485.
Changes
DeviceState(thermal, power, memory, battery, network, lifecycle): thelk.device.*.changedevents, a cadence factor of up to 4×, and soft and hard upload holdsDeviceEvent: audio route changes, interruptions and capture failuresgetStats()report per track or peer connection mapped to samples, simulcast layers folded into one series, windowed on device intolk.rtc.stats.sampleSPEC.md: the cadence policyVerification
At
fe4f566f, from a clean checkout (CI's test workflow runs only for PRs intomain, so these were run locally; there is no clippy job in CI):cargo fmt -- --checkcargo clippy -p livekit-telemetry --all-targets --all-features -- -D warningscargo check -p livekit-telemetry --all-targets --no-default-featureswith features[],[net],[uniffi],[net,uniffi]cargo test -p livekit-telemetry: 51 unit;--all-features: 52 unitcargo doc -D warningsreports links to items #1483 adds (Telemetry::stats,Telemetry::with_cache,Telemetry::set_device_state,Exporter), plus what it reports on6aba1b68too (private-item links,ExportError::from_response).