Skip to content

refactor(realtime): introduce RealtimeObservability orchestrator (PR 1 of 3)#132

Merged
VerioN1 merged 4 commits into
mainfrom
alon/livekit-pr1-observability
May 15, 2026
Merged

refactor(realtime): introduce RealtimeObservability orchestrator (PR 1 of 3)#132
VerioN1 merged 4 commits into
mainfrom
alon/livekit-pr1-observability

Conversation

@VerioN1
Copy link
Copy Markdown
Contributor

@VerioN1 VerioN1 commented May 11, 2026

Summary

  • First of three PRs splitting the LiveKit migration on alon/livekit-prod into reviewable chunks.
  • Moves diagnostics.ts + telemetry-reporter.ts into a new observability/ folder; deletes root webrtc-stats.ts in favor of an expanded observability/webrtc-stats.ts.
  • Adds RealtimeObservability — a single orchestrator that owns telemetry reporting, stats collection, and diagnostic emission. webrtc-manager.ts + webrtc-connection.ts migrate onto it.
  • WebRTCStats gains ~20 standard RTCStats fields (NACK/PLI/FIR counts, jitter buffer averages, simulcast accumulation, remote-inbound, selectedCandidatePairs). Strictly additive — every prior field name is preserved.
  • No LiveKit code. No new dependencies. No public API break — all exported names from index.ts keep their original identifiers (only the internal re-export paths change).

Why this PR exists in isolation

The thesis is "the new observability abstraction is sound, and aiortc already uses it." A reviewer can validate the abstraction by reading one consumer (aiortc) without needing to also evaluate LiveKit, new transports, or any client-side wiring rewrite. PRs 2 and 3 build on this foundation.

Test plan

  • pnpm exec tsc --noEmit — clean.
  • pnpm test — 161/161 pass.
  • pnpm exec biome check src/ tests/ — clean for changed files.
  • Manual smoke against staging realtime endpoint to confirm telemetry payloads still land in platform.decart.ai/api/v1/telemetry.

Note

Medium Risk
Touches realtime connection/reconnect and telemetry/stats plumbing; while largely a refactor, expanded stats parsing and new lifecycle wiring could change when/what events and telemetry are emitted.

Overview
Introduces RealtimeObservability as a single owner for diagnostic emission, telemetry reporting, WebRTC stats polling, and video-stall detection, and rewires realtime connect/subscribe, WebRTCManager, and WebRTCConnection to use it instead of ad-hoc callbacks.

Moves diagnostics/telemetry/stats code under realtime/observability/, adds stats events to the subscribe client, and replaces the old webrtc-stats.ts with an expanded collector that includes additional RTCStats fields (e.g., jitter-buffer/decode metrics, NACK/PLI/FIR, simulcast-aware outbound aggregation, remote-inbound, and selected ICE candidate pairs) while preserving existing field names and SDK export identifiers.

Reviewed by Cursor Bugbot for commit 1bacb33. Bugbot is set up for automated code reviews on this repo. Configure here.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 11, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@decartai/sdk@132

commit: 1bacb33

Comment thread packages/sdk/src/realtime/client.ts
Comment thread packages/sdk/src/realtime/observability/webrtc-stats.ts Outdated
Comment thread packages/sdk/src/realtime/client.ts Outdated

