Conversation
`global::install` makes one pipeline the process's and starts the platform's instruments on it; `scope`, `emit`, `log`, `device_event`, `set_device_state` and `stats` reach it from anywhere and no-op until it is installed. `global::disable` is the opt-out: before it returns the pipeline is removed, its instruments stopped and every generation revoked, so nothing is captured, cached or sent again and later installs are refused; the future it returns deletes everything still held.
One test per row of the device contract: crashes and kills at every storage step, full or vanished storage, offline and constrained networks, background and suspension, clock changes, device pressure, the opt-out at every point of a capture and upload, and the lifecycle of the process pipeline and its instruments.
bd2245f to
172d65f
Compare
There was a problem hiding this comment.
Devin Review found 2 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| let generations: Vec<Generation> = | ||
| GENERATIONS.lock().unwrap_or_else(|e| e.into_inner()).drain(..).collect(); | ||
| generations | ||
| .into_iter() | ||
| .filter_map(|g| Some((g.shared.upgrade()?, g.commands))) |
There was a problem hiding this comment.
🔴 Repeated opt-out bypasses pending purge
When disable() runs twice before the first purge finishes, the second call returns success without awaiting it. GENERATIONS was drained by the first call, so cached batches and exporters can remain active after the second returns.
Learn more
The opt-out has a synchronous phase that revokes all generations and an asynchronous phase that clears storage and stops exporters. The first disable() moves all entries out of GENERATIONS before its returned future is polled. A second disable() therefore returns a future with no work, even when the first future is pending. Awaiting that second future does not establish that the purge has finished.
Example: Call let first = disable() while a disk-backed pipeline has cached records; call disable().await before polling first. The second call returns true with the cache still on disk, rather than waiting for its deletion.
Recommended fix: Track the active purge as shared lifecycle state. Make subsequent disable() futures join its completion and return its final result; ensure this also works when a prior future is dropped.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let previous = SHARED | ||
| .write() | ||
| .unwrap_or_else(|e| e.into_inner()) | ||
| .replace(Installed { telemetry, instruments: instruments.clone() }); | ||
| if let Some(previous) = &previous { | ||
| for instrument in &previous.instruments { | ||
| instrument.stop(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Old instrument records reach new pipeline
When install() replaces a pipeline, an outgoing instrument's stop() can emit into the replacement. SHARED already points to the new pipeline, so shutdown records acquire the wrong session and destination.
Learn more
Global capture functions look up the currently installed telemetry through current. install switches that pointer before calling the outgoing instruments' stop methods. A stop method that forwards a final event using the global API therefore sends it to a different generation, whose project and session may differ.
Example: A device instrument calls global::device_event during stop() as project A is replaced with project B. Its final event goes into B's pipeline rather than A's.
Recommended fix: Stop the outgoing instruments while the outgoing pipeline is still current, then publish the replacement and start its instruments, retaining the lifecycle lock across that transition.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
global, the process-wide pipeline, and the opt-out. From herelivekit-telemetryis exactly what #1396 ships. Every link below points at the stage that adds the test (the UniFFI ones at #1396).Changes
global: one pipeline per process, installed once and reachable from anywhere, with the platform's instruments started on itglobal::disable: the opt-out, in effect before it returnsdevice_tests.rs: the device contract, one test per rowdead_codeallowances removedDevice contract: every failure scenario → behaviour → max lost → why → test (32 rows)
What can go wrong on a device, and the most it can cost. OTel specifies no client persistence or device policy, so every row is custom. U = records not yet committed (≤ one flush interval, ≤ 2 048 queued) + open spans (≤ 256) + open RTC windows; B = one batch (≤ 512 records); E = records evicted or expired, always counted. N = records committed to the cache and not yet sent (≤ 4 MiB compressed, ≤ 512 batches, ≤ 24 h old). A batch is committed when the cache's
pushreturns (filefsynced, renamed, directoryfsynced on Unix); crash points are driven deterministically by test-only fault hooks around every write, fsync, rename, delete and publish.Max lost: 0 = nothing; 0 until limits = nothing until the cache's bounds (4 MiB, 512 files, 24 h) evict, then E; U, B, E, N as defined above. Every row is custom (no OTel spec covers client persistence or device policy); Why names the platform behaviour, spec or reason behind the row.
entering_background_flushes_immediatelyentering_the_background_flushes_even_with_the_allowance_spenta_crash_loses_only_what_was_not_yet_cachedbatch_is_written_before_upload_and_replayed_on_next_startpre_connect_records_replay_after_a_restarta_crash_between_the_answer_and_the_delete_resends_the_batch_oncea_batch_in_flight_at_a_crash_is_sent_again_once.tmp+fsync+ rename; a torn.tmpremoved at openrename(2)is atomic;fsync(2)first, so a crash never exposes a torn filea_write_killed_half_way_leaves_no_partial_batchfile_cache_evicts_oldest_beyond_max_bytes_and_drops_stray_tmpa_split_crashed_at_every_step_loses_and_duplicates_nothinga_split_failing_after_its_commit_point_is_kept_and_purgeablea_second_cache_on_the_directory_never_breaks_a_split_in_progressa_stuck_journal_is_retried_after_a_write_not_every_listinga_crash_while_binding_still_replays_to_the_rooms_projectcorruptcorrupt_and_stray_cache_files_are_dropped_and_countedan_inaccessible_batch_is_kept_not_counted_corrupta_failed_delete_after_acceptance_is_not_sent_againan_accepted_batch_awaiting_its_delete_is_not_rebound_and_resentmax_cache_bytes, oldest evicted and counted), still uploaded;cache.write_errors; a split that cannot commit keeps the parentENOSPCis ordinary on phones, iOS may purge Caches while the app runs, a failedfsync(2)leaves durability unknowna_full_disk_keeps_batches_in_memory_and_says_soa_failed_directory_sync_keeps_the_batch_in_memory_not_half_on_diskdurability_failures_are_errors_not_silent_successesthrottledduring a server pause)cache_eviction_is_counted_as_a_dropfile_cache_caps_the_number_of_batchesa_hold_longer_than_the_cache_reports_what_it_costbatches_older_than_a_day_are_dropped_and_counted_at_startbatches_expire_while_the_app_runsa_clock_jump_does_not_expire_a_fresh_backlogdevice_holds_record_without_attemptinga_device_change_during_a_pass_stops_its_remaining_requestsa_soft_hold_turning_hard_does_not_spindevice_holds_record_without_attemptingthe_soft_hold_cap_is_scheduled_and_splits_count_against_itdevice_state_emits_change_events_and_stretches_cadencecpu_limitation_counter_drives_cadence_pressurerelief_brings_the_pending_tick_forwardunknown_thermal_and_low_power_are_silent_and_neutraluploads_hold_while_connecting_but_never_beyond_the_captyped_spans_hold_uploads_while_connecting_and_export_when_endeddeadlines_are_served_while_a_request_is_outexport_timeout_ms, then cancels the request on the wire and stops; the rest stays committed; the device state pushed at install always ships byflush+shutdownexport_timeout_msthe_initial_device_state_always_ships_by_flush_and_shutdownshutdown_during_a_slow_backlog_is_bounded_and_joins_the_exportershutdown_drains_every_finished_span_batchshutdown_leaves_a_session_summarymaxQueueSize; drop-oldest keeps the freshest contextin_memory_bounds_count_every_dropqueue_overflow_is_counted_by_reasonflood_guard_caps_events_but_not_stats_windowsnotifications_within_an_interval_never_exceed_one_allowancethe_allowance_applies_only_while_a_room_is_in_a_calllivekit*targets, neverlivekit_telemetry*) wherever the platform callslog_forward_bootstrap(Swift's defaultOSLoggerwithffi: truedoes), whatever level it passes — that level filters only the console forwarding; platforms must not feed forwarded Rust entries totelemetry_log; any compact JWT (three base64url segments decoding to JSON, whatever the header's encoding, also inside punctuation such as(...<jwt>)) masked as<jwt>, bearer andtokencredential values (token=,"token": "…", quoted or spaced) as<redacted>; console entry unchangedonly_the_cores_own_warnings_and_errors_are_copied(added in #1396)the_platforms_level_filters_the_console_not_the_telemetry_copy(added in #1396)tokens_in_copied_records_are_masked(added in #1396)the_console_entry_is_what_it_always_was(added in #1396) (lands in #1396)Retry-After/RetryInfopause is held in memory only: after a relaunch each project gets one request (once its token is handed over) before a new 429/503 pauses it againa_failing_server_never_costs_a_batchtelemetry_disable()returns (no scope, no capture; a later configure only purges its storage dir and starts no exporter), the purge then runs on the core's runtime (a repeated opt-out joins the one already running);telemetry_is_disabled()istrueprocess-wide before the call returns, so a platform with per-isolate state checks it before eachgetStatsor submit; lifecycle serialized with instruments; every generation revoked under the locks that commit data, its exporter cancelled and awaited (draining ones too), queue/spans/windows/cache purged, later configures refused; only actual deletions counted; incomplete deletes reportedthe_opt_out_is_in_effect_before_its_purge_runsan_opt_out_counts_the_halves_of_an_unpublished_splitopting_out_stops_collection_and_purges_everythingopting_out_during_an_upload_resurrects_nothinga_capture_racing_the_opt_out_leaves_nothing_behindan_instrument_never_starts_after_the_opt_out_stopped_itthe_opt_out_cancels_a_draining_generation_before_returningthe_opt_out_withdraws_a_replaced_generations_pulled_request(added in #1396)a_repeated_opt_out_waits_for_the_first_purge(added in #1396)after_the_opt_out_nothing_is_capturedan_incomplete_purge_is_reportedpurges_count_only_actual_deletions_onceclearing_an_unreadable_directory_failsthe_process_pipeline_no_ops_until_installed(lands in #1396)next()ortry_next(); unknown ids ignoredtry_next_serves_without_waiting_and_never_a_stale_request(added in #1396)a_suspended_host_is_never_served_stale_requests(added in #1396)a_cancelled_request_is_never_served(added in #1396)finishing_discards_what_is_queued(added in #1396) (lands in #1396)invalidcustom_data_is_room_scoped_validated_and_snapshottedrtc_windows_carry_the_attributes_of_their_readingsa_window_closed_before_a_change_keeps_its_snapshot_whenever_it_is_queueda_project_change_splits_the_rtc_windows_by_projecttimed_outon the core's clock, whoever looks first, kept through disconnectthe_subscribe_span_is_owned_by_the_corelate_first_media_is_timed_out_whoever_looks_firsta_subscribe_past_its_deadline_stays_timed_out_through_disconnectinvalid, detached spans included)a_pending_subscribe_never_keeps_the_pipeline_aliveper_track_state_is_retired_with_the_trackthe_same_track_in_two_sessions_never_mergesretained_span_state_is_boundedfinal_limits_hold_after_decoration_and_caller_strings_are_boundedUninstall or an OS cache purge is beyond the SDK's reach (N + U). Defaults, polling (fast after a subscribe or a publish) and explicit flush (drains without the per-pass budget, within holds and pauses):
defaults_export_and_window_once_a_minutethe_core_paces_stats_pollingpublishing_polls_fast_until_the_first_outbound_readingdevice_holds_uploads_and_a_backlog_replays_within_the_budget.Verification
At
172d65f8, 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: 175 unit, 2 doc;--all-features: 178 unit, 2 doccargo doc -D warningsreports exactly what it reports on6aba1b68(private-item links,ExportError::from_response).