let webrtcManager: WebRTCManager | undefined;
const observability = new RealtimeObservability({
telemetryEnabled: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this disabled on purpose?

this.observability =
callbacks.observability ??
new RealtimeObservability({
telemetryEnabled: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

callbacks.observability ??
new RealtimeObservability({
telemetryEnabled: false,
apiKey: "",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why empty

this.observability =
config.observability ??
new RealtimeObservability({
telemetryEnabled: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

config.observability ??
new RealtimeObservability({
telemetryEnabled: false,
apiKey: "",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

VerioN1 pushed a commit that referenced this pull request May 13, 2026
## Summary
- Propagate the SDK `telemetryEnabled` option through the realtime
subscribe path instead of forcing telemetry off.
- Require injected `RealtimeObservability` in `WebRTCManager` /
`WebRTCConnection`, removing the hard-coded disabled observability
fallback with an empty API key.
- Rename inter-frame delay `Variance` stat to `StdDev` to match the
actual computation.

## Checks
- `pnpm --filter @decartai/sdk typecheck`
- `pnpm --filter @decartai/sdk format:check` (passes with existing
warnings/info)
- `pnpm --filter @decartai/sdk test -- --run`
- `pnpm --filter @decartai/sdk build` (passes; existing tsdown warning
about `define` option)
- Independent Claude Code review of diff vs
`origin/alon/livekit-pr1-observability`

Base for #132.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes realtime telemetry behavior (subscribe path now honors
`telemetryEnabled`) and makes `WebRTCManager`/`WebRTCConnection` require
an injected `RealtimeObservability`, which can be a breaking change for
any consumers constructing these classes directly. Renaming a
`WebRTCStats` field also risks downstream type/runtime expectations.
> 
> **Overview**
> Realtime subscribe sessions now honor the SDK’s `telemetryEnabled`
option and immediately call `observability.sessionStarted(sid)` so
telemetry/diagnostics are associated with the session.
> 
> `WebRTCManager` and `WebRTCConnection` no longer create an internal
fallback `RealtimeObservability` (previously hard-coded telemetry off
with an empty API key); callers must pass `observability` explicitly,
and unit tests are updated accordingly.
> 
> In `WebRTCStats`, the inter-frame delay metric is renamed from
`interFrameDelayVarianceMs` to `interFrameDelayStdDevMs` to match the
actual std-dev calculation.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
16388f0. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Comment thread packages/sdk/src/realtime/client.ts
Comment thread packages/sdk/src/realtime/observability/realtime-observability.ts Outdated
VerioN1 and others added 2 commits May 14, 2026 11:42
…1 of 3)

Move diagnostics/telemetry/stats into a single observability/ folder.
Adds RealtimeObservability orchestrator that owns telemetry reporting,
stats collection, and diagnostic emission. WebRTCStats gains ~20 standard
RTCStats fields. aiortc (webrtc-manager + webrtc-connection) migrates onto
the orchestrator.

No LiveKit code, no new dependencies, no public API break.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Propagate the SDK `telemetryEnabled` option through the realtime
subscribe path instead of forcing telemetry off.
- Require injected `RealtimeObservability` in `WebRTCManager` /
`WebRTCConnection`, removing the hard-coded disabled observability
fallback with an empty API key.
- Rename inter-frame delay `Variance` stat to `StdDev` to match the
actual computation.

- `pnpm --filter @decartai/sdk typecheck`
- `pnpm --filter @decartai/sdk format:check` (passes with existing
warnings/info)
- `pnpm --filter @decartai/sdk test -- --run`
- `pnpm --filter @decartai/sdk build` (passes; existing tsdown warning
about `define` option)
- Independent Claude Code review of diff vs
`origin/alon/livekit-pr1-observability`

Base for #132.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes realtime telemetry behavior (subscribe path now honors
`telemetryEnabled`) and makes `WebRTCManager`/`WebRTCConnection` require
an injected `RealtimeObservability`, which can be a breaking change for
any consumers constructing these classes directly. Renaming a
`WebRTCStats` field also risks downstream type/runtime expectations.
>
> **Overview**
> Realtime subscribe sessions now honor the SDK’s `telemetryEnabled`
option and immediately call `observability.sessionStarted(sid)` so
telemetry/diagnostics are associated with the session.
>
> `WebRTCManager` and `WebRTCConnection` no longer create an internal
fallback `RealtimeObservability` (previously hard-coded telemetry off
with an empty API key); callers must pass `observability` explicitly,
and unit tests are updated accordingly.
>
> In `WebRTCStats`, the inter-frame delay metric is renamed from
`interFrameDelayVarianceMs` to `interFrameDelayStdDevMs` to match the
actual std-dev calculation.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
16388f0. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@VerioN1 VerioN1 force-pushed the alon/livekit-pr1-observability branch from 88f9a0f to 0f5169d Compare May 14, 2026 08:45
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7fad891. Configure here.

Comment thread packages/sdk/src/realtime/client.ts
@VerioN1 VerioN1 force-pushed the alon/livekit-pr1-observability branch from 0f599c7 to 1bacb33 Compare May 14, 2026 16:23
@VerioN1 VerioN1 merged commit cc1029c into main May 15, 2026
5 checks passed
@VerioN1 VerioN1 deleted the alon/livekit-pr1-observability branch May 15, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants