From 0c1befa79ced76b3e7a5ea33e50224b9b7d18f40 Mon Sep 17 00:00:00 2001 From: balazskreith Date: Wed, 14 Oct 2026 23:02:52 +0300 Subject: [PATCH 1/9] feat: refactor sink management in Observer - Introduced SinkRegistry to manage per-client sinks, allowing for better lifecycle management and unique identification of sinks across observers. - Replaced createClientSink with createClientJsonlSink for writing client samples to JSONL files, enhancing file management and simplifying the API. - Updated Observer to utilize the new sink management system, ensuring sinks are created and managed under the appropriate client IDs. - Refactored existing sinks (ClientSampleSink, JsonlFileSink, InMemorySink) to implement a unified Sink interface, improving consistency and extensibility. - Added tests for the new sink management features, ensuring proper functionality and integration within the Observer. --- CHANGELOG.md | 137 ++++++++++++++++++ README.md | 194 ++++++++++++++----------- examples/client-summaries.ts | 251 ++++++++++++++++++++++++++++++++ package.json | 1 + src/ObservedCall.ts | 23 ++- src/ObservedClient.ts | 26 ++-- src/ObservedMediasoupRouter.ts | 15 ++ src/Observer.ts | 41 +++++- src/ObserverEvents.ts | 4 +- src/index.ts | 13 +- src/sinks/ClientSampleSink.ts | 63 ++------ src/sinks/InMemorySink.ts | 36 ++++- src/sinks/JsonlFileSink.ts | 77 +++++----- src/sinks/Sink.ts | 62 ++++++++ src/sinks/SinkRegistry.ts | 112 ++++++++++++++ src/sinks/index.ts | 11 +- src/utils/stats.ts | 11 +- tests/Observer.spec.ts | 17 ++- tests/injection.spec.ts | 6 +- tests/sinkRegistry.spec.ts | 257 +++++++++++++++++++++++++++++++++ tests/sinks.spec.ts | 47 +++++- 21 files changed, 1164 insertions(+), 240 deletions(-) create mode 100644 examples/client-summaries.ts create mode 100644 src/sinks/Sink.ts create mode 100644 src/sinks/SinkRegistry.ts create mode 100644 tests/sinkRegistry.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ada45..7d394fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,142 @@ # Changelog +## Unreleased + +### Sinks and `SinkRegistry` (new) + +Every observed object that has a lifecycle now owns a `sinks` registry, so any number of sinks can +be attached to it and are ended with it. The sinks are agnostic about what passes through them; +the library never polls — what is written, and when, is the application's. + +```ts +const sink = observedRouter.sinks.create(JsonlFileSink, { id: transport.id, path }); +observedRouter.sinks.write(transport.id, await transport.getStats()); // or sink.write(...) +// Ended when the router closes; call sink.end() to end it earlier. +``` + +- `Sink`: an interface — `id`, `write(record)`, `end()`, and the events `ended` / `close` / + `error` / `finish` / `drain`. `end()` must emit `ended` synchronously (that removes the sink from + its registry) and later writes must be refused. `JsonlFileSink` and + `InMemorySink` are the built-in implementations; `ClientSampleSink` is now the type + `Sink`. `SinkSettings` (`{ id }`) is what `create` passes to a sink's constructor. +- `SinkRegistry`: `create(SinkClass, settings)` constructs `new SinkClass({ ...settings, id })` + and registers it (settings typed from the constructor; id generated when omitted), `get(id)`, `write(id, record)`, `clear()`, `size`, `keys()`, `values()`. There is no + `remove`: `sink.end()` removes a sink from its registry. The registry attaches an `error` + listener, so a failing sink is logged, not thrown. +- `observer.sinks` is the parent of every `observedCall.sinks` and `observedMediasoupRouter.sinks`. + **Sink ids are unique per observer**: `create` returns `undefined` for an id held by any registry + of the observer, before constructing anything (so no file is opened). A registry sees its own and + its descendants' sinks, so a parent's `get` / `write` / `clear` reach a child's sinks. +- Lifecycle: a router's sinks are ended when it closes, a call's when it closes, and + `observer.close()` ends every sink that is left, including those of routers that are still open + (the observer does not close routers). +- A client writes its samples to the sink under its client id in `observedCall.sinks` + (`observedClient.sink` is now a getter for it) and ends it when the client closes. Client ids + are therefore sink ids and must be unique across the observer's open calls. +- `createClientJsonlSink: ({ observedCall, clientId }) => string | undefined` (new `ObserverConfig` + option): return a path to write that client's samples to a `JsonlFileSink`. `client-sink-created` + announces the file (`sink: JsonlFileSink`). +- `JsonlFileSink` and `createJsonlFileSink` take any record type. + +**Breaking** (see *Migrating from 1.2* below): + +- `ObserverConfig.createClientSink` is removed. Use `createClientJsonlSink` for JSONL files, or + create your own sink under the client id on `client-added`. +- `ClientSampleSink` is no longer a class to extend: implement `Sink` (e.g. + `extends EventEmitter implements ClientSampleSink`), take `SinkSettings` in the constructor, and + emit `ended` from `end()`. `JsonlFileSink` and `InMemorySink` implement the interface instead of + extending a base class. `JsonlFileSinkOptions` gained `id`; `new InMemorySink(samples)` is now + `new InMemorySink({ id?, samples? })`. +- `createJsonlFileSinkFactory`, `JsonlFileSinkFactoryOptions` and `ClientSampleSinkFactory` are + removed (`createClientJsonlSink` replaces the factory). +- `client-sink-created` now carries a `JsonlFileSink` and fires only for `createClientJsonlSink` + files. +- `ObservedClient.sink` is a getter (read-only, as before). + +#### Migrating from 1.2 + +**JSONL files per client** — `createClientSink: createJsonlFileSinkFactory(...)` becomes +`createClientJsonlSink`, which returns the path. The factory's default file name was +`${callId}__${clientId}.jsonl`; a `getFileName` option becomes the body of the callback. + +```ts +// 1.2 +new Observer({ createClientSink: createJsonlFileSinkFactory({ directory: './stats' }) }); + +// 1.3 +new Observer({ + createClientJsonlSink: ({ observedCall, clientId }) => path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`), +}); +``` + +A `serializeSample` option has no equivalent in `createClientJsonlSink`; create the sink yourself +as below, with `sinks.create(JsonlFileSink, { id, path, serializeSample })`. + +**Any other client sink** — return it no longer from `createClientSink`; create it under the client +id when the client is added. The client writes its samples to it and ends it on close, as before. + +```ts +// 1.2 +new Observer({ createClientSink: ({ clientId, observedCall }) => new HttpSink(urlOf(observedCall.callId, clientId)) }); + +// 1.3 +observer.on('client-added', ({ observedCall, observedClient }) => { + observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url: urlOf(observedCall.callId, observedClient.clientId) }); +}); +``` + +**Custom sink classes** — `class HttpSink extends ClientSampleSink` becomes an implementation of +the interface. The constructor receives the settings object passed to `create`, including `id`: + +```ts +// 1.2 +class HttpSink extends ClientSampleSink { + constructor(private readonly url: string) { super(); } + write(sample: ClientSample) { /* ... */ return true; } + end() { /* flush, then */ this.emit('close'); } +} + +// 1.3 +class HttpSink extends EventEmitter implements ClientSampleSink { + readonly id: string; + private readonly url: string; + private ended = false; + + constructor(settings: SinkSettings & { url: string }) { + super(); + this.id = settings.id; + this.url = settings.url; + } + write(sample: ClientSample) { + if (this.ended) return false; // refuse writes after end() + /* ... */ + return true; + } + end() { + if (this.ended) return; + this.ended = true; + this.emit('ended'); // required: removes it from its registry + /* flush, then */ this.emit('close'); + } +} +``` + +**`client-sink-created`** — still fires right after `client-added`, but only for the +`createClientJsonlSink` file, and its `sink` is a `JsonlFileSink` (no `instanceof` narrowing needed +to read `sink.path`). For a sink you create yourself, you already hold it. + +**Unchanged:** `observedClient.sink` still returns the client's sink, and the client still ends it +when it closes. + +### Other + +- `percentileOfHistogram` narrows the bucket holding the observed `min` / `max` to the observed + range before interpolating, so a distribution inside one bucket (40–49 ms in [40, 60)) is no + longer reported as its maximum. +- `examples/client-summaries.ts` (`yarn example:summaries`): a per-client record read off + `ObservedClient`, folded into the `CallSummary` with enrichers, including a call-level RTT p95 from + the clients' combined `rttHistogram`. + ## 1.2.0 — 2026-10-13 ### `ObservedClient` session totals diff --git a/README.md b/README.md index 92e7be4..8e01acd 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ and emits a single, unified stream of typed events the application can react to. 11. [Call summaries](#call-summaries) 12. [Remote track resolution (mediasoup / SFU)](#remote-track-resolution-mediasoup--sfu) 13. [Mediasoup router observation](#mediasoup-router-observation) -14. [Sinks (per-client sample persistence)](#sinks-per-client-sample-persistence) +14. [Sinks](#sinks) 15. [Injecting data into a client](#injecting-data-into-a-client) 16. [Logging](#logging) 17. [Design notes](#design-notes) @@ -83,14 +83,14 @@ yarn add @observertc/observer-js works the same whether your project is ESM or CommonJS — your import line is unchanged either way: ```ts -import { Observer, ClientSample, createJsonlFileSinkFactory } from '@observertc/observer-js'; +import { Observer, ClientSample, JsonlFileSink } from '@observertc/observer-js'; ``` In an ESM project this resolves to the `.mjs` build; in a CommonJS project (where TypeScript compiles your `import` down to `require()`) it resolves to the `.js` build. Everything is exported from the single `@observertc/observer-js` entry. Written in TypeScript; ships type declarations for both formats (`dist/index.d.ts` for `require`, `dist/index.d.mts` for `import`). Runtime -dependencies: `@bufbuild/protobuf`, `events`, `uuid`. The library does **not** bundle a logger or +dependencies: `@bufbuild/protobuf`, `uuid`. The library does **not** bundle a logger or any transport — see [Logging](#logging). `ClientSample` and friends are re-exported from this package, and are also published as the @@ -404,7 +404,7 @@ See [Mediasoup router observation](#mediasoup-router-observation) for the full d | Event | Extra | Fires when | |-------|-------|-----------| | `client-added` | — | a client is created | -| `client-sink-created` | `{ sink: ClientSampleSink }` | a per-client sink was created (only when `createClientSink` returns one); fires right after `client-added` | +| `client-sink-created` | `{ sink: JsonlFileSink }` | the client's JSONL sample file was created (only when `createClientJsonlSink` returns a path); fires right after `client-added` | | `client-updated` | `{ sample: ClientSample, elapsedTimeInMs: number, context?: AcceptContext }` | the client processed a sample | | `client-closed` | — | the client closed | | `client-joined` | — | first `CLIENT_JOINED` event seen | @@ -479,8 +479,8 @@ type ObserverConfig> = { // (incl. lazily by accept()). appData is application-owned; accept `context` never touches it. createCallAppData?: (p: { callId: string; observer: Observer; acceptCtx?: AcceptContext }) => Record; createClientAppData?: (p: { clientId: string; observedCall: ObservedCall; acceptCtx?: AcceptContext }) => Record; - // sink factory — produces a per-client sink that receives every accepted sample (see Sinks). - createClientSink?: (p: { clientId: string; observedCall: ObservedCall }) => ClientSampleSink | undefined; + // a JSONL file per client: return its path, or undefined for none (see Sinks). + createClientJsonlSink?: (p: { clientId: string; observedCall: ObservedCall }) => string | undefined; // remote-track-resolver factory — produces a call's RemoteTrackResolver (see Remote track resolution). createRemoteTrackResolver?: (observedCall: ObservedCall) => RemoteTrackResolver | undefined; }; @@ -591,7 +591,7 @@ Key members: - `readonly clientId: string`, `appData: AppData`, `readonly call: ObservedCall` - `readonly observedPeerConnections: Map` -- `readonly sink?: ClientSampleSink` — the per-client sink (see [Sinks](#sinks-per-client-sample-persistence)), if `createClientSink` is configured; listen on it for `close`/`error` +- `get sink(): Sink | undefined` — the sink under its client id in `call.sinks` (see [Sinks](#sinks)); listen on it for `close`/`error` - **Injection API** (queue app data to be merged into the next sample processing): `injectEvent(ClientEvent)`, `injectIssue(ClientIssue)`, `injectMetaData(ClientMetaData)`, `injectExtensionStat(ExtensionStat)`, `injectAttachment(attachments: Record)` @@ -1905,127 +1905,149 @@ observer.on('mediasoup-router-removed', ({ observedMediasoupRouter }: ObservedMe --- -## Sinks (per-client sample persistence) +## Sinks -A **sink** receives the samples a client accepts — for archival, streaming, or later offline -replay. Each `ObservedClient` gets its **own** sink, produced by the -`ObserverConfig.createClientSink` factory when the client is created (return `undefined` for no -sink). The client pushes every accepted sample to its sink, and `end()`s it on close. +A **sink** is a destination for records — client samples, a mediasoup transport's `getStats()`, +anything the application wants to keep. Sinks are agnostic about what passes through them; their +**lifecycle is owned by the object they are registered on**, through that object's `sinks` +registry. The library never polls: what is written, and when, is the application's. -### The `ClientSampleSink` base class +| Registry | Ended when | Typical use | +|----------|-----------|-------------| +| `observer.sinks` | the observer closes | worker- or process-level stats | +| `observedCall.sinks` | the call closes | each client's sample sink (under its client id), call-level stats | +| `observedMediasoupRouter.sinks` | the router closes | e.g. one file per pipe transport | -`ClientSampleSink` is an **abstract base class** (a typed `EventEmitter`). You create a sink by -**subclassing it** and implementing `write` and `end`. It is **object-mode**: `write` receives -the `ClientSample` *object*, so each sink decides how (or whether) to serialize it — JSON line, -protobuf, a remote POST body, an in-memory push, etc. +`observer.sinks` is the parent of every call's and router's registry. **Sink ids are unique per +observer**: creating a sink with an id that any registry of the observer already holds is refused +(`create` returns `undefined`, and nothing is constructed). A registry sees its own sinks and its descendants', so `observer.sinks.write(id, record)` +reaches any sink, and `observer.close()` ends every sink that is left — including those of routers +that are still open, since the observer does not close mediasoup routers. -```ts -import { ClientSampleSink, ClientSample } from '@observertc/observer-js'; +### `SinkRegistry` + +| Method | Does | +|--------|------| +| `create(SinkClass, settings)` | constructs `new SinkClass(settings)` and registers it under `settings.id` (generated when omitted); `undefined` if the id is taken | +| `get(id)` | the sink, if any (own or a descendant's) | +| `write(id, record): boolean` | writes to the sink; `false` if there is none, or on backpressure | +| `clear()` | ends every sink of the registry and its descendants | +| `size`, `keys()`, `values()` | own and descendants' sinks | + +There is no `remove`: **`sink.end()` closes a sink and removes it from its registry**, which frees +its id. The registry attaches an `error` listener to each sink, so a failing destination is logged +and never crashes the process. -abstract class ClientSampleSink /* extends EventEmitter */ { - abstract write(sample: ClientSample): boolean; // accept one sample; `false` = backpressure - abstract end(): void; // flush; emit `close` when the destination is ready +### The `Sink` interface + +```ts +import type { Sink, SinkSettings } from '@observertc/observer-js'; - // typed events (inherited): the listener signature is inferred from the event name - on(event: 'close' | 'finish' | 'drain', listener: () => void): this; - on(event: 'error', listener: (err: Error) => void): this; - // ...and the matching `once` / `off` / `emit` +interface Sink { // typically: class X extends EventEmitter implements Sink + readonly id: string; + write(record: T): boolean; // `false` once ended (nothing written), or on backpressure + end(): void; // idempotent; MUST emit `ended` synchronously, then flush and emit `close` + on / once / off(event, listener); // the events below } + +type SinkSettings = { id: string }; // what `create` passes to the constructor, with your own settings ``` +An implementation must emit `ended` from `end()` — that is how its registry removes it and frees +its id — refuse writes after it, and emit `close` / `error` for its destination. + | Event | Meaning | |-------|---------| +| `ended` | `end()` was called: no more writes, removed from its registry | | `close` | the destination is fully written and closed (e.g. a file flushed and its fd closed) — "ready" | | `error` | the destination failed | | `finish` | `end()` was processed and queued data flushed (before `close`) | | `drain` | the buffer drained after backpressure; safe to write more | -The library calls `write(sample)` **synchronously** per accepted sample (it is not awaited), -`end()`s the sink when the client closes, and attaches an `error` listener so a failing sink -can't crash the process (it also catches throws from `write`/`end`). The application — which -created the sink — listens for `close` (destination ready) and `error`. Because `write` isn't -awaited in the `accept()` hot path, **backpressure and batching are the sink's concern**. - -### Built-in sinks - -```ts -import { Observer, createJsonlFileSinkFactory } from '@observertc/observer-js'; - -const observer = new Observer({ - // one ./stats/__.jsonl per client - createClientSink: createJsonlFileSinkFactory({ directory: './stats' }), -}); - -// React when a sink is created for a client: -observer.on('client-sink-created', ({ observedClient, sink }) => { - sink.on('close', () => { - // the file is fully flushed and its fd closed — ready to upload, move, etc. - }); -}); -``` - -| Export | Signature | Notes | -|--------|-----------|-------| -| `createJsonlFileSinkFactory` | `({ directory, flags?, getFileName?, serializeSample? }) => ClientSampleSinkFactory` | per-client JSONL files; path defaults to `${callId}__${clientId}.jsonl` under `directory` (which **must exist**) | -| `createJsonlFileSink` | `({ path, flags?, serializeSample? }) => ClientSampleSink` | a single JSONL file; wraps `fs.WriteStream` and re-emits its `close`/`finish`/`drain`/`error` | -| `JsonlFileSink` | `class extends ClientSampleSink` | the underlying class; exposes `readonly path` so a `close` handler knows which file is ready | -| `createInMemorySink` / `InMemorySink` | `(samples?: ClientSample[]) => InMemorySink` | collects the accepted **sample objects** into `.samples: ClientSample[]`; emits `close` on `end()` | +`ClientSampleSink` is `Sink`. Built-in implementations: -`serializeSample?: (sample: ClientSample) => string` overrides the default `JSON.stringify` for -the JSONL sinks (e.g. to redact or reshape before writing). +| Export | Notes | +|--------|-------| +| `JsonlFileSink` / `createJsonlFileSink({ path, serializeSample? })` | appends one JSON line per record; wraps `fs.WriteStream` and re-emits its events; exposes `readonly path`. The directory **must exist** | +| `InMemorySink` / `createInMemorySink(samples?)` | collects client samples into `.samples`; emits `close` on `end()` | -### Reading sink-specific info (e.g. the file path) +### Client samples -The bus hands you the sink as the base `ClientSampleSink`. To read information specific to a sink -type — for a file sink, where it was written — **narrow with `instanceof`** and read the sink's -public fields. `JsonlFileSink` exposes `path`: +A client writes every accepted sample to the sink registered under its **client id** in its call's +registry (`observedClient.sink` returns it), and removes — ends — that sink when it closes. For the +common case, a JSONL file per client, return a path from `createClientJsonlSink`: ```ts -import { JsonlFileSink } from '@observertc/observer-js'; +const observer = new Observer({ + // return undefined for no file + createClientJsonlSink: ({ observedCall, clientId }) => `./stats/${observedCall.callId}__${clientId}.jsonl`, +}); observer.on('client-sink-created', ({ observedClient, sink }) => { - if (sink instanceof JsonlFileSink) { - const { path } = sink; // the file this client's samples go to - sink.once('close', () => uploadFile(path)); // close = flushed & fd closed → ready - } + sink.once('close', () => uploadFile(sink.path)); // flushed & fd closed → ready }); ``` -The general pattern: each concrete sink exposes whatever it wants as `public readonly` fields, and -consumers narrow (`instanceof YourSink`) to read them. Your own sinks do the same. - -### Writing your own sink - -Subclass `ClientSampleSink` and emit the lifecycle events yourself — for any non-file -destination (a remote endpoint, a message queue, an object store, …): +For any other destination, create your own sink under the client id when the client is created: ```ts -import { ClientSampleSink, ClientSample, ClientSampleSinkFactory } from '@observertc/observer-js'; - -class HttpSink extends ClientSampleSink { +class HttpSink extends EventEmitter implements ClientSampleSink { + readonly id: string; + private readonly url: string; private buffer: ClientSample[] = []; - constructor(private readonly url: string) { super(); } + private ended = false; + constructor(settings: SinkSettings & { url: string }) { + super(); + this.id = settings.id; + this.url = settings.url; + } write(sample: ClientSample): boolean { + if (this.ended) return false; this.buffer.push(sample); // batch; decide your own backpressure return true; } end(): void { + if (this.ended) return; + this.ended = true; + this.emit('ended'); // required: removes it from its registry fetch(this.url, { method: 'POST', body: JSON.stringify(this.buffer) }) .then(() => this.emit('close')) // signal "destination ready" .catch((err) => this.emit('error', err)); } } -const createClientSink: ClientSampleSinkFactory = ({ clientId, observedCall }) => - new HttpSink(`https://stats.example.com/${observedCall.callId}/${clientId}`); +observer.on('client-added', ({ observedCall, observedClient }) => { + observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url: `https://stats.example.com/${observedClient.clientId}` }); +}); +``` + +Client ids double as sink ids, so they must be unique across the observer's open calls (client +monitors generate uuids). A client whose id is already a sink id gets no sample file, and a +warning is logged. + +The library calls `write(sample)` **synchronously** per accepted sample (it is not awaited), so +**backpressure and batching are the sink's concern**. + +### Other records: e.g. mediasoup pipe transport stats + +```ts +const observedRouter = observer.createObservedMediasoupRouter({ router })!; -const observer = new Observer({ createClientSink }); +observedRouter.on('transport-sample-added', ({ transport }) => { + if (transport.type !== 'pipe') return; + + observedRouter.sinks.create(JsonlFileSink, { id: transport.id, path: `./stats/${transport.id}.jsonl` }); + + // Your own polling. A write to an ended sink returns false, so a late getStats() is harmless. + const timer = setInterval(async () => observedRouter.sinks.write(transport.id, await transport.getStats()), 5_000); + + transport.observer.once('close', () => clearInterval(timer)); +}); ``` -`observedClient.sink?` exposes the created sink; the `client-sink-created` event delivers it on -the bus with full ancestry. `ClientSampleSinkFactory` is -`(p: { clientId: string; observedCall: ObservedCall }) => ClientSampleSink | undefined`. +The files are ended when the router closes, which is usually the lifetime you want for a pipe +transport's stats. To end one earlier, call `end()` on it: `observedRouter.sinks.get(transport.id)?.end()`. --- @@ -2036,7 +2058,7 @@ client-reported `ClientSample` — a room id or display name, an application-lev (*"recording started"*), a server-detected issue, an extension stat, or a device/meta item. `ObservedClient` exposes **injection** methods that merge such data into the client's sample stream, so it updates the live model **and** is persisted to the client's -[sink](#sinks-per-client-sample-persistence) exactly like sampled data. +[sink](#sinks) exactly like sampled data. | Method | Adds to the sample's | Surfaces as | |--------|----------------------|-------------| @@ -2161,7 +2183,7 @@ event map + scope types), `detectors/` (`Detector`, `Detectors`, and one file pe `ActiveIssueTracker`, `ActiveIssuesRegistry`, `ObservedClientIssueRegistry`), `scores/`, `resolvers/` (remote-track resolvers), `utils/` (`stats`, `SlidingWindow`, `TrendTester`, `CallHealthAggregator`), `common/` (`logger`, `utils`, `Middleware`), `schema/` -(sample/event/meta types), and `sinks/` (the `ClientSampleSink` base + `JsonlFileSink` / +(sample/event/meta types), and `sinks/` (the `Sink` interface, `SinkRegistry` + `JsonlFileSink` / `InMemorySink`, re-exported from the package root). **Conventions to follow when developing further:** diff --git a/examples/client-summaries.ts b/examples/client-summaries.ts new file mode 100644 index 0000000..a4bcb73 --- /dev/null +++ b/examples/client-summaries.ts @@ -0,0 +1,251 @@ +/** + * Per-client and per-call summaries. + * + * Shows how to turn what an `ObservedClient` accumulated over its session into a record you can + * save, and how to fold those records into the call's `CallSummary` with an enricher: + * + * ObservedClient session totals ──client-closed──▶ enricher ──▶ summary.attachments + * │ + * ObservedCall.close() ──▶ call-summary ◀────────────────────────────┘ + * + * Run it with: yarn example:summaries + * + * Nothing here needs a helper object next to the client: every value is read off `ObservedClient` + * at the moment it closes (`rttSummary`, `scoreSummary`, `issueHistogram`, the `total*` counters + * and the `numberOfSamplesOver*` path counters). The histograms are also kept raw, so the call + * level can combine them bucket by bucket — percentiles of a call cannot be averaged from the + * percentiles of its clients. + */ +import { + Observer, + ObservedClient, + percentileOfHistogram, + RTT_HISTOGRAM_BUCKET_WIDTH_IN_MS, + RTT_HISTOGRAM_NUMBER_OF_BUCKETS, + type CallSummary, + type ClientSample, + type RttSummary, + type ScoreSummary, +} from '../src'; + +/* ------------------------------------------------------------------------------------------------ + * 1. The record the application saves per client. + * ---------------------------------------------------------------------------------------------- */ + +type ClientPath = 'udp' | 'tcp' | 'turn-udp' | 'turn-tcp' | 'mixed' | 'unknown'; + +type ClientStatsSummary = { + clientId: string; + samples: number; + browser?: string; + os?: string; + path: ClientPath; + rtt: RttSummary; + maxRttInMs?: number; + score: ScoreSummary; + inboundPacketsLost: number; + inboundPacketsReceived: number; + inboundLossPct?: number; + issues: Record; +}; + +/** The `path` label is an application decision, so it is derived here rather than in the library. */ +function pathOf(client: ObservedClient): ClientPath { + const used: ClientPath[] = []; + + if (0 < client.numberOfSamplesOverUdp) used.push('udp'); + if (0 < client.numberOfSamplesOverTcp) used.push('tcp'); + if (0 < client.numberOfSamplesOverTurnUdp) used.push('turn-udp'); + if (0 < client.numberOfSamplesOverTurnTcp) used.push('turn-tcp'); + + if (used.length === 0) return 'unknown'; + + return used.length === 1 ? used[0] : 'mixed'; +} + +function summarizeClient(client: ObservedClient): ClientStatsSummary { + const lost = client.totalInboundPacketsLost; + const received = client.totalInboundPacketsReceived; + + return { + clientId: client.clientId, + samples: client.acceptedSamples, + browser: client.browser ? `${client.browser.name} ${client.browser.version}` : undefined, + os: client.operationSystem ? `${client.operationSystem.name} ${client.operationSystem.version}` : undefined, + path: pathOf(client), + // Both are frozen: safe to store as they are. + rtt: client.rttSummary, + maxRttInMs: client.maxRttInMs, + score: client.scoreSummary, + inboundPacketsLost: lost, + inboundPacketsReceived: received, + inboundLossPct: 0 < lost + received ? (lost / (lost + received)) * 100 : undefined, + // Copied: `issueHistogram` is live and has no prototype, so spread it into a plain object. + issues: { ...client.issueHistogram }, + }; +} + +/* ------------------------------------------------------------------------------------------------ + * 2. The call-level state the enrichers keep in `summary.attachments`. + * ---------------------------------------------------------------------------------------------- */ + +type CallAttachments = { + clients: ClientStatsSummary[]; + rttHistogram: number[]; + issues: Record; + regions: string[]; +}; + +/** Typed access to the enricher-owned part of the summary, created on first use. */ +function attachmentsOf(summary: CallSummary): CallAttachments { + return (summary.attachments.app ??= { + clients: [], + rttHistogram: new Array(RTT_HISTOGRAM_NUMBER_OF_BUCKETS).fill(0), + issues: {}, + regions: [], + } satisfies CallAttachments) as CallAttachments; +} + +/* ------------------------------------------------------------------------------------------------ + * 3. The observer, with built-in sections plus enrichers. + * ---------------------------------------------------------------------------------------------- */ + +const observer = new Observer({ + closeCallIfEmptyForMs: undefined, + callSummary: { + // Built-in sections: who was there, every call issue, TURN relays, the call score. + include: [ 'clients', 'issues', 'turnServers', 'scores' ], + enrich: { + // Low-frequency event, once per client: enrich from `appData` the application set. + 'client-added': (summary, { observedClient }) => { + const region = observedClient.appData.region; + + if (typeof region === 'string' && !attachmentsOf(summary).regions.includes(region)) { + attachmentsOf(summary).regions.push(region); + } + }, + + // Fires once per client, also for every client still open when the call closes, and + // always before `call-summary`. This is where a client's session is complete. + 'client-closed': (summary, { observedClient }) => { + const app = attachmentsOf(summary); + + app.clients.push(summarizeClient(observedClient)); + + observedClient.rttHistogram.forEach((count, bucket) => { + app.rttHistogram[bucket] += count; + }); + for (const [ type, count ] of Object.entries(observedClient.issueHistogram)) { + app.issues[type] = (app.issues[type] ?? 0) + count; + } + }, + }, + }, +}); + +observer.on('call-summary', ({ summary }) => { + const app = attachmentsOf(summary); + const callRttP95 = percentileOfHistogram(app.rttHistogram, RTT_HISTOGRAM_BUCKET_WIDTH_IN_MS, 0.95, { + max: Math.max(...app.clients.map((c) => c.maxRttInMs ?? 0)), + openEnded: true, + }); + + // in production: archive(summary) — it is plain, serialisable data. + console.log(JSON.stringify({ + callId: summary.callId, + durationInMs: summary.durationInMs, + clients: summary.clients, + callScore: summary.scores, + callRttP95InMs: callRttP95, + issuesByType: app.issues, + regions: app.regions, + perClient: app.clients, + }, null, 2)); +}); + +/* ------------------------------------------------------------------------------------------------ + * 4. Synthetic traffic. In production these come from client-monitor-js. + * ---------------------------------------------------------------------------------------------- */ + +let now = Date.now(); + +type Tick = { + rttInMs: number, + received: number, + lost: number, + score: number, + relay?: 'udp' | 'tcp', + issues?: ClientSample['clientIssues'], + events?: ClientSample['clientEvents'], +}; + +function sampleOf(clientId: string, tick: Tick): ClientSample { + const timestamp = now; + const pcId = `${clientId}-pc`; + const local = tick.relay + ? { id: 'local', timestamp, candidateType: 'relay', protocol: 'udp', relayProtocol: tick.relay, url: 'turn:turn.example.com:3478' } + : { id: 'local', timestamp, candidateType: 'srflx', protocol: 'udp' }; + + return { + callId: 'call-1', + clientId, + timestamp, + score: tick.score, + clientIssues: tick.issues, + clientEvents: tick.events, + clientMetaItems: [ + { type: 'BROWSER', payload: JSON.stringify({ name: 'Chrome', version: '141' }) }, + { type: 'OPERATION_SYSTEM', payload: JSON.stringify({ name: 'macOS', version: '15.6' }) }, + ], + peerConnections: [ { + peerConnectionId: pcId, + inboundRtps: [ { timestamp, id: 'in', ssrc: 1, kind: 'audio', trackIdentifier: `${clientId}-in`, packetsReceived: tick.received, packetsLost: tick.lost, bytesReceived: tick.received * 120 } ], + inboundTracks: [ { timestamp, id: `${clientId}-in`, kind: 'audio' } ], + mediaSources: [ { timestamp, id: 'ms', kind: 'audio', trackIdentifier: `${clientId}-out` } ], + outboundRtps: [ { timestamp, id: 'out', ssrc: 2, kind: 'audio', trackIdentifier: `${clientId}-out`, mediaSourceId: 'ms', packetsSent: tick.received, bytesSent: tick.received * 120 } ], + outboundTracks: [ { timestamp, id: `${clientId}-out`, kind: 'audio' } ], + remoteInboundRtps: [ { timestamp, id: 'rin', ssrc: 2, kind: 'audio', roundTripTime: tick.rttInMs / 1000 } ], + iceCandidates: [ local, { id: 'remote', timestamp, candidateType: 'host', protocol: 'udp' } ], + iceCandidatePairs: [ { id: 'pair', timestamp, localCandidateId: 'local', remoteCandidateId: 'remote', nominated: true, bytesReceived: 0, bytesSent: 0 } ], + iceTransports: [ { id: 'transport', timestamp, selectedCandidatePairId: 'pair' } ], + } ], + } as unknown as ClientSample; +} + +const call = observer.createObservedCall({ callId: 'call-1' })!; + +call.createObservedClient({ clientId: 'alice', appData: { region: 'eu-north' } }); +call.createObservedClient({ clientId: 'bob', appData: { region: 'us-east' } }); + +// The packet counters are cumulative, as `getStats()` reports them. +let bobLost = 0; + +for (let i = 0; i < 60; ++i) { + now += 1000; + + // CLIENT_JOINED / CLIENT_LEFT give the call its `startedAt` / `endedAt`, hence `durationInMs`. + const events = () => (i === 0 ? [ { type: 'CLIENT_JOINED', timestamp: now } ] : i === 59 ? [ { type: 'CLIENT_LEFT', timestamp: now } ] : undefined); + + // alice: healthy, direct UDP. + observer.accept(sampleOf('alice', { rttInMs: 40 + (i % 10), received: (i + 1) * 50, lost: 0, score: 4.6, events: events() })); + + // bob: congested for a while, falls back to TURN over TCP halfway through. + const congested = 20 <= i && i < 35; + + bobLost += congested ? 8 : (i % 10 === 0 ? 1 : 0); + observer.accept(sampleOf('bob', { + events: events(), + rttInMs: congested ? 350 + (i * 5) : 120, + received: (i + 1) * 50, + lost: bobLost, + score: congested ? 2.1 : 4.0, + relay: 30 <= i ? 'tcp' : undefined, + issues: i === 20 ? [ { type: 'network-congestion', key: 'bob:network-congestion', timestamp: now } ] + : i === 35 ? [ { type: 'network-congestion-resolved', key: 'bob:network-congestion', timestamp: now } ] + : undefined, + })); +} + +// Closing the call closes its clients (each fires `client-closed`), then emits `call-summary`. +call.close(); +observer.close(); diff --git a/package.json b/package.json index 9d132ca..bcf0159 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "test:coverage": "jest --config jest.config.js --coverage", "example": "ts-node --compiler-options '{\"module\":\"commonjs\"}' examples/sfu-observer.ts", "example:detectors": "ts-node --compiler-options '{\"module\":\"commonjs\"}' examples/detectors.ts", + "example:summaries": "ts-node --compiler-options '{\"module\":\"commonjs\"}' examples/client-summaries.ts", "bench": "ts-node --compiler-options '{\"module\":\"commonjs\"}' bench/detectors.bench.ts" }, "keywords": [ diff --git a/src/ObservedCall.ts b/src/ObservedCall.ts index 11c8186..3f55581 100644 --- a/src/ObservedCall.ts +++ b/src/ObservedCall.ts @@ -1,4 +1,7 @@ import { EventEmitter } from 'node:events'; +import { SinkRegistry } from './sinks/SinkRegistry'; +import { JsonlFileSink } from './sinks/JsonlFileSink'; +import type { ClientSample } from './schema/ClientSample'; import { createLogger } from './common/logger'; import { ObservedClient, ObservedClientEvents, ObservedClientSettings } from './ObservedClient'; import { Observer } from './Observer'; @@ -107,6 +110,8 @@ export class ObservedCall = Record; public constructor( @@ -119,6 +124,7 @@ export class ObservedCall = Record = Record = Record, { id: settings.clientId, path: jsonlPath }); + } + this.emit('newclient', result); this._notify('client-added', { ...this.eventScope, observedClient: result }); - if (result.sink) { - this._notify('client-sink-created', { ...this.eventScope, observedClient: result, sink: result.sink }); + if (jsonlSink) { + this._notify('client-sink-created', { ...this.eventScope, observedClient: result, sink: jsonlSink }); } if (wasEmpty) { diff --git a/src/ObservedClient.ts b/src/ObservedClient.ts index f3efb23..fa9ae12 100644 --- a/src/ObservedClient.ts +++ b/src/ObservedClient.ts @@ -11,7 +11,7 @@ import { normalizeScoreReasons, parseJsonAs, parseJsonObject } from './common/ut import { CalculatedScore } from './scores/CalculatedScore'; import type { AcceptContext } from './Observer'; import type { ObserverEvents, ObservedClientScope } from './ObserverEvents'; -import type { ClientSampleSink } from './sinks/ClientSampleSink'; +import type { Sink } from './sinks/Sink'; import { percentileOfHistogram } from './utils/stats'; import { ObservedClientIssueRegistry } from './issues/ObservedClientIssueRegistry'; import { ActiveClientIssue, RESOLVED_ISSUE_SUFFIX, baseIssueType, isClientIssueResolutionEntry } from './issues/ActiveClientIssue'; @@ -77,9 +77,6 @@ export class ObservedClient = Record(); - /** Per-client sink (from `ObserverConfig.createClientSink`), if any. */ - public readonly sink?: ClientSampleSink; - /** Ancestry base shared by all Observer-bus events originating at this client. */ public readonly eventScope: ObservedClientScope; public readonly calculatedScore: CalculatedScore = { @@ -253,7 +250,6 @@ export class ObservedClient = Record = Record logger.warn('ClientSampleSink error for client %s: %o', this.clientId, err); - - this.sink.once('close', () => this.sink?.off('error', onError)); - this.sink.on('error', onError); - } this.activeIssues = activeIssues; } /** The distinct `publicAddress`es currently known by this client's open peer connections. */ + /** + * The sink this client's samples are written to: whatever is registered under its client id in + * `call.sinks` (the `createClientJsonlSink` file, or a sink the application added), if any. + */ + public get sink(): Sink | undefined { + return this.call.sinks.get>(this.clientId); + } + public get publicAddresses(): string[] { const result = new Set(); @@ -395,7 +393,7 @@ export class ObservedClient = Record = Record = Record = R public readonly webrtcTransportIds = new Set(); public closed = false; + /** + * The router's sinks, e.g. one per pipe transport for its `getStats()`. They are ended when the + * router closes, which is usually the lifetime wanted; `sink.end()` ends one earlier. Writing to + * them, and polling what is written, is the application's. Its parent is `observer.sinks`. + */ + public readonly sinks: SinkRegistry; + // Id -> sample indexes over the arrays in `this.sample`, so an entity can be reached in O(1) // instead of scanning (`sample.producers.find(...)`). They hold the *same* objects as the arrays. private readonly _transportSamples = new Map(); @@ -103,14 +111,20 @@ export class ObservedMediasoupRouter = R private readonly _dataConsumerSamples = new Map(); private readonly _enrich?: MediasoupSampleEnricher; + /** + * @param parentSinks the parent of `sinks`: `observer.sinks`, passed by + * `Observer.createObservedMediasoupRouter`. Internal; a router constructed directly has none. + */ public constructor( settings: ObservedMediasoupRouterSettings, + parentSinks?: SinkRegistry, ) { super(); this.router = settings.router; this.appData = (settings.appData ?? {}) as AppData; this._enrich = settings.enrich; + this.sinks = new SinkRegistry(parentSinks); this.sample = { routerId: this.router.id, attachments: settings.attachments ?? {}, @@ -204,6 +218,7 @@ export class ObservedMediasoupRouter = R this.closed = true; if (this.sample.closedAt === undefined) this.sample.closedAt = Date.now(); + this.sinks.clear(); this.emit('close'); } diff --git a/src/Observer.ts b/src/Observer.ts index 95e9922..d81e9b0 100644 --- a/src/Observer.ts +++ b/src/Observer.ts @@ -5,9 +5,9 @@ import { ClientSample } from './schema/ClientSample'; import { ObservedTURN } from './ObservedTURN'; import type { RemoteTrackResolverFactory } from './resolvers/RemoteTrackResolver'; import type { ObserverEvents, ObserverEventBase, ObservedMediasoupRouterScope } from './ObserverEvents'; -import type { ClientSampleSinkFactory } from './sinks/ClientSampleSink'; import { Middleware, MiddlewareProcessor } from './common/Middleware'; import { ObservedMediasoupRouter, ObservedMediasoupRouterSettings } from './ObservedMediasoupRouter'; +import { SinkRegistry } from './sinks/SinkRegistry'; import { AvailableCallScopeDetectorsConfigs, AvailableObserverScopeDetectorsConfigs, Detectors } from './detectors/Detectors'; import type { AvailableValidatorConfigs } from './validators/Validators'; import type { ValidationReport, RunningValidator } from './validators/Validator'; @@ -228,11 +228,35 @@ export type ObserverConfig = Record path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`), + * }); + * + * // before (1.2): any other sink + * new Observer({ createClientSink: ({ clientId, observedCall }) => new HttpSink(url(observedCall, clientId)) }); + * // after: create it under the client id when the client is added (HttpSink implements + * // `ClientSampleSink`, takes `SinkSettings & { ... }` and emits `ended` from `end()`) + * observer.on('client-added', ({ observedCall, observedClient }) => { + * observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url: url(observedCall, observedClient.clientId) }); + * }); + * ``` */ - createClientSink?: ClientSampleSinkFactory, + createClientJsonlSink?: (params: { observedCall: ObservedCall, clientId: string }) => string | undefined, /** * Optional factory invoked when a call is created, producing the call's `RemoteTrackResolver` @@ -256,6 +280,8 @@ export class Observer = Record(); public readonly observedMediasoupRouters = new Map(); + public readonly sinks = new SinkRegistry(); + /** Ancestry base shared by all Observer-bus events originating at the observer. */ public readonly eventScope: ObserverEventBase = { observer: this }; public readonly config: ObserverConfig; @@ -646,7 +672,7 @@ export class Observer = Record = Record` (and `ClientSampleSink = Sink`) is the interface to implement for a custom destination; +// `JsonlFileSink` / `InMemorySink` are the built-ins; `SinkRegistry` owns their lifecycle. export { - ClientSampleSink, + SinkRegistry, JsonlFileSink, createJsonlFileSink, - createJsonlFileSinkFactory, InMemorySink, createInMemorySink, } from './sinks'; export type { + Sink, + SinkEvents, + SinkSettings, + ClientSampleSink, ClientSampleSinkEvents, - ClientSampleSinkFactory, JsonlFileSinkOptions, - JsonlFileSinkFactoryOptions, } from './sinks'; export { Middleware } from './common/Middleware'; // `Observer.acceptMiddlewares` is a public field of this type, so the type has to be nameable. diff --git a/src/sinks/ClientSampleSink.ts b/src/sinks/ClientSampleSink.ts index bcc01b0..cd31c7b 100644 --- a/src/sinks/ClientSampleSink.ts +++ b/src/sinks/ClientSampleSink.ts @@ -1,58 +1,17 @@ -import { EventEmitter } from 'node:events'; +import type { Sink, SinkEvents } from './Sink'; import type { ClientSample } from '../schema/ClientSample'; -import type { ObservedCall } from '../ObservedCall'; -/** The lifecycle events a sink may emit (a subset of Node's writable-stream events). */ -export type ClientSampleSinkEvents = { - - /** The destination is fully written and closed (e.g. a file flushed and its fd closed). */ - close: []; - - /** The destination failed. */ - error: [Error]; - - /** `end()` was processed and all queued data was flushed (before `close`). */ - finish: []; - - /** The buffer drained after backpressure; safe to write more. */ - drain: []; -}; - -export declare interface ClientSampleSink { - on(event: E, listener: (...args: ClientSampleSinkEvents[E]) => void): this; - once(event: E, listener: (...args: ClientSampleSinkEvents[E]) => void): this; - off(event: E, listener: (...args: ClientSampleSinkEvents[E]) => void): this; - emit(event: E, ...args: ClientSampleSinkEvents[E]): boolean; -} +/** The lifecycle events a client sample sink may emit. Same as {@link SinkEvents}. */ +export type ClientSampleSinkEvents = SinkEvents; /** - * A per-client destination for accepted samples — a typed `EventEmitter` base class. One sink is - * created per `ObservedClient` (via `ObserverConfig.createClientSink`). It is **object-mode**: - * `write` receives the `ClientSample` itself, so each sink decides how to serialize it (JSON - * line, protobuf, msgpack, a remote payload, …). + * A per-client destination for accepted samples. Create one under the client id with + * `observedCall.sinks.create(MySink, { id: clientId, ... })`, or let + * `ObserverConfig.createClientJsonlSink` create a `JsonlFileSink` there. * - * The library calls `write(sample)` synchronously per accepted sample (not awaited), `end()`s the - * sink when the client closes, and attaches an `error` listener so a failing sink can't crash the - * process. The application — which created the sink — listens for `close` (destination ready, - * e.g. a file flushed and its fd closed) and `error`. - * - * Subclass this and implement `write`/`end` (and `emit` your lifecycle events). Built-in sinks - * (`JsonlFileSink`, `InMemorySink`) and their factories are exported from the package root. - */ -export abstract class ClientSampleSink extends EventEmitter { - /** Accept one sample; the sink serializes it however it likes. `false` signals backpressure. */ - public abstract write(sample: ClientSample): boolean; - - /** Finish writing; the sink flushes and (if applicable) emits `close`. */ - public abstract end(): void; -} - -/** - * Produces a sink for a newly-created client, or `undefined` for no sink. Receives the - * client id and its (already-created) parent call, so the destination can be derived - * from `callId` / `clientId`. + * The library calls `write(sample)` synchronously per accepted sample (not awaited) and `end()`s + * the sink when the client closes; the registry attaches an `error` listener so a failing sink + * can't crash the process. Because `write` isn't awaited, backpressure and batching are the sink's + * concern. */ -export type ClientSampleSinkFactory = (params: { - clientId: string, - observedCall: ObservedCall, -}) => ClientSampleSink | undefined; +export type ClientSampleSink = Sink; diff --git a/src/sinks/InMemorySink.ts b/src/sinks/InMemorySink.ts index a039d98..9b841d8 100644 --- a/src/sinks/InMemorySink.ts +++ b/src/sinks/InMemorySink.ts @@ -1,26 +1,50 @@ -import { ClientSampleSink } from './ClientSampleSink'; +import { EventEmitter } from 'node:events'; +import type { Sink, SinkEvents, SinkSettings } from './Sink'; import type { ClientSample } from '../schema/ClientSample'; +export declare interface InMemorySink { + on(event: E, listener: (...args: SinkEvents[E]) => void): this; + once(event: E, listener: (...args: SinkEvents[E]) => void): this; + off(event: E, listener: (...args: SinkEvents[E]) => void): this; + emit(event: E, ...args: SinkEvents[E]): boolean; +} + /** - * A `ClientSampleSink` that collects the accepted sample objects into an array. Useful for - * tests and offline replay. Emits `close` on `end()`. + * A client sample sink that collects the accepted sample objects into an array. Useful for tests + * and offline replay. Emits `ended` and `close` on `end()`. */ -export class InMemorySink extends ClientSampleSink { - public constructor(public readonly samples: ClientSample[] = []) { +export class InMemorySink extends EventEmitter implements Sink { + public readonly id: string; + public readonly samples: ClientSample[]; + + private _ended = false; + + public constructor(settings: Partial & { samples?: ClientSample[] } = {}) { super(); + this.id = settings.id ?? 'in-memory'; + this.samples = settings.samples ?? []; + } + + public get ended(): boolean { + return this._ended; } public write(sample: ClientSample): boolean { + if (this._ended) return false; this.samples.push(sample); return true; } public end(): void { + if (this._ended) return; + + this._ended = true; + this.emit('ended'); this.emit('close'); } } export function createInMemorySink(samples?: ClientSample[]): InMemorySink { - return new InMemorySink(samples); + return new InMemorySink({ samples }); } diff --git a/src/sinks/JsonlFileSink.ts b/src/sinks/JsonlFileSink.ts index 93dea09..6fba3df 100644 --- a/src/sinks/JsonlFileSink.ts +++ b/src/sinks/JsonlFileSink.ts @@ -1,31 +1,44 @@ import * as fs from 'fs'; -import * as path from 'path'; -import { ClientSampleSink, ClientSampleSinkFactory } from './ClientSampleSink'; +import { EventEmitter } from 'node:events'; +import type { Sink, SinkEvents, SinkSettings } from './Sink'; import type { ClientSample } from '../schema/ClientSample'; -export type JsonlFileSinkOptions = { +export type JsonlFileSinkOptions = Partial & { /** Absolute or relative file path to append JSONL lines to. */ path: string, /** Optional custom serializer; defaults to `JSON.stringify`. */ - serializeSample?: (sample: ClientSample) => string, + serializeSample?: (record: T) => string, }; /** - * A sink that serializes each sample to a JSON line and appends it to a file. It wraps an + * A sink that serializes each record to a JSON line and appends it to a file. Defaults to client + * samples, but takes any record type: `sinks.create(JsonlFileSink, { id, path })`. It wraps an * `fs.WriteStream` and re-emits its lifecycle events, so `close` fires once the file is flushed * and its descriptor is closed (file ready) and `error` surfaces file errors. */ -export class JsonlFileSink extends ClientSampleSink { +export declare interface JsonlFileSink { + on(event: E, listener: (...args: SinkEvents[E]) => void): this; + once(event: E, listener: (...args: SinkEvents[E]) => void): this; + off(event: E, listener: (...args: SinkEvents[E]) => void): this; + emit(event: E, ...args: SinkEvents[E]): boolean; +} + +export class JsonlFileSink extends EventEmitter implements Sink { + public readonly id: string; + /** The file this sink writes to. Read it (e.g. in a `close` handler) to upload/move the file. */ public readonly path: string; private readonly _stream: fs.WriteStream; - private readonly _serializeSample: (sample: ClientSample) => string; + private readonly _serializeSample: (record: T) => string; + private _ended = false; - public constructor(options: JsonlFileSinkOptions) { + public constructor(options: JsonlFileSinkOptions) { super(); + // A registry always passes an id; a standalone sink falls back to its path. + this.id = options.id ?? options.path; this.path = options.path; this._stream = fs.createWriteStream(this.path, { flags: 'a' }); @@ -34,47 +47,29 @@ export class JsonlFileSink extends ClientSampleSink { this._stream.on('drain', () => this.emit('drain')); this._stream.on('error', (err) => this.emit('error', err)); - this._serializeSample = options.serializeSample ?? ((sample) => JSON.stringify(sample)); + this._serializeSample = options.serializeSample ?? ((record) => JSON.stringify(record)); + } + + public get ended(): boolean { + return this._ended; } - public write(sample: ClientSample): boolean { - return this._stream.write(`${this._serializeSample(sample)}\n`); + public write(record: T): boolean { + if (this._ended) return false; + + return this._stream.write(`${this._serializeSample(record)}\n`); } public end(): void { + if (this._ended) return; + + this._ended = true; + this.emit('ended'); this._stream.end(); } } /** Build a JSONL file sink for a single path. */ -export function createJsonlFileSink(options: JsonlFileSinkOptions): ClientSampleSink { - return new JsonlFileSink(options); -} - -export type JsonlFileSinkFactoryOptions = { - - /** Directory the per-client files are written into (must exists). */ - directory: string, - - /** Builds the file name from the call/client ids. Defaults to `${callId}__${clientId}.jsonl`. */ - getFileName?: (params: { callId: string, clientId: string }) => string, - - /** Optional custom serializer; defaults to `JSON.stringify`. */ - serializeSample?: (sample: ClientSample) => string, -}; - -/** - * Convenience factory that writes one JSONL file per client, with the path derived from - * `callId` / `clientId`. Plug into `ObserverConfig.createClientSink`. - */ -export function createJsonlFileSinkFactory(options: JsonlFileSinkFactoryOptions): ClientSampleSinkFactory { - - return ({ clientId, observedCall }) => { - const filename = options.getFileName?.({ callId: observedCall.callId, clientId }) ?? `${observedCall.callId}__${clientId}.jsonl`; - - return createJsonlFileSink({ - path: path.join(options.directory, filename), - serializeSample: options.serializeSample, - }); - }; +export function createJsonlFileSink(options: JsonlFileSinkOptions): JsonlFileSink { + return new JsonlFileSink(options); } diff --git a/src/sinks/Sink.ts b/src/sinks/Sink.ts new file mode 100644 index 0000000..0c3b34a --- /dev/null +++ b/src/sinks/Sink.ts @@ -0,0 +1,62 @@ +/** The lifecycle events a sink emits. */ +export type SinkEvents = { + + /** + * `end()` was called: the sink accepts no more writes. **Required**: a sink emits it + * synchronously, once, from `end()` — it is how its registry removes it and frees its id. + */ + ended: []; + + /** The destination is fully written and closed (e.g. a file flushed and its fd closed). */ + close: []; + + /** The destination failed. */ + error: [Error]; + + /** All queued data was flushed (before `close`). */ + finish: []; + + /** The buffer drained after backpressure; safe to write more. */ + drain: []; +}; + +/** What a registry passes to a sink's constructor. Implementations extend it with their own settings. */ +export type SinkSettings = { + + /** Unique per observer. The registry generates one (uuid v4) when it is omitted. */ + id: string; +}; + +/** + * A destination for records. It is **object-mode** and agnostic about what passes through it: + * `write` receives the record itself, and the implementation decides how to serialize it (JSON + * line, protobuf, a remote payload, …). `JsonlFileSink` and `InMemorySink` are the built-in + * implementations; `ClientSampleSink` is `Sink`. + * + * Create sinks with a registry's `create(SinkClass, settings)`, which constructs + * `new SinkClass({ ...settings, id })` and registers it on the object that owns its lifecycle. + * `end()` closes a sink and — through the `ended` event — removes it from its registry; the owner + * ends whatever is left when it closes. + * + * An implementation typically `extends EventEmitter implements Sink`, and must: + * - emit `ended` synchronously, once, from `end()` (and make a second `end()` a no-op); + * - refuse writes after `end()` (return `false`); + * - emit `close` when the destination is ready, and `error` when it fails. + */ +export interface Sink { + readonly id: string; + + /** Accept one record. `false` once ended (nothing written), or on backpressure. */ + write(record: T): boolean; + + /** No more writes; emit `ended`, then flush and emit `close`. Idempotent. */ + end(): void; + + // Listener args are untyped so that a plain `EventEmitter` satisfies the interface; see + // `SinkEvents` for each event's arguments (the built-in sinks declare them fully typed). + /* eslint-disable @typescript-eslint/no-explicit-any */ + on(event: keyof SinkEvents, listener: (...args: any[]) => void): this; + once(event: keyof SinkEvents, listener: (...args: any[]) => void): this; + off(event: keyof SinkEvents, listener: (...args: any[]) => void): this; + /* eslint-enable @typescript-eslint/no-explicit-any */ +} diff --git a/src/sinks/SinkRegistry.ts b/src/sinks/SinkRegistry.ts new file mode 100644 index 0000000..d8840ec --- /dev/null +++ b/src/sinks/SinkRegistry.ts @@ -0,0 +1,112 @@ +import type { Sink } from './Sink'; +import { v4 as uuidv4 } from 'uuid'; +import { createLogger } from '../common/logger'; + +const logger = createLogger('SinkRegistry'); + +/** + * The sinks of one object, ended by that object's lifecycle. `create(SinkClass, settings)` builds a + * sink and registers it; `sink.end()` closes it and removes it; the owner calls `clear()` when it + * closes, ending whatever is left. + * + * Registries form a tree like the issue registries: `observer.sinks` is the parent of every + * `observedCall.sinks` and `observedMediasoupRouter.sinks`. **A sink id is unique per observer**: + * `create` refuses an id that any registry of the tree holds. Each registry sees its own sinks and + * its descendants', so `get`, `write` and `clear` on a parent reach a child's sinks too. + * + * A client's sample sink is the one under its client id in its call's registry. + */ +export class SinkRegistry { + // This registry's sinks and its descendants'. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private readonly _sinks = new Map>(); + + public constructor( + public readonly parent?: SinkRegistry, + ) { + } + + /** Number of open sinks in this registry and its descendants. */ + public get size(): number { + return this._sinks.size; + } + + public keys(): IterableIterator { + return this._sinks.keys(); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public values(): IterableIterator> { + return this._sinks.values(); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public get = Sink>(id: string): T | undefined { + return this._sinks.get(id) as T | undefined; + } + + /** + * Construct `new SinkClass(settings)` and register it. The id (`settings.id`, generated when + * omitted) is checked first, so nothing is constructed — no file is opened — when it is taken: + * `undefined` is returned and a warning logged. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public create Sink>( + SinkClass: C, + settings: Omit[0], 'id'> & { id?: string }, + ): InstanceType | undefined { + const lineage = this._lineage(); + const id = settings.id ?? uuidv4(); + + if (lineage[lineage.length - 1]._sinks.has(id)) { + logger.warn('Sink id %s is already in use; sink ids must be unique per observer. No sink was created', id); + + return undefined; + } + + const sink = new SinkClass({ ...settings, id }) as InstanceType; + // An EventEmitter 'error' with no listener throws; a failing destination must not crash the process. + const onError = (err: Error) => logger.warn('Sink %s failed: %o', id, err); + + sink.on('error', onError); + sink.once('close', () => sink.off('error', onError)); + sink.once('ended', () => { + for (const registry of lineage) { + if (registry._sinks.get(id) === sink) registry._sinks.delete(id); + } + }); + for (const registry of lineage) registry._sinks.set(id, sink); + + return sink; + } + + /** Write `record` to the sink `id`. `false` when there is no such sink, or on backpressure. */ + public write(id: string, record: unknown): boolean { + const sink = this._sinks.get(id); + + if (!sink) { + logger.debug('No sink %s; record dropped', id); + + return false; + } + + return sink.write(record); + } + + /** End every sink of this registry and of its descendants. */ + public clear(): void { + // Snapshot first: each `end()` removes its sink from the map. + for (const sink of [ ...this._sinks.values() ]) { + try { + sink.end(); + } catch (err) { + logger.warn('Sink %s failed to end: %o', sink.id, err); + } + } + } + + /** This registry and its ancestors, up to the root. */ + private _lineage(): SinkRegistry[] { + return [ this, ...(this.parent?._lineage() ?? []) ]; + } +} diff --git a/src/sinks/index.ts b/src/sinks/index.ts index 0b3949c..2daa0eb 100644 --- a/src/sinks/index.ts +++ b/src/sinks/index.ts @@ -1,6 +1,7 @@ -// Internal barrel for the per-client sample sinks, re-exported from the package root. -export { ClientSampleSink } from './ClientSampleSink'; -export type { ClientSampleSinkEvents, ClientSampleSinkFactory } from './ClientSampleSink'; -export { JsonlFileSink, createJsonlFileSink, createJsonlFileSinkFactory } from './JsonlFileSink'; -export type { JsonlFileSinkOptions, JsonlFileSinkFactoryOptions } from './JsonlFileSink'; +// Internal barrel for the sinks, re-exported from the package root. +export type { Sink, SinkEvents, SinkSettings } from './Sink'; +export { SinkRegistry } from './SinkRegistry'; +export type { ClientSampleSink, ClientSampleSinkEvents } from './ClientSampleSink'; +export { JsonlFileSink, createJsonlFileSink } from './JsonlFileSink'; +export type { JsonlFileSinkOptions } from './JsonlFileSink'; export { InMemorySink, createInMemorySink } from './InMemorySink'; diff --git a/src/utils/stats.ts b/src/utils/stats.ts index 5738034..2414c94 100644 --- a/src/utils/stats.ts +++ b/src/utils/stats.ts @@ -426,7 +426,9 @@ function erf(x: number): number { * holds the values in `[i * bucketWidth, (i + 1) * bucketWidth)`. Interpolates linearly inside * the bucket the rank falls in, so the error is at most one bucket width. With `openEnded`, the * last bucket holds every value above its lower bound, and a percentile falling in it is reported - * as `max`. The estimate is clamped to `[min, max]` when given. `undefined` for an empty histogram. + * as `max`. With `min`/`max` (the observed extremes), the bucket holding them is narrowed to the + * observed range before interpolating, so the estimate never leaves `[min, max]`. `undefined` for an + * empty histogram. */ export function percentileOfHistogram( histogram: readonly number[], @@ -451,9 +453,12 @@ export function percentileOfHistogram( } if (options.openEnded && index === histogram.length - 1 && options.max !== undefined) return options.max; - const estimate = (index * bucketWidth) + (((rank - cumulative) / count) * bucketWidth); + // Narrow the bucket to the observed range, so a distribution inside one bucket (40–49 ms in + // [40, 60)) is interpolated over 40–49 rather than over the whole bucket and then clamped. + const lower = Math.max(index * bucketWidth, options.min ?? -Infinity); + const upper = Math.min((index + 1) * bucketWidth, options.max ?? Infinity); - return Math.min(Math.max(estimate, options.min ?? estimate), options.max ?? estimate); + return lower + (((rank - cumulative) / count) * Math.max(0, upper - lower)); } return options.max; diff --git a/tests/Observer.spec.ts b/tests/Observer.spec.ts index 0645215..fcd7320 100644 --- a/tests/Observer.spec.ts +++ b/tests/Observer.spec.ts @@ -206,20 +206,21 @@ describe('Observer', () => { }); describe('per-client sinks', () => { - it('creates a sink via createClientSink and emits client-sink-created', () => { - const observer = new Observer({ - createClientSink: () => createInMemorySink(), - }); - let eventSink: unknown; + it('uses a sink added under the client id on client-added', () => { + const observer = new Observer(); - observer.on('client-sink-created', ({ sink }) => { eventSink = sink; }); + observer.on('client-added', ({ observedCall, observedClient }) => { + observedCall.sinks.create(InMemorySink, { id: observedClient.clientId }); + }); observer.accept(makeSample({})); - const client = observer.getObservedCall('call-1')?.getObservedClient('client-1'); + const call = observer.getObservedCall('call-1'); + const client = call?.getObservedClient('client-1'); expect(client?.sink).toBeInstanceOf(InMemorySink); - expect(eventSink).toBe(client?.sink); + expect(client?.sink).toBe(call?.sinks.get('client-1')); + expect((client?.sink as InMemorySink).samples).toHaveLength(1); observer.close(); }); diff --git a/tests/injection.spec.ts b/tests/injection.spec.ts index 719aa92..23a862d 100644 --- a/tests/injection.spec.ts +++ b/tests/injection.spec.ts @@ -5,8 +5,10 @@ import { makeSample } from './helpers/samples'; function setup() { let sink: InMemorySink | undefined; - const observer = new Observer({ - createClientSink: () => (sink = new InMemorySink()), + const observer = new Observer(); + + observer.on('client-added', ({ observedCall, observedClient }) => { + sink = observedCall.sinks.create(InMemorySink, { id: observedClient.clientId }); }); // First accept creates the call + client (and the sink) and persists the first sample. diff --git a/tests/sinkRegistry.spec.ts b/tests/sinkRegistry.spec.ts new file mode 100644 index 0000000..f7e56b6 --- /dev/null +++ b/tests/sinkRegistry.spec.ts @@ -0,0 +1,257 @@ +import { EventEmitter } from 'node:events'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { Observer } from '../src/Observer'; +import type { Sink, SinkSettings } from '../src/sinks/Sink'; +import { SinkRegistry } from '../src/sinks/SinkRegistry'; +import type { ClientSampleSink } from '../src/sinks/ClientSampleSink'; +import { JsonlFileSink } from '../src/sinks/JsonlFileSink'; +import { InMemorySink } from '../src/sinks/InMemorySink'; +import type { ClientSample } from '../src/schema/ClientSample'; + +/** Records what happened to it. Counts constructions so a refused `create` can be checked. */ +class RecordingSink extends EventEmitter implements Sink { + public static constructed = 0; + public readonly id: string; + public readonly records: unknown[] = []; + public readonly label?: string; + public endCalls = 0; + public ended = false; + + public constructor(settings: SinkSettings & { label?: string }) { + super(); + this.id = settings.id; + this.label = settings.label; + ++RecordingSink.constructed; + } + + public write(record: unknown): boolean { + if (this.ended) return false; + this.records.push(record); + + return true; + } + + public end(): void { + if (this.ended) return; + this.ended = true; + ++this.endCalls; + this.emit('ended'); + this.emit('close'); + } +} + +/* Fakes for the mediasoup `types.*` objects, as in mediasoupRouter.spec.ts. */ +const fakeRouter = (id = 'router-1') => ({ id, observer: new EventEmitter() }) as any; +const fakePipeTransport = (id: string) => ({ + id, + type: 'pipe', + tuple: { localAddress: '10.0.0.1', localPort: 4000, protocol: 'udp' }, + observer: new EventEmitter(), +}) as any; + +const sample = (clientId: string): ClientSample => ({ callId: 'call-1', clientId, timestamp: Date.now() } as ClientSample); + +describe('SinkRegistry', () => { + it('creates and registers a sink, writes to it by id, and drops it once the sink is ended', () => { + const registry = new SinkRegistry(); + const sink = registry.create(RecordingSink, { id: 'a', label: 'x' })!; + + expect(sink).toBeInstanceOf(RecordingSink); + expect(sink.id).toBe('a'); + expect(sink.label).toBe('x'); + expect(registry.get('a')).toBe(sink); + expect(registry.write('a', { n: 1 })).toBe(true); + expect(registry.write('missing', { n: 2 })).toBe(false); + + sink.end(); + sink.end(); + + expect(sink.ended).toBe(true); + expect(sink.endCalls).toBe(1); + expect(registry.get('a')).toBeUndefined(); + expect(registry.write('a', { n: 3 })).toBe(false); + expect(sink.write({ n: 4 })).toBe(false); + expect(sink.records).toEqual([ { n: 1 } ]); + }); + + it('generates an id when none is given', () => { + const registry = new SinkRegistry(); + const sink = registry.create(RecordingSink, {})!; + + expect(sink.id).toMatch(/^[0-9a-f-]{36}$/); + expect(registry.get(sink.id)).toBe(sink); + }); + + it('keeps ids unique across the tree without constructing the refused sink, and lets a parent reach its descendants\' sinks', () => { + const root = new SinkRegistry(); + const callA = new SinkRegistry(root); + const callB = new SinkRegistry(root); + const aliceA = callA.create(RecordingSink, { id: 'alice' })!; + const before = RecordingSink.constructed; + + // Taken in callA: refused in the same registry, a sibling and the parent — nothing constructed. + expect(callA.create(RecordingSink, { id: 'alice' })).toBeUndefined(); + expect(callB.create(RecordingSink, { id: 'alice' })).toBeUndefined(); + expect(root.create(RecordingSink, { id: 'alice' })).toBeUndefined(); + expect(RecordingSink.constructed).toBe(before); + + const own = root.create(RecordingSink, { id: 'own' })!; + + expect(callA.create(RecordingSink, { id: 'own' })).toBeUndefined(); + + // A parent addresses its descendants' sinks; a child does not see its siblings' or its parent's. + expect(root.get('alice')).toBe(aliceA); + expect(root.write('alice', { n: 1 })).toBe(true); + expect(aliceA.records).toEqual([ { n: 1 } ]); + expect(callB.get('alice')).toBeUndefined(); + expect(callB.get('own')).toBeUndefined(); + expect([ root.size, callA.size, callB.size ]).toEqual([ 2, 1, 0 ]); + + // Ending a sink frees its id everywhere. + aliceA.end(); + expect(root.get('alice')).toBeUndefined(); + + const aliceB = callB.create(RecordingSink, { id: 'alice' })!; + + root.clear(); + expect([ aliceB.endCalls, own.endCalls ]).toEqual([ 1, 1 ]); + expect([ root.size, callA.size, callB.size ]).toEqual([ 0, 0, 0 ]); + }); + + it('keeps a failing sink from crashing the process', () => { + const sink = new SinkRegistry().create(RecordingSink, { id: 'a' })!; + + expect(() => sink.emit('error', new Error('disk full'))).not.toThrow(); + }); +}); + +describe('sinks on the observed objects', () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'sinks-')); + }); + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('writes a client\'s samples to the sink under its id in the call, and ends it when the client closes', () => { + const sinks: InMemorySink[] = []; + const observer = new Observer({ closeCallIfEmptyForMs: undefined }); + + observer.on('client-added', ({ observedCall, observedClient }) => { + sinks.push(observedCall.sinks.create(InMemorySink, { id: observedClient.clientId })!); + }); + observer.accept(sample('alice')); + + const call = observer.getObservedCall('call-1')!; + const client = call.getObservedClient('alice')!; + + expect(client.sink).toBe(sinks[0]); + expect(sinks[0].samples).toHaveLength(1); + + let closed = 0; + + sinks[0].on('close', () => ++closed); + client.close(); + + expect(sinks[0].ended).toBe(true); + expect(call.sinks.get('alice')).toBeUndefined(); + expect(closed).toBe(1); + + observer.close(); + expect(closed).toBe(1); + }); + + it('ends a router\'s sinks when the router closes, and lets the app end one when its transport closes', async () => { + const observer = new Observer(); + const router = fakeRouter(); + const observedRouter = observer.createObservedMediasoupRouter({ router })!; + const pipe1 = fakePipeTransport('pipe-1'); + const pipe2 = fakePipeTransport('pipe-2'); + + observedRouter.on('transport-sample-closed', ({ transport }) => observedRouter.sinks.get(transport.id)?.end()); + router.observer.emit('newtransport', pipe1); + router.observer.emit('newtransport', pipe2); + + const file1 = path.join(dir, 'pipe-1.jsonl'); + const sink1 = observedRouter.sinks.create(JsonlFileSink<{ bytesSent: number }[]>, { id: 'pipe-1', path: file1 })!; + const sink2 = observedRouter.sinks.create(RecordingSink, { id: 'pipe-2' })!; + + observedRouter.sinks.write('pipe-1', [ { bytesSent: 1 } ]); + sink1.write([ { bytesSent: 2 } ]); + + const flushed = new Promise((resolve) => sink1.once('close', () => resolve())); + + pipe1.observer.emit('close'); + await flushed; + + expect(observedRouter.sinks.get('pipe-1')).toBeUndefined(); + expect(fs.readFileSync(file1, 'utf8').trim().split('\n').map((line) => JSON.parse(line))).toEqual([ [ { bytesSent: 1 } ], [ { bytesSent: 2 } ] ]); + expect(sink2.ended).toBe(false); + + // mediasoup closes the router's transports first, then the router. + router.observer.emit('close'); + expect(sink2.endCalls).toBe(1); + observer.close(); + }); + + it('ends the call\'s own sinks when the call closes, and every remaining sink when the observer closes', () => { + const observer = new Observer({ closeCallIfEmptyForMs: undefined }); + const call = observer.createObservedCall({ callId: 'call-1' })!; + const observedRouter = observer.createObservedMediasoupRouter({ router: fakeRouter() })!; + const callSink = call.sinks.create(RecordingSink, { id: 'sfu' })!; + const routerSink = observedRouter.sinks.create(RecordingSink, { id: 'router-stats' })!; + const observerSink = observer.sinks.create(RecordingSink, { id: 'worker' })!; + + call.close(); + expect([ callSink.ended, routerSink.ended, observerSink.ended ]).toEqual([ true, false, false ]); + + // observer.close() does not close mediasoup routers, but it does end their sinks. + observer.close(); + expect([ callSink.endCalls, routerSink.endCalls, observerSink.endCalls ]).toEqual([ 1, 1, 1 ]); + expect(observedRouter.closed).toBe(false); + }); + + it('takes custom ClientSampleSink implementations', () => { + class HttpLikeSink extends EventEmitter implements ClientSampleSink { + public readonly id: string; + public readonly received: ClientSample[] = []; + public readonly url: string; + public ended = false; + + public constructor(settings: SinkSettings & { url: string }) { + super(); + this.id = settings.id; + this.url = settings.url; + } + + public write(s: ClientSample): boolean { + this.received.push(s); + + return true; + } + + public end(): void { + if (this.ended) return; + this.ended = true; + this.emit('ended'); + this.emit('close'); + } + } + let custom: HttpLikeSink | undefined; + const observer = new Observer(); + + observer.on('client-added', ({ observedCall, observedClient }) => { + custom = observedCall.sinks.create(HttpLikeSink, { id: observedClient.clientId, url: `https://x/${observedClient.clientId}` }); + }); + observer.accept(sample('bob')); + observer.close(); + + expect(custom?.url).toBe('https://x/bob'); + expect(custom?.received).toHaveLength(1); + expect(custom?.ended).toBe(true); + }); +}); diff --git a/tests/sinks.spec.ts b/tests/sinks.spec.ts index 9340573..9837b0d 100644 --- a/tests/sinks.spec.ts +++ b/tests/sinks.spec.ts @@ -2,8 +2,8 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { InMemorySink, createInMemorySink } from '../src/sinks/InMemorySink'; -import { JsonlFileSink, createJsonlFileSink, createJsonlFileSinkFactory } from '../src/sinks/JsonlFileSink'; -import type { ObservedCall } from '../src/ObservedCall'; +import { JsonlFileSink, createJsonlFileSink } from '../src/sinks/JsonlFileSink'; +import { Observer } from '../src/Observer'; import { makeSample } from './helpers/samples'; describe('InMemorySink', () => { @@ -72,12 +72,45 @@ describe('JsonlFileSink', () => { expect(fs.readFileSync(filePath, 'utf8').trim()).toBe('id=zed'); }); - it('factory derives the per-client file path from callId/clientId', async () => { - const factory = createJsonlFileSinkFactory({ directory: dir }); - const sink = factory({ clientId: 'cl', observedCall: { callId: 'cid' } as ObservedCall }) as JsonlFileSink; + it('createClientJsonlSink writes each client\'s samples to the path it returns, announced by client-sink-created', async () => { + const observer = new Observer({ + createClientJsonlSink: ({ observedCall, clientId }) => (clientId === 'skip' ? undefined : path.join(dir, `${observedCall.callId}__${clientId}.jsonl`)), + }); + const created: JsonlFileSink[] = []; - expect(sink.path).toBe(path.join(dir, 'cid__cl.jsonl')); - await flush(sink); + observer.on('client-sink-created', ({ sink }) => created.push(sink)); + observer.accept(makeSample({ callId: 'cid', clientId: 'cl' })); + observer.accept(makeSample({ callId: 'cid', clientId: 'skip' })); + + const call = observer.getObservedCall('cid')!; + + expect(created.map((s) => s.path)).toEqual([ path.join(dir, 'cid__cl.jsonl') ]); + expect(call.sinks.get('cl')).toBe(created[0]); + expect(call.getObservedClient('skip')?.sink).toBeUndefined(); + + const flushed = flush(created[0]); + + observer.close(); + await flushed; + + const lines = fs.readFileSync(path.join(dir, 'cid__cl.jsonl'), 'utf8').trim().split('\n'); + + expect(lines).toHaveLength(1); + expect(JSON.parse(lines[0]).clientId).toBe('cl'); + }); + + it('createClientJsonlSink opens no file for a client whose id is already a sink id in another call', () => { + const observer = new Observer({ + createClientJsonlSink: ({ observedCall, clientId }) => path.join(dir, `${observedCall.callId}__${clientId}.jsonl`), + }); + + observer.accept(makeSample({ callId: 'c1', clientId: 'dup' })); + observer.accept(makeSample({ callId: 'c2', clientId: 'dup' })); + + expect(observer.getObservedCall('c1')?.getObservedClient('dup')?.sink).toBeDefined(); + expect(observer.getObservedCall('c2')?.getObservedClient('dup')?.sink).toBeUndefined(); + expect(fs.existsSync(path.join(dir, 'c2__dup.jsonl'))).toBe(false); + observer.close(); }); it('createJsonlFileSink builds a JsonlFileSink', async () => { From e2091901157457558a9f4d5637a1a7b48771a458 Mon Sep 17 00:00:00 2001 From: balazskreith Date: Thu, 15 Oct 2026 11:01:44 +0300 Subject: [PATCH 2/9] feat: add extension stats management to Observer, ObservedCall, and ObservedMediasoupRouter --- CHANGELOG.md | 29 ++++ README.md | 41 ++++++ src/ObservedCall.ts | 20 ++- src/ObservedMediasoupRouter.ts | 23 ++- src/Observer.ts | 17 +++ src/index.ts | 4 + src/sinks/ExtensionStatsRegistry.ts | 90 ++++++++++++ src/sinks/SinkRegistry.ts | 13 +- src/sinks/index.ts | 3 + tests/extensionStats.spec.ts | 219 ++++++++++++++++++++++++++++ 10 files changed, 448 insertions(+), 11 deletions(-) create mode 100644 src/sinks/ExtensionStatsRegistry.ts create mode 100644 tests/extensionStats.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d394fa..f1a58d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,6 +128,35 @@ to read `sink.path`). For a sink you create yourself, you already hold it. **Unchanged:** `observedClient.sink` still returns the client's sink, and the client still ends it when it closes. +### Extension stats on `Observer`, `ObservedCall` and `ObservedMediasoupRouter` (new) + +The last data per id, for stats the library does not collect itself, written to the owner's sink +with the same id when there is one. The same API on all three owners: + +```ts +call.createExtensionStats({ id: 'call-1:sfu', createSink: [ JsonlFileSink, { path } ] }); +call.acceptExtensionStats('call-1:sfu', await collectSfuStats()); // the application polls +call.getExtensionStats('call-1:sfu'); // the last data +call.extensionStats.values; // Map +``` + +- `createExtensionStats({ id, createSink? })` registers the id. `createSink` is what + `sinks.create` takes (`[SinkClass, settings]`); the sink is created under `id`, ending and + replacing a sink the owner already holds under it. Returns the sink, or `undefined` (no + `createSink`, a closed owner, an id already in `extensionStats.values` — created or accepted + before —, or the id held as a sink by another object of the observer). +- `acceptExtensionStats(id, data)` keeps `data` in `extensionStats.values` and writes it + to the sink with the same id, if any — also one created later with `sinks.create`. It does not + require `createExtensionStats` first; an id accepted first gets a sink through `sinks.create`. +- `getExtensionStats(id)` returns the last data. +- Extension stats ids share the sink id namespace, so they must be unique per observer. +- `extensionStats` on each owner is its `ExtensionStatsRegistry` (`create`, `accept`, `get`, + `values`, `closed`, `close()`), which the three methods above delegate to. The owner closes it when + it closes: data accepted afterwards is dropped, `create` is refused, and the extension stats' + sinks are ended (the owner's other sinks end with its `sinks.clear()`). +- `ExtensionStatsRegistry`, `SinkConstructor`, `SinkCreateSettings` and + `CreateExtensionStatsSettings` are exported. + ### Other - `percentileOfHistogram` narrows the bucket holding the observed `min` / `max` to the observed diff --git a/README.md b/README.md index 8e01acd..aa56189 100644 --- a/README.md +++ b/README.md @@ -2029,6 +2029,47 @@ warning is logged. The library calls `write(sample)` **synchronously** per accepted sample (it is not awaited), so **backpressure and batching are the sink's concern**. +### Extension stats + +Stats the library does not collect itself can be kept as **extension stats** on the object they +belong to — `observer` (e.g. a mediasoup worker's resource usage), `observedCall` (an SFU's view of +the call) or `observedMediasoupRouter` (a pipe transport's `getStats()`): the last data per id, and +optionally a sink that receives every data. The API is the same on all three; a call is shown. + +```ts +// Register the id; with `createSink` (what `sinks.create` takes), a sink is created under the same id. +call.createExtensionStats({ id: `${call.callId}:sfu`, createSink: [ JsonlFileSink, { path: './stats/sfu.jsonl' } ] }); + +// Your own polling: keep the last data, and write it to the sink with the same id, if any. +call.acceptExtensionStats(`${call.callId}:sfu`, await collectSfuStats(call.callId)); + +call.getExtensionStats(`${call.callId}:sfu`); // the last data +call.extensionStats.values; // Map +``` + +| Method | Behaviour | +|--------|-----------| +| `createExtensionStats({ id, createSink? })` | Registers `id`. With `createSink` — what `sinks.create` takes, `[SinkClass, settings]` — it also creates a sink under `id` (an `id` in the sink settings is ignored), ending and replacing a sink the owner already holds under `id`. Returns the created sink, or `undefined`: without `createSink`, on a closed owner, when `id` is already in `extensionStats.values` (created or accepted before), or when another object of the observer holds a sink with this id | +| `acceptExtensionStats(id, data)` | Keeps `data` as the last data of `id` and writes it to the owner's sink with the same id, if there is one (a sink that throws is logged). `id` does not have to be created first. Dropped once the owner is closed | +| `getExtensionStats(id)` | The last data accepted for `id`, or `undefined` | +| `extensionStats` | The owner's `ExtensionStatsRegistry`: `values` (`Map`), `closed`, `close()`, and `create` / `accept` / `get`, which the three methods above delegate to | + +**Extension stats ids share the sink id namespace, so they must be unique per observer**: the sink +of extension stats `x` is the sink with id `x`. Consequently: + +- With `createSink`, a sink the owner already holds under the id is ended and replaced. If another + object of the observer holds it, no sink is created and the data is only kept. +- Without `createSink`, a sink you create later under the id (`sinks.create(SinkClass, { id, ... })`) + receives the data accepted from then on. +- An id accepted before it was created can no longer be created with `createSink`; give it a sink + with `sinks.create` instead. + +**Lifecycle.** When the owner (the call, the router or the observer) closes, it closes its +`extensionStats`: data accepted afterwards is dropped, `createExtensionStats` returns `undefined`, +and the extension stats' sinks are ended. The owner's other sinks end with its `sinks.clear()` as +usual. `extensionStats.close()` can also be called directly; it ends only the extension stats' +sinks. + ### Other records: e.g. mediasoup pipe transport stats ```ts diff --git a/src/ObservedCall.ts b/src/ObservedCall.ts index 3f55581..899ea2f 100644 --- a/src/ObservedCall.ts +++ b/src/ObservedCall.ts @@ -1,5 +1,8 @@ import { EventEmitter } from 'node:events'; import { SinkRegistry } from './sinks/SinkRegistry'; +import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; +import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; +import type { SinkConstructor } from './sinks/SinkRegistry'; import { JsonlFileSink } from './sinks/JsonlFileSink'; import type { ClientSample } from './schema/ClientSample'; import { createLogger } from './common/logger'; @@ -111,6 +114,7 @@ export class ObservedCall = Record; @@ -125,6 +129,7 @@ export class ObservedCall = Record = Record = Record(settings: CreateExtensionStatsSettings): InstanceType | undefined { + return this.extensionStats.create(settings); + } + + public acceptExtensionStats(id: string, data: unknown): void { + this.extensionStats.accept(id, data); + } + + public getExtensionStats(id: string): T | undefined { + return this.extensionStats.get(id); + } + public getObservedClient = Record>(clientId: string): ObservedClient | undefined { if (this.closed || !this.observedClients.has(clientId)) return; diff --git a/src/ObservedMediasoupRouter.ts b/src/ObservedMediasoupRouter.ts index ba452f6..4904fa8 100644 --- a/src/ObservedMediasoupRouter.ts +++ b/src/ObservedMediasoupRouter.ts @@ -13,6 +13,9 @@ import { } from './schema/MediasoupRouter'; import { createLogger } from './common/logger'; import { SinkRegistry } from './sinks/SinkRegistry'; +import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; +import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; +import type { SinkConstructor } from './sinks/SinkRegistry'; const logger = createLogger('ObservedMediasoupRouter'); @@ -95,12 +98,8 @@ export class ObservedMediasoupRouter = R public readonly webrtcTransportIds = new Set(); public closed = false; - /** - * The router's sinks, e.g. one per pipe transport for its `getStats()`. They are ended when the - * router closes, which is usually the lifetime wanted; `sink.end()` ends one earlier. Writing to - * them, and polling what is written, is the application's. Its parent is `observer.sinks`. - */ public readonly sinks: SinkRegistry; + public readonly extensionStats: ExtensionStatsRegistry; // Id -> sample indexes over the arrays in `this.sample`, so an entity can be reached in O(1) // instead of scanning (`sample.producers.find(...)`). They hold the *same* objects as the arrays. @@ -125,6 +124,7 @@ export class ObservedMediasoupRouter = R this.appData = (settings.appData ?? {}) as AppData; this._enrich = settings.enrich; this.sinks = new SinkRegistry(parentSinks); + this.extensionStats = new ExtensionStatsRegistry(this.router.id, this.sinks); this.sample = { routerId: this.router.id, attachments: settings.attachments ?? {}, @@ -218,10 +218,23 @@ export class ObservedMediasoupRouter = R this.closed = true; if (this.sample.closedAt === undefined) this.sample.closedAt = Date.now(); + this.extensionStats.close(); this.sinks.clear(); this.emit('close'); } + public createExtensionStats(settings: CreateExtensionStatsSettings): InstanceType | undefined { + return this.extensionStats.create(settings); + } + + public acceptExtensionStats(id: string, data: unknown): void { + this.extensionStats.accept(id, data); + } + + public getExtensionStats(id: string): T | undefined { + return this.extensionStats.get(id); + } + public addTransport = (transport: types.Transport) => { let transportType = transport.type; diff --git a/src/Observer.ts b/src/Observer.ts index d81e9b0..56e2efa 100644 --- a/src/Observer.ts +++ b/src/Observer.ts @@ -8,6 +8,9 @@ import type { ObserverEvents, ObserverEventBase, ObservedMediasoupRouterScope } import { Middleware, MiddlewareProcessor } from './common/Middleware'; import { ObservedMediasoupRouter, ObservedMediasoupRouterSettings } from './ObservedMediasoupRouter'; import { SinkRegistry } from './sinks/SinkRegistry'; +import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; +import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; +import type { SinkConstructor } from './sinks/SinkRegistry'; import { AvailableCallScopeDetectorsConfigs, AvailableObserverScopeDetectorsConfigs, Detectors } from './detectors/Detectors'; import type { AvailableValidatorConfigs } from './validators/Validators'; import type { ValidationReport, RunningValidator } from './validators/Validator'; @@ -281,6 +284,7 @@ export class Observer = Record(); public readonly sinks = new SinkRegistry(); + public readonly extensionStats = new ExtensionStatsRegistry('observer', this.sinks); /** Ancestry base shared by all Observer-bus events originating at the observer. */ public readonly eventScope: ObserverEventBase = { observer: this }; @@ -731,11 +735,24 @@ export class Observer = Record(settings: CreateExtensionStatsSettings): InstanceType | undefined { + return this.extensionStats.create(settings); + } + + public acceptExtensionStats(id: string, data: unknown): void { + this.extensionStats.accept(id, data); + } + + public getExtensionStats(id: string): T | undefined { + return this.extensionStats.get(id); + } + public accept(sample: ClientSample, context?: AcceptContext): void { if (this.closed) { return this._notify('sample-rejected', { ...this.eventScope, reason: 'observer-closed', sample }); diff --git a/src/index.ts b/src/index.ts index d3b3b1f..9cdec25 100644 --- a/src/index.ts +++ b/src/index.ts @@ -299,6 +299,7 @@ export type { Logger, ObserverLogger } from './common/logger'; // `JsonlFileSink` / `InMemorySink` are the built-ins; `SinkRegistry` owns their lifecycle. export { SinkRegistry, + ExtensionStatsRegistry, JsonlFileSink, createJsonlFileSink, InMemorySink, @@ -308,6 +309,9 @@ export type { Sink, SinkEvents, SinkSettings, + SinkConstructor, + SinkCreateSettings, + CreateExtensionStatsSettings, ClientSampleSink, ClientSampleSinkEvents, JsonlFileSinkOptions, diff --git a/src/sinks/ExtensionStatsRegistry.ts b/src/sinks/ExtensionStatsRegistry.ts new file mode 100644 index 0000000..1166ff1 --- /dev/null +++ b/src/sinks/ExtensionStatsRegistry.ts @@ -0,0 +1,90 @@ +import type { SinkConstructor, SinkCreateSettings, SinkRegistry } from './SinkRegistry'; +import { createLogger } from '../common/logger'; + +const logger = createLogger('ExtensionStats'); + +/** The settings of `createExtensionStats` on an observer, a call or a mediasoup router. */ +export type CreateExtensionStatsSettings = { + + /** Unique per observer: it is also the id of the sink that receives the stats. */ + id: string, + + /** What `sinks.create` takes; the sink is created under `id` (an `id` in its settings is ignored). */ + createSink?: [SinkClass: C, settings: Omit, 'id'>], +}; + +/** + * The extension stats of one object — `observer.extensionStats`, `observedCall.extensionStats`, + * `observedMediasoupRouter.extensionStats`: the last data per id (`values`), written to the + * object's sink with the same id. The owner closes it when it closes. + */ +export class ExtensionStatsRegistry { + /** The last data accepted for each extension stats id. */ + public readonly values = new Map(); + public closed = false; + + public constructor( + /** The owner's id (call id, router id, or `observer`), used in log messages. */ + public readonly id: string, + private readonly _sinks: SinkRegistry, + ) { + } + + /** + * Refuse further `create` / `accept`, and end the extension stats' sinks (the sinks whose id is + * an extension stats id). Other sinks of the owner, e.g. client sample sinks, are left to the + * owner's `sinks.clear()`. Called by the owner when it closes. + */ + public close(): void { + if (this.closed) return; + + this.closed = true; + + for (const id of this.values.keys()) { + this._sinks.get(id)?.end(); + } + } + + public create(settings: CreateExtensionStatsSettings): InstanceType | undefined { + if (this.closed) { + logger.warn('Attempted to create extension stats %s on closed %s', settings.id, this.id); + + return undefined; + } + if (this.values.has(settings.id)) { + logger.warn('Extension stats with id %s already exists on %s', settings.id, this.id); + + return undefined; + } + + this.values.set(settings.id, undefined); + + if (!settings.createSink) return undefined; + + const [ SinkClass, sinkSettings ] = settings.createSink; + + this._sinks.get(settings.id)?.end(); + + return this._sinks.create(SinkClass, { ...sinkSettings, id: settings.id } as SinkCreateSettings); + } + + public accept(id: string, data: unknown): void { + if (this.closed) { + logger.debug('Extension stats %s dropped: %s is closed', id, this.id); + + return; + } + + this.values.set(id, data); + + try { + this._sinks.get(id)?.write(data); + } catch (err) { + logger.warn('Failed to write extension stats %s to sink: %o', id, err); + } + } + + public get(id: string): T | undefined { + return this.values.get(id) as T | undefined; + } +} diff --git a/src/sinks/SinkRegistry.ts b/src/sinks/SinkRegistry.ts index d8840ec..4f7fe65 100644 --- a/src/sinks/SinkRegistry.ts +++ b/src/sinks/SinkRegistry.ts @@ -4,6 +4,13 @@ import { createLogger } from '../common/logger'; const logger = createLogger('SinkRegistry'); +/** A sink class `SinkRegistry.create` can construct: its constructor takes one settings object (with `id`). */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type SinkConstructor = new(settings: any) => Sink; + +/** The settings `create` takes for `SinkClass`: its constructor's, with `id` optional. */ +export type SinkCreateSettings = Omit[0], 'id'> & { id?: string }; + /** * The sinks of one object, ended by that object's lifecycle. `create(SinkClass, settings)` builds a * sink and registers it; `sink.end()` closes it and removes it; the owner calls `clear()` when it @@ -50,11 +57,7 @@ export class SinkRegistry { * omitted) is checked first, so nothing is constructed — no file is opened — when it is taken: * `undefined` is returned and a warning logged. */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any - public create Sink>( - SinkClass: C, - settings: Omit[0], 'id'> & { id?: string }, - ): InstanceType | undefined { + public create(SinkClass: C, settings: SinkCreateSettings): InstanceType | undefined { const lineage = this._lineage(); const id = settings.id ?? uuidv4(); diff --git a/src/sinks/index.ts b/src/sinks/index.ts index 2daa0eb..f69bd34 100644 --- a/src/sinks/index.ts +++ b/src/sinks/index.ts @@ -1,7 +1,10 @@ // Internal barrel for the sinks, re-exported from the package root. export type { Sink, SinkEvents, SinkSettings } from './Sink'; export { SinkRegistry } from './SinkRegistry'; +export type { SinkConstructor, SinkCreateSettings } from './SinkRegistry'; export type { ClientSampleSink, ClientSampleSinkEvents } from './ClientSampleSink'; export { JsonlFileSink, createJsonlFileSink } from './JsonlFileSink'; export type { JsonlFileSinkOptions } from './JsonlFileSink'; export { InMemorySink, createInMemorySink } from './InMemorySink'; +export { ExtensionStatsRegistry } from './ExtensionStatsRegistry'; +export type { CreateExtensionStatsSettings } from './ExtensionStatsRegistry'; diff --git a/tests/extensionStats.spec.ts b/tests/extensionStats.spec.ts new file mode 100644 index 0000000..d65e2ec --- /dev/null +++ b/tests/extensionStats.spec.ts @@ -0,0 +1,219 @@ +import { EventEmitter } from 'node:events'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { Observer } from '../src/Observer'; +import { JsonlFileSink } from '../src/sinks/JsonlFileSink'; +import { InMemorySink } from '../src/sinks/InMemorySink'; +import type { ClientSample } from '../src/schema/ClientSample'; + +type SfuStats = { bitrate: number }; + +function setup() { + const observer = new Observer({ closeCallIfEmptyForMs: undefined }); + const call = observer.createObservedCall({ callId: 'call-1' })!; + + return { observer, call }; +} + +// InMemorySink is typed for client samples; the records here are arbitrary. +const records = (sink: InMemorySink) => sink.samples as unknown[]; + +describe('ObservedCall extension stats', () => { + let dir: string; + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'extension-stats-')); + }); + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('keeps the last data per id, with no sink required', () => { + const { observer, call } = setup(); + + expect(call.createExtensionStats({ id: 'sfu' })).toBeUndefined(); + expect(call.extensionStats.values.has('sfu')).toBe(true); + expect(call.getExtensionStats('sfu')).toBeUndefined(); + + call.acceptExtensionStats('sfu', { bitrate: 1 }); + call.acceptExtensionStats('sfu', { bitrate: 2 }); + // Not created first: accepted all the same. + call.acceptExtensionStats('other', { bitrate: 3 }); + + expect(call.getExtensionStats('sfu')).toEqual({ bitrate: 2 }); + expect(call.getExtensionStats('other')).toEqual({ bitrate: 3 }); + observer.close(); + }); + + it('writes every accepted data to the sink created with it, which ends with the call', async () => { + const { observer, call } = setup(); + const file = path.join(dir, 'sfu.jsonl'); + const sink = call.createExtensionStats({ id: 'sfu', createSink: [ JsonlFileSink, { path: file } ] })!; + + expect(sink).toBeInstanceOf(JsonlFileSink); + expect(sink.id).toBe('sfu'); + expect(call.sinks.get('sfu')).toBe(sink); + + call.acceptExtensionStats('sfu', { bitrate: 1 }); + call.acceptExtensionStats('sfu', { bitrate: 2 }); + + const flushed = new Promise((resolve) => sink.once('close', () => resolve())); + + call.close(); + await flushed; + + expect(fs.readFileSync(file, 'utf8').trim().split('\n').map((line) => JSON.parse(line))).toEqual([ { bitrate: 1 }, { bitrate: 2 } ]); + expect(call.getExtensionStats('sfu')).toEqual({ bitrate: 2 }); + observer.close(); + }); + + it('replaces a sink the call already holds under the id when created with createSink', () => { + const { observer, call } = setup(); + const existing = call.sinks.create(InMemorySink, { id: 'sfu' })!; + const created = call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })!; + + expect(existing.ended).toBe(true); + expect(call.sinks.get('sfu')).toBe(created); + + call.acceptExtensionStats('sfu', { bitrate: 1 }); + expect(records(created)).toEqual([ { bitrate: 1 } ]); + expect(records(existing)).toEqual([]); + observer.close(); + }); + + it('writes to a sink created later under the same id, from then on', () => { + const { observer, call } = setup(); + + call.createExtensionStats({ id: 'sfu' }); + call.acceptExtensionStats('sfu', { bitrate: 1 }); + + const later = call.sinks.create(InMemorySink, { id: 'sfu' })!; + + call.acceptExtensionStats('sfu', { bitrate: 2 }); + expect(records(later)).toEqual([ { bitrate: 2 } ]); + observer.close(); + }); + + it('creates no sink when another object of the observer holds the id, and nothing on a closed call', () => { + const { observer, call } = setup(); + const other = observer.createObservedCall({ callId: 'call-2' })!; + const held = other.sinks.create(InMemorySink, { id: 'sfu' })!; + + expect(call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(held.ended).toBe(false); + + // The data is still kept on the call; it just has no sink here. + call.acceptExtensionStats('sfu', { bitrate: 1 }); + expect(call.getExtensionStats('sfu')).toEqual({ bitrate: 1 }); + expect(records(held)).toEqual([]); + + call.close(); + expect(call.createExtensionStats({ id: 'late', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + call.acceptExtensionStats('sfu', { bitrate: 2 }); + expect(call.getExtensionStats('sfu')).toEqual({ bitrate: 1 }); + observer.close(); + }); + + it('keeps a client sample sink apart from extension stats with a different id', () => { + const { observer, call } = setup(); + + call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] }); + observer.on('client-added', ({ observedCall, observedClient }) => { + observedCall.sinks.create(InMemorySink, { id: observedClient.clientId }); + }); + observer.accept({ callId: 'call-1', clientId: 'alice', timestamp: Date.now() } as ClientSample); + call.acceptExtensionStats('sfu', { bitrate: 1 }); + + expect(records(call.sinks.get('sfu')!)).toEqual([ { bitrate: 1 } ]); + expect(call.sinks.get('alice')!.samples).toHaveLength(1); + observer.close(); + }); +}); + +describe('extension stats ids', () => { + it('refuses to create an id twice, or one already accepted', () => { + const { observer, call } = setup(); + + call.createExtensionStats({ id: 'sfu' }); + expect(call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(call.sinks.get('sfu')).toBeUndefined(); + + call.acceptExtensionStats('accepted-first', { bitrate: 1 }); + expect(call.createExtensionStats({ id: 'accepted-first', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + + // The way to give it a sink afterwards. + const sink = call.sinks.create(InMemorySink, { id: 'accepted-first' })!; + + call.acceptExtensionStats('accepted-first', { bitrate: 2 }); + expect(records(sink)).toEqual([ { bitrate: 2 } ]); + observer.close(); + }); +}); + +describe('ObservedMediasoupRouter extension stats', () => { + const fakeRouter = () => ({ id: 'router-1', observer: new EventEmitter() }) as any; + + it('keeps the last data, writes it to its sink, and ends the sink when the router closes', () => { + const observer = new Observer(); + const router = fakeRouter(); + const observedRouter = observer.createObservedMediasoupRouter({ router })!; + const sink = observedRouter.createExtensionStats({ id: 'pipe-1', createSink: [ InMemorySink, {} ] })!; + + observedRouter.acceptExtensionStats('pipe-1', [ { bytesSent: 1 } ]); + observedRouter.acceptExtensionStats('pipe-1', [ { bytesSent: 2 } ]); + + expect(observedRouter.getExtensionStats('pipe-1')).toEqual([ { bytesSent: 2 } ]); + expect(observedRouter.extensionStats.values.size).toBe(1); + expect(records(sink)).toEqual([ [ { bytesSent: 1 } ], [ { bytesSent: 2 } ] ]); + + // Unique per observer: the call cannot take the router's id. + const call = observer.createObservedCall({ callId: 'call-1' })!; + + expect(call.createExtensionStats({ id: 'pipe-1', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(sink.ended).toBe(false); + + router.observer.emit('close'); + expect(sink.ended).toBe(true); + observedRouter.acceptExtensionStats('pipe-1', [ { bytesSent: 3 } ]); + expect(observedRouter.getExtensionStats('pipe-1')).toEqual([ { bytesSent: 2 } ]); + expect(observedRouter.createExtensionStats({ id: 'late' })).toBeUndefined(); + observer.close(); + }); +}); + +describe('Observer extension stats', () => { + it('keeps the last data, writes it to its sink, and ends the sink when the observer closes', () => { + const observer = new Observer(); + const sink = observer.createExtensionStats({ id: 'worker-1', createSink: [ InMemorySink, {} ] })!; + + observer.acceptExtensionStats('worker-1', { cpu: 12 }); + + expect(observer.getExtensionStats('worker-1')).toEqual({ cpu: 12 }); + expect(observer.sinks.get('worker-1')).toBe(sink); + expect(records(sink)).toEqual([ { cpu: 12 } ]); + + observer.close(); + expect(sink.ended).toBe(true); + observer.acceptExtensionStats('worker-1', { cpu: 99 }); + expect(observer.getExtensionStats('worker-1')).toEqual({ cpu: 12 }); + }); +}); + +describe('ExtensionStatsRegistry.close', () => { + it('ends only the extension stats sinks of its owner', () => { + const observer = new Observer({ closeCallIfEmptyForMs: undefined }); + const call = observer.createObservedCall({ callId: 'call-1' })!; + const statsSink = observer.createExtensionStats({ id: 'worker-1', createSink: [ InMemorySink, {} ] })!; + const otherSink = observer.sinks.create(InMemorySink, { id: 'other' })!; + const callSink = call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })!; + + observer.extensionStats.close(); + + expect(statsSink.ended).toBe(true); + expect([ otherSink.ended, callSink.ended ]).toEqual([ false, false ]); + expect(observer.createExtensionStats({ id: 'late' })).toBeUndefined(); + observer.close(); + expect([ otherSink.ended, callSink.ended ]).toEqual([ true, true ]); + }); +}); From 841ea212091263162d65d768a7c518cfd9e2f46d Mon Sep 17 00:00:00 2001 From: balazskreith Date: Thu, 15 Oct 2026 21:29:24 +0300 Subject: [PATCH 3/9] feat: implement release workflow and prepare-release script for versioning and changelog management --- .github/workflows/pages.yml | 109 ------------------------------ .github/workflows/publish.yaml | 117 ++++++++++++--------------------- .github/workflows/release.yml | 81 +++++++++++++++++++++++ AGENTS.md | 6 +- CONTRIBUTING.md | 23 +++++++ scripts/prepare-release.cjs | 26 ++++++++ 6 files changed, 175 insertions(+), 187 deletions(-) delete mode 100644 .github/workflows/pages.yml create mode 100644 .github/workflows/release.yml create mode 100644 scripts/prepare-release.cjs diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml deleted file mode 100644 index 85597a7..0000000 --- a/.github/workflows/pages.yml +++ /dev/null @@ -1,109 +0,0 @@ -name: Docs (GitHub Pages) - -# Publishes a minimal static site that serves `llms.txt` (the AI-doc index) at the site root, -# alongside the README, the CHANGELOG, and the `docs/` markdown — all verbatim -# (`.nojekyll`), so agents/crawlers can fetch them directly. Project Pages are served under a -# `//` subpath, so the discoverable file is e.g. -# https://.github.io/observer-js/llms.txt -# -# ONE-TIME MANUAL SETUP, required before the first run can succeed: -# repo → Settings → Pages → "Build and deployment" → Source: "GitHub Actions" -# -# There is no way to automate this from the workflow. `actions/configure-pages` has an `enablement` -# input that calls the API to create the Pages site, but that call needs repo-administration rights -# which `GITHUB_TOKEN` does not have and which `permissions:` cannot grant — it fails with -# "Resource not accessible by integration". - -on: - push: - branches: [master] - paths: - - 'llms.txt' - - 'README.md' - - 'CHANGELOG.md' - - 'AGENTS.md' - - 'docs/**' - - '.github/workflows/pages.yml' - workflow_dispatch: - -permissions: - contents: read - pages: write - id-token: write - -# Allow only one concurrent deployment; let an in-progress run finish. -concurrency: - group: pages - cancel-in-progress: false - -jobs: - deploy: - environment: - name: github-pages - url: ${{ steps.deployment.outputs.page_url }} - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v6 - - - name: Assemble site - run: | - set -e - mkdir -p _site/docs - # AI-doc index + one-file export, served verbatim at the site root. - cp llms.txt _site/ - # Human/agent-readable markdown, served raw (no Jekyll processing). - cp README.md CHANGELOG.md _site/ - [ -f AGENTS.md ] && cp AGENTS.md _site/ || true - cp docs/*.md _site/docs/ 2>/dev/null || true - # Disable Jekyll so .txt/.md are served as-is (not converted to HTML). - touch _site/.nojekyll - # Minimal landing page linking the docs. - cat > _site/index.html <<'HTML' - - - - - - @observertc/observer-js — docs - - - -

@observertc/observer-js

-

Server-side Node.js library for monitoring WebRTC sessions.

-

For AI agents

-
    -
  • llms.txt — curated index of the documentation
    • -
-

Documentation

- -

Source: github.com/ObserveRTC/observer-js · - Package: npm

- - - HTML - echo "Assembled _site:"; ls -R _site - - # NOTE no `enablement: true`. That option asks the API to CREATE the Pages site, which needs - # repo-administration rights the workflow's GITHUB_TOKEN does not have and cannot be granted - # through `permissions:` — it fails with "Resource not accessible by integration". Enabling - # Pages is a one-time manual step; see the header comment. - - name: Setup Pages - uses: actions/configure-pages@v6 - - - name: Upload artifact - uses: actions/upload-pages-artifact@v5 - with: - path: _site - - - name: Deploy to GitHub Pages - id: deployment - uses: actions/deploy-pages@v5 diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 3f3fd55..5f79499 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -1,92 +1,57 @@ name: Publish on: - push: - branches: - - master - - develop + pull_request: + branches: [master] + workflow_call: + inputs: + release-ref: + required: true + type: string -# Required for npm Trusted Publishing (OIDC). `id-token: write` lets the job mint the -# short-lived OIDC token npm exchanges for publish rights, and enables provenance attestations. -# No NPM_TOKEN secret is used anymore. permissions: contents: read - id-token: write jobs: - publish: + snapshot: + if: github.event_name == 'pull_request' runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - # NOTE deliberately no `registry-url`. That input makes setup-node write an `.npmrc` containing - # //registry.npmjs.org/:_authToken=${NODE_AUTH_TOKEN} - # and yarn 1 resolves env placeholders when it *parses* config, so it throws - # "Failed to replace env in config: ${NODE_AUTH_TOKEN}" before installing anything — and - # `NODE_AUTH_TOKEN` no longer exists, because publishing moved to Trusted Publishing (OIDC). - # npm defaults to registry.npmjs.org anyway, and mints its own short-lived credential. + with: + persist-credentials: false - uses: actions/setup-node@v7 with: node-version: 22 - - # Trusted Publishing requires npm >= 11.5.1, which is newer than the npm bundled with Node 22. - - name: Upgrade npm - run: npm install -g npm@latest - - - name: Install - run: yarn install --frozen-lockfile - - - name: Build - run: yarn build - - - name: Test - run: yarn test --ci - - - name: Resolve context - id: ctx - shell: bash + cache: yarn + - run: yarn install --frozen-lockfile + - run: yarn lint && yarn typecheck && yarn build && yarn test --ci + # Install the pkg.pr.new GitHub app on this repository first. + - name: Publish PR snapshot + run: npx --yes pkg-pr-new@0.0.54 publish --no-compact + + release: + if: github.event_name == 'workflow_dispatch' + permissions: + contents: read + id-token: write + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ inputs.release-ref }} + persist-credentials: false + # No registry-url: Yarn 1 cannot resolve an unset NODE_AUTH_TOKEN. + - uses: actions/setup-node@v7 + with: + node-version: 22 + cache: yarn + - run: npm install -g npm@11 + - run: yarn install --frozen-lockfile + - run: yarn lint && yarn typecheck && yarn build && yarn test --ci + - name: Publish version to npm run: | VERSION="$(node -p "require('./package.json').version")" - echo "branch=${GITHUB_REF##*/}" >> "$GITHUB_OUTPUT" - echo "version=$VERSION" >> "$GITHUB_OUTPUT" - echo "short_sha=$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT" - - # Skip (don't fail) if this exact version is already on npm — avoids a red build when a - # master push didn't bump the version. (develop publishes a unique snapshot, so it's exempt.) - - name: Check if version already published - id: published - if: steps.ctx.outputs.branch == 'master' - shell: bash - run: | - NAME="$(node -p "require('./package.json').name")" - if npm view "$NAME@${{ steps.ctx.outputs.version }}" version >/dev/null 2>&1; then - echo "Version $NAME@${{ steps.ctx.outputs.version }} already published — skipping publish." - echo "exists=true" >> "$GITHUB_OUTPUT" - else - echo "exists=false" >> "$GITHUB_OUTPUT" - fi - - # master -> always "latest", whatever the version looks like. - # - # master is the release line: whatever is on it is what `npm install @observertc/observer-js` - # should give you. The dist-tag is therefore a property of the *branch*, not of the version - # string — a prerelease version on master is still the current release. - # - # Note npm permits a prerelease version under "latest"; it is a convention that stops it, not a - # rule. The consequence is deliberate and worth being clear about: while `package.json` reads - # e.g. 1.0.0-beta.20, a plain `npm install` resolves to that beta. Publish prereleases you do - # *not* want installed by default from a branch other than master. - # - # Auth is via OIDC (trusted publisher); provenance is attached automatically. - - name: Publish - if: steps.ctx.outputs.branch == 'master' && steps.published.outputs.exists == 'false' - run: npm publish --access public --ignore-scripts --tag latest - - # develop -> publish a per-commit release candidate under a custom dist-tag. - - name: Make snapshot version - if: steps.ctx.outputs.branch != 'master' - run: npm version prerelease --preid=${{ steps.ctx.outputs.short_sha }} --git-tag-version false - - - name: Publish (release candidate) - if: steps.ctx.outputs.branch != 'master' - run: npm publish --access public --ignore-scripts --tag "$(echo "develop-${{ steps.ctx.outputs.version }}-rc" | sed 's/[.]//g')" + TAG=latest + if [[ "$VERSION" == *-* ]]; then TAG=beta; fi + npm publish --access public --ignore-scripts --tag "$TAG" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..3544aa6 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,81 @@ +name: Release + +on: + workflow_dispatch: + inputs: + version: + description: 'Exact version to publish, without v (for example 1.3.0)' + required: true + type: string + +permissions: + contents: read + +concurrency: + group: release + cancel-in-progress: false + +jobs: + prepare: + if: github.ref == 'refs/heads/master' + permissions: + contents: write + runs-on: ubuntu-latest + outputs: + tag: ${{ steps.prepare.outputs.tag }} + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + - uses: actions/setup-node@v7 + with: + node-version: 22 + cache: yarn + - run: yarn install --frozen-lockfile + - name: Finalize version and changelog + id: prepare + env: + RELEASE_VERSION: ${{ inputs.version }} + run: | + node scripts/prepare-release.cjs + echo "tag=v$RELEASE_VERSION" >> "$GITHUB_OUTPUT" + - run: yarn lint && yarn typecheck && yarn build && yarn test --ci + - name: Commit and tag release + env: + RELEASE_VERSION: ${{ inputs.version }} + run: | + git config user.name 'github-actions[bot]' + git config user.email '41898282+github-actions[bot]@users.noreply.github.com' + git add package.json CHANGELOG.md + git commit -m "chore: release $RELEASE_VERSION" + git tag -a "v$RELEASE_VERSION" -m "Release $RELEASE_VERSION" + git push --atomic origin HEAD:master "v$RELEASE_VERSION" + + publish: + needs: prepare + permissions: + contents: read + id-token: write + uses: ./.github/workflows/publish.yaml + with: + release-ref: ${{ needs.prepare.outputs.tag }} + + release: + needs: [prepare, publish] + permissions: + contents: write + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ needs.prepare.outputs.tag }} + - name: Extract release notes + run: node scripts/prepare-release.cjs --notes + - name: Create GitHub release + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ needs.prepare.outputs.tag }} + run: | + FLAGS=() + if [[ "$RELEASE_TAG" == *-* ]]; then FLAGS+=(--prerelease); fi + gh release create "$RELEASE_TAG" --verify-tag --title "$RELEASE_TAG" --notes-file release-notes.md "${FLAGS[@]}" diff --git a/AGENTS.md b/AGENTS.md index 48ef5b3..4ca6176 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,8 +38,10 @@ yarn test:coverage # jest --coverage (enforces the coverage thresholds in j `prepublishOnly` runs lint + typecheck; `prepare` runs the build. CI (`.github/workflows/ci.yml`) runs lint + typecheck + **build** + `test:coverage` on every push/PR. -Publishing (`.github/workflows/publish.yaml`) uses npm **Trusted Publishing (OIDC) + provenance** on -pushes to `master` (prerelease versions publish under the `beta` dist-tag). +Publishing (`.github/workflows/publish.yaml`) creates pkg.pr.new snapshots for PRs targeting +`master`. Manual `Release` on `master` takes an exact version, commits the finalized changelog +and package version, tags it, publishes through npm Trusted Publishing, and creates a GitHub release. +See `CONTRIBUTING.md` for setup and recovery. --- diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d96a038..8b88ad4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -23,3 +23,26 @@ Once you are ready with your changes: - Send us a [pull request](https://help.github.com/articles/creating-a-pull-request) + +## Snapshots and releases + +PRs targeting `master` run Publish and upload an installable snapshot to pkg.pr.new. +Install the [pkg.pr.new GitHub app](https://github.com/apps/pkg-pr-new) on this repository +once; snapshot install URLs appear on the PR. These snapshots do not consume npm versions. +Pushes and merges do not publish packages. The retired Pages workflow has been removed. + +To release, merge the changes into `master`, then open Actions → Release → Run workflow, +select `master`, and enter the exact version (for example `1.3.0` or `1.4.0-beta.1`). +Keep release notes under `## Unreleased` in `CHANGELOG.md`. The workflow validates the +version and notes, runs checks, commits the package version and dated changelog section, +creates `v`, publishes to npm, and creates a GitHub release using those same notes. +Stable versions use `latest`; prereleases use `beta`. A fresh Unreleased section is retained. + +npm Trusted Publishing must authorize `publish.yaml` in this repository (the reusable +workflow that actually publishes). No npm token is needed. Repository rules must allow +`github-actions[bot]` to push the release commit to master and create tags. Manual dispatch +requires the workflow to exist on the default branch; releases run only from master. + +If publishing or GitHub release creation fails after the tag is pushed, fix the cause and +use **Re-run failed jobs** on that run. Do not start a fresh release for the same version: +npm versions are immutable and the release preparation rejects existing tags. diff --git a/scripts/prepare-release.cjs b/scripts/prepare-release.cjs new file mode 100644 index 0000000..b472255 --- /dev/null +++ b/scripts/prepare-release.cjs @@ -0,0 +1,26 @@ +const fs = require('node:fs'); +const { execFileSync } = require('node:child_process'); +const semver = require('semver'); + +const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8')); +const changelog = fs.readFileSync('CHANGELOG.md', 'utf8'); +if (process.argv.includes('--notes')) { + const heading = `## ${pkg.version} — `; + const start = changelog.indexOf(heading); + if (start < 0) throw new Error('Release section not found'); + const body = changelog.slice(changelog.indexOf('\n', start) + 1); + fs.writeFileSync('release-notes.md', body.split(/^## /m)[0].trim() + '\n'); +} else { + const version = process.env.RELEASE_VERSION; + if (!version || semver.valid(version) !== version) throw new Error('Provide an exact SemVer without a v prefix'); + if (semver.lt(version, pkg.version)) throw new Error('Version must not go backwards'); + const tags = execFileSync('git', ['tag', '--list', `v${version}`], { encoding: 'utf8' }).trim(); + if (tags) throw new Error(`Tag v${version} already exists`); + const match = /^## Unreleased\s*\n([\s\S]*?)(?=^## |$(?![\s\S]))/m.exec(changelog); + if (!match || !match[1].trim()) throw new Error('Unreleased changelog must contain release notes'); + if (changelog.includes(`## ${version} — `)) throw new Error('Changelog already contains this version'); + const date = new Date().toISOString().slice(0, 10); + fs.writeFileSync('CHANGELOG.md', changelog.replace(match[0], `## Unreleased\n\n## ${version} — ${date}\n\n${match[1].trim()}\n\n`)); + pkg.version = version; + fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n'); +} From bb808da797d9e51c3e8b5e8944733a7a8d9ca795 Mon Sep 17 00:00:00 2001 From: balazskreith Date: Thu, 15 Oct 2026 21:35:53 +0300 Subject: [PATCH 4/9] feat: implement snapshot publishing workflow and testing for PRs targeting master --- .github/workflows/ci.yml | 3 + .github/workflows/publish.yaml | 36 +++++++--- .github/workflows/snapshot.yml | 32 +++++++++ AGENTS.md | 4 +- CONTRIBUTING.md | 23 +++++-- scripts/publish-snapshot.py | 109 +++++++++++++++++++++++++++++++ scripts/test_publish_snapshot.py | 102 +++++++++++++++++++++++++++++ 7 files changed, 292 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/snapshot.yml create mode 100644 scripts/publish-snapshot.py create mode 100644 scripts/test_publish_snapshot.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0df4cb6..354fb55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,9 @@ jobs: node-version: 22 cache: yarn + - name: Test snapshot publisher + run: python3 -m unittest discover -s scripts -p 'test_*.py' + - name: Install run: yarn install --frozen-lockfile diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index 5f79499..114c953 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -1,8 +1,9 @@ name: Publish on: - pull_request: - branches: [master] + workflow_run: + workflows: [Snapshot package] + types: [completed] workflow_call: inputs: release-ref: @@ -14,21 +15,38 @@ permissions: jobs: snapshot: - if: github.event_name == 'pull_request' + if: >- + github.event_name == 'workflow_run' && + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'pull_request' + permissions: + actions: read + contents: read + id-token: write runs-on: ubuntu-latest steps: + # Use the trusted default-branch scripts, never execute code from the PR here. - uses: actions/checkout@v6 with: persist-credentials: false - uses: actions/setup-node@v7 with: node-version: 22 - cache: yarn - - run: yarn install --frozen-lockfile - - run: yarn lint && yarn typecheck && yarn build && yarn test --ci - # Install the pkg.pr.new GitHub app on this repository first. - - name: Publish PR snapshot - run: npx --yes pkg-pr-new@0.0.54 publish --no-compact + - run: npm install -g npm@11 + - name: Download tested PR package + uses: actions/download-artifact@v7 + with: + name: snapshot-package + path: ${{ runner.temp }}/snapshot-artifact + github-token: ${{ github.token }} + run-id: ${{ github.event.workflow_run.id }} + - name: Publish next npm snapshot + env: + SNAPSHOT_ARTIFACT: ${{ runner.temp }}/snapshot-artifact + SNAPSHOT_RUN_ID: ${{ github.event.workflow_run.id }} + SNAPSHOT_RUN_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} + NPM_CONFIG_USERCONFIG: ${{ runner.temp }}/snapshot.npmrc + run: python3 scripts/publish-snapshot.py release: if: github.event_name == 'workflow_dispatch' diff --git a/.github/workflows/snapshot.yml b/.github/workflows/snapshot.yml new file mode 100644 index 0000000..cd66cfe --- /dev/null +++ b/.github/workflows/snapshot.yml @@ -0,0 +1,32 @@ +name: Snapshot package + +on: + pull_request: + branches: [master] + +permissions: + contents: read + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + - uses: actions/setup-node@v7 + with: + node-version: 22 + cache: yarn + - run: yarn install --frozen-lockfile + - run: yarn lint && yarn typecheck && yarn build && yarn test --ci + - name: Pack tested snapshot + run: | + mkdir -p "$RUNNER_TEMP/snapshot-package" + npm pack --ignore-scripts --pack-destination "$RUNNER_TEMP/snapshot-package" + - uses: actions/upload-artifact@v7 + with: + name: snapshot-package + path: ${{ runner.temp }}/snapshot-package/*.tgz + if-no-files-found: error + retention-days: 7 diff --git a/AGENTS.md b/AGENTS.md index 4ca6176..40cf63a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,8 +38,8 @@ yarn test:coverage # jest --coverage (enforces the coverage thresholds in j `prepublishOnly` runs lint + typecheck; `prepare` runs the build. CI (`.github/workflows/ci.yml`) runs lint + typecheck + **build** + `test:coverage` on every push/PR. -Publishing (`.github/workflows/publish.yaml`) creates pkg.pr.new snapshots for PRs targeting -`master`. Manual `Release` on `master` takes an exact version, commits the finalized changelog +Snapshots (`.github/workflows/snapshot.yml`) build PRs targeting `master`; +`publish.yaml` publishes the tested artifact to npm as `-rc.` under `snapshot`. Manual `Release` on `master` takes an exact version, commits the finalized changelog and package version, tags it, publishes through npm Trusted Publishing, and creates a GitHub release. See `CONTRIBUTING.md` for setup and recovery. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8b88ad4..17f82a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,10 +26,20 @@ Once you are ready with your changes: ## Snapshots and releases -PRs targeting `master` run Publish and upload an installable snapshot to pkg.pr.new. -Install the [pkg.pr.new GitHub app](https://github.com/apps/pkg-pr-new) on this repository -once; snapshot install URLs appear on the PR. These snapshots do not consume npm versions. -Pushes and merges do not publish packages. The retired Pages workflow has been removed. +PRs targeting `master` build and test the package in Snapshot package. Publish then +publishes that artifact to npm as `@observertc/observer-js@-rc.`, +under the `snapshot` dist-tag. The base version comes from the PR's `package.json` +(with any prerelease suffix removed). For example, a package version of `1.3.0` +produces `1.3.0-rc.1`, then `1.3.0-rc.2`. The counter uses the highest existing rc +number on npm for that base version; simultaneous runs retry version collisions. +Install with `npm install @observertc/observer-js@snapshot` or the exact rc version. +The snapshot version is never committed or tagged, and `latest` is untouched. + +No external snapshot app or npm token is needed. The publisher runs separately on +`workflow_run`, so fork PR builds never receive publishing credentials or execute +scripts in the publishing job. Both workflows and the publisher script must be on +the default branch before automatic snapshots can run. Pushes and merges do not +publish packages. The retired Pages workflow has been removed. To release, merge the changes into `master`, then open Actions → Release → Run workflow, select `master`, and enter the exact version (for example `1.3.0` or `1.4.0-beta.1`). @@ -38,8 +48,9 @@ version and notes, runs checks, commits the package version and dated changelog creates `v`, publishes to npm, and creates a GitHub release using those same notes. Stable versions use `latest`; prereleases use `beta`. A fresh Unreleased section is retained. -npm Trusted Publishing must authorize `publish.yaml` in this repository (the reusable -workflow that actually publishes). No npm token is needed. Repository rules must allow +npm Trusted Publishing must authorize `publish.yaml` for snapshots and `release.yml` +for manual releases in `ObserveRTC/observer-js`. npm checks the calling workflow +for reusable workflows. Both publishers must allow direct `npm publish`. No npm token is needed. Repository rules must allow `github-actions[bot]` to push the release commit to master and create tags. Manual dispatch requires the workflow to exist on the default branch; releases run only from master. diff --git a/scripts/publish-snapshot.py b/scripts/publish-snapshot.py new file mode 100644 index 0000000..c7c7cd1 --- /dev/null +++ b/scripts/publish-snapshot.py @@ -0,0 +1,109 @@ +"""Publish a tested PR tarball without running PR code in the trusted job.""" +import json +import os +from pathlib import Path, PurePosixPath +import re +import subprocess +import tarfile +import tempfile +import time +from urllib.request import Request, urlopen + +PACKAGE = '@observertc/observer-js' +REGISTRY = 'https://registry.npmjs.org' + + +def next_version(base, versions): + pattern = re.compile(re.escape(base) + r'-rc\.(0|[1-9][0-9]*)$') + numbers = [int(match[1]) for version in versions if (match := pattern.fullmatch(version))] + return f'{base}-rc.{max(numbers, default=0) + 1}' + + +def registry_versions(): + request = Request(f'{REGISTRY}/@observertc%2fobserver-js', headers={ + 'Accept': 'application/json', 'Cache-Control': 'no-cache', + }) + # Registry/auth/network errors must fail rather than reset the counter. + with urlopen(request, timeout=60) as response: + return json.load(response)['versions'] + + +def unpack(archive, destination): + with tarfile.open(archive, 'r:gz') as package: + for member in package: + path = PurePosixPath(member.name) + if (path.is_absolute() or '..' in path.parts or not path.parts + or path.parts[0] != 'package'): + raise ValueError(f'Invalid package path: {member.name}') + if not (member.isfile() or member.isdir()): + raise ValueError(f'Unsupported package entry: {member.name}') + # Never let PR-provided npm configuration reach the publishing job. + if path.name == '.npmrc': + continue + target = destination.joinpath(*path.parts) + if member.isdir(): + target.mkdir(parents=True, exist_ok=True) + else: + target.parent.mkdir(parents=True, exist_ok=True) + with package.extractfile(member) as source: + target.write_bytes(source.read()) + target.chmod(member.mode & 0o777) + + +def publish(artifact_dir): + archives = list(Path(artifact_dir).glob('*.tgz')) + if len(archives) != 1: + raise ValueError('Expected exactly one tested package tarball') + run = f"{os.environ['SNAPSHOT_RUN_ID']}/{os.environ['SNAPSHOT_RUN_ATTEMPT']}" + with tempfile.TemporaryDirectory(prefix='observer-snapshot-') as temp: + unpack(archives[0], Path(temp)) + directory = Path(temp) / 'package' + manifest_path = directory / 'package.json' + manifest = json.loads(manifest_path.read_text()) + if manifest['name'] != PACKAGE: + raise ValueError(f'Expected package {PACKAGE}') + base = re.split(r'[-+]', manifest['version'], maxsplit=1)[0] + if not re.fullmatch(r'(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)', base): + raise ValueError('Package version must have a valid SemVer base') + manifest.pop('scripts', None) + manifest.pop('publishConfig', None) + manifest.pop('workspaces', None) + manifest['repository'] = { + 'type': 'git', 'url': 'git+https://github.com/ObserveRTC/observer-js.git', + } + manifest['observerSnapshotRun'] = run + # Concurrent PRs may read the same next number. Retry only confirmed + # collisions; authentication, network and other publish errors fail. + for attempt in range(10): + versions = registry_versions() + existing = next((v for v, data in versions.items() + if data.get('observerSnapshotRun') == run), None) + if existing: + print(f'Already published {PACKAGE}@{existing}', flush=True) + return + version = next_version(base, versions) + manifest['version'] = version + manifest_path.write_text(json.dumps(manifest, indent=2) + '\n') + result = subprocess.run([ + 'npm', 'publish', '--registry', REGISTRY, '--access', 'public', + '--ignore-scripts', '--tag', 'snapshot', + ], cwd=directory, check=False) + if result.returncode == 0: + install = f'npm install {PACKAGE}@{version}' + print(install, flush=True) + if os.environ.get('GITHUB_STEP_SUMMARY'): + with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as summary: + summary.write(f'Published snapshot: `{install}`\n') + return + published = registry_versions().get(version) + if published is not None and published.get('observerSnapshotRun') == run: + print(f'Confirmed published {PACKAGE}@{version}', flush=True) + return + if published is None: + raise RuntimeError(f'npm publish failed for {version} (exit {result.returncode})') + time.sleep(min(attempt + 1, 5)) + raise RuntimeError('Too many concurrent snapshot version collisions; re-run this job') + + +if __name__ == '__main__': + publish(os.environ['SNAPSHOT_ARTIFACT']) diff --git a/scripts/test_publish_snapshot.py b/scripts/test_publish_snapshot.py new file mode 100644 index 0000000..12ae9d2 --- /dev/null +++ b/scripts/test_publish_snapshot.py @@ -0,0 +1,102 @@ +import importlib.util +import io +import json +from pathlib import Path +import tarfile +import tempfile +import unittest +from unittest.mock import patch + +spec = importlib.util.spec_from_file_location('snapshot', Path(__file__).with_name('publish-snapshot.py')) +snapshot = importlib.util.module_from_spec(spec) +spec.loader.exec_module(snapshot) + + +class SnapshotTests(unittest.TestCase): + def test_counter_is_per_base_and_numeric(self): + versions = ['1.3.0-rc.2', '1.3.0-rc.10', '1.4.0-rc.99', '1.3.0-beta.50', '1.3.0'] + self.assertEqual(snapshot.next_version('1.3.0', versions), '1.3.0-rc.11') + self.assertEqual(snapshot.next_version('2.0.0', versions), '2.0.0-rc.1') + + def archive(self, directory, name='@observertc/observer-js'): + archive = Path(directory) / 'snapshot.tgz' + manifest = { + 'name': name, 'version': '1.3.0-beta.1', + 'scripts': {'prepublishOnly': 'exit 1'}, + 'publishConfig': {'registry': 'https://example.invalid'}, + } + with tarfile.open(archive, 'w:gz') as tar: + for path, data in [('package/package.json', json.dumps(manifest)), + ('package/.npmrc', 'registry=https://example.invalid')]: + content = data.encode() + member = tarfile.TarInfo(path) + member.size = len(content) + tar.addfile(member, io.BytesIO(content)) + return archive + + def test_publish_retries_collision_and_removes_pr_configuration(self): + seen = [] + + def npm(args, cwd, check): + manifest = json.loads((cwd / 'package.json').read_text()) + seen.append(manifest['version']) + self.assertNotIn('scripts', manifest) + self.assertNotIn('publishConfig', manifest) + self.assertFalse((cwd / '.npmrc').exists()) + self.assertIn('--ignore-scripts', args) + self.assertEqual(args[-2:], ['--tag', 'snapshot']) + return type('Result', (), {'returncode': 1 if len(seen) == 1 else 0})() + + with tempfile.TemporaryDirectory() as temp: + self.archive(temp) + with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}), \ + patch.object(snapshot, 'registry_versions', side_effect=[ + {}, {'1.3.0-rc.1': {}}, {'1.3.0-rc.1': {}}, + ]), patch.object(snapshot.subprocess, 'run', side_effect=npm), \ + patch.object(snapshot.time, 'sleep'): + snapshot.publish(temp) + self.assertEqual(seen, ['1.3.0-rc.1', '1.3.0-rc.2']) + + def test_already_published_run_is_not_published_again(self): + with tempfile.TemporaryDirectory() as temp: + self.archive(temp) + with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}), \ + patch.object(snapshot, 'registry_versions', return_value={ + '1.3.0-rc.7': {'observerSnapshotRun': '123/1'}, + }), patch.object(snapshot.subprocess, 'run') as npm: + snapshot.publish(temp) + npm.assert_not_called() + + def test_publish_error_is_not_treated_as_collision(self): + with tempfile.TemporaryDirectory() as temp: + self.archive(temp) + with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}), \ + patch.object(snapshot, 'registry_versions', return_value={}), \ + patch.object(snapshot.subprocess, 'run', return_value=type('Result', (), {'returncode': 1})()): + with self.assertRaisesRegex(RuntimeError, 'npm publish failed'): + snapshot.publish(temp) + + def test_rejects_path_traversal_and_links(self): + for name, kind in [('../escape', tarfile.REGTYPE), + ('/absolute', tarfile.REGTYPE), + ('package/link', tarfile.SYMTYPE)]: + with self.subTest(name=name), tempfile.TemporaryDirectory() as temp: + archive = Path(temp) / 'bad.tgz' + with tarfile.open(archive, 'w:gz') as tar: + member = tarfile.TarInfo(name) + member.type = kind + member.linkname = '/tmp' + tar.addfile(member) + with self.assertRaises(ValueError): + snapshot.unpack(archive, Path(temp) / 'out') + + def test_wrong_package_is_rejected(self): + with tempfile.TemporaryDirectory() as temp: + self.archive(temp, '@other/package') + with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}): + with self.assertRaisesRegex(ValueError, 'Expected package'): + snapshot.publish(temp) + + +if __name__ == '__main__': + unittest.main() From 466ec13594ce151ef5d90dd4c6c35d498a7847ea Mon Sep 17 00:00:00 2001 From: balazskreith Date: Thu, 15 Oct 2026 22:00:12 +0300 Subject: [PATCH 5/9] feat: enhance snapshot publishing with next minor version calculation and tests --- CONTRIBUTING.md | 7 ++++--- scripts/publish-snapshot.py | 22 +++++++++++++++------- scripts/test_publish_snapshot.py | 25 +++++++++++++++++++------ 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 17f82a9..b30cf32 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,9 +28,10 @@ Once you are ready with your changes: PRs targeting `master` build and test the package in Snapshot package. Publish then publishes that artifact to npm as `@observertc/observer-js@-rc.`, -under the `snapshot` dist-tag. The base version comes from the PR's `package.json` -(with any prerelease suffix removed). For example, a package version of `1.3.0` -produces `1.3.0-rc.1`, then `1.3.0-rc.2`. The counter uses the highest existing rc +under the `snapshot` dist-tag. The base version is the next minor after npm's +`latest` release, regardless of the PR's package version. For example, `latest` at +`1.2.0` (or `1.2.9`) produces `1.3.0-rc.1`, then `1.3.0-rc.2`. Once `1.3.0` is +released as `latest`, snapshots target `1.4.0-rc.1`. The counter uses the highest existing rc number on npm for that base version; simultaneous runs retry version collisions. Install with `npm install @observertc/observer-js@snapshot` or the exact rc version. The snapshot version is never committed or tagged, and `latest` is untouched. diff --git a/scripts/publish-snapshot.py b/scripts/publish-snapshot.py index c7c7cd1..7a84ba5 100644 --- a/scripts/publish-snapshot.py +++ b/scripts/publish-snapshot.py @@ -19,13 +19,20 @@ def next_version(base, versions): return f'{base}-rc.{max(numbers, default=0) + 1}' -def registry_versions(): +def next_minor(latest): + match = re.fullmatch(r'(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)', latest) + if not match: + raise ValueError('npm latest must point to a stable SemVer version') + return f'{match[1]}.{int(match[2]) + 1}.0' + + +def registry_metadata(): request = Request(f'{REGISTRY}/@observertc%2fobserver-js', headers={ 'Accept': 'application/json', 'Cache-Control': 'no-cache', }) # Registry/auth/network errors must fail rather than reset the counter. with urlopen(request, timeout=60) as response: - return json.load(response)['versions'] + return json.load(response) def unpack(archive, destination): @@ -62,9 +69,6 @@ def publish(artifact_dir): manifest = json.loads(manifest_path.read_text()) if manifest['name'] != PACKAGE: raise ValueError(f'Expected package {PACKAGE}') - base = re.split(r'[-+]', manifest['version'], maxsplit=1)[0] - if not re.fullmatch(r'(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)', base): - raise ValueError('Package version must have a valid SemVer base') manifest.pop('scripts', None) manifest.pop('publishConfig', None) manifest.pop('workspaces', None) @@ -74,8 +78,12 @@ def publish(artifact_dir): manifest['observerSnapshotRun'] = run # Concurrent PRs may read the same next number. Retry only confirmed # collisions; authentication, network and other publish errors fail. + metadata = registry_metadata() + base = next_minor(metadata['dist-tags']['latest']) for attempt in range(10): - versions = registry_versions() + if attempt: + metadata = registry_metadata() + versions = metadata['versions'] existing = next((v for v, data in versions.items() if data.get('observerSnapshotRun') == run), None) if existing: @@ -95,7 +103,7 @@ def publish(artifact_dir): with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as summary: summary.write(f'Published snapshot: `{install}`\n') return - published = registry_versions().get(version) + published = registry_metadata()['versions'].get(version) if published is not None and published.get('observerSnapshotRun') == run: print(f'Confirmed published {PACKAGE}@{version}', flush=True) return diff --git a/scripts/test_publish_snapshot.py b/scripts/test_publish_snapshot.py index 12ae9d2..aaab8a5 100644 --- a/scripts/test_publish_snapshot.py +++ b/scripts/test_publish_snapshot.py @@ -12,7 +12,20 @@ spec.loader.exec_module(snapshot) +def metadata(versions, latest='1.2.0'): + return {'dist-tags': {'latest': latest}, 'versions': versions} + + class SnapshotTests(unittest.TestCase): + def test_next_minor_uses_latest_stable_release(self): + self.assertEqual(snapshot.next_minor('1.2.0'), '1.3.0') + self.assertEqual(snapshot.next_minor('1.2.9'), '1.3.0') + self.assertEqual(snapshot.next_minor('1.3.0'), '1.4.0') + self.assertEqual(snapshot.next_minor('0.99.42'), '0.100.0') + for invalid in ['1.3.0-beta.1', 'v1.2.0', 'garbage', '01.2.0']: + with self.subTest(invalid=invalid), self.assertRaises(ValueError): + snapshot.next_minor(invalid) + def test_counter_is_per_base_and_numeric(self): versions = ['1.3.0-rc.2', '1.3.0-rc.10', '1.4.0-rc.99', '1.3.0-beta.50', '1.3.0'] self.assertEqual(snapshot.next_version('1.3.0', versions), '1.3.0-rc.11') @@ -21,7 +34,7 @@ def test_counter_is_per_base_and_numeric(self): def archive(self, directory, name='@observertc/observer-js'): archive = Path(directory) / 'snapshot.tgz' manifest = { - 'name': name, 'version': '1.3.0-beta.1', + 'name': name, 'version': '1.2.0', 'scripts': {'prepublishOnly': 'exit 1'}, 'publishConfig': {'registry': 'https://example.invalid'}, } @@ -50,8 +63,8 @@ def npm(args, cwd, check): with tempfile.TemporaryDirectory() as temp: self.archive(temp) with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}), \ - patch.object(snapshot, 'registry_versions', side_effect=[ - {}, {'1.3.0-rc.1': {}}, {'1.3.0-rc.1': {}}, + patch.object(snapshot, 'registry_metadata', side_effect=[ + metadata({}), metadata({'1.3.0-rc.1': {}}), metadata({'1.3.0-rc.1': {}}), ]), patch.object(snapshot.subprocess, 'run', side_effect=npm), \ patch.object(snapshot.time, 'sleep'): snapshot.publish(temp) @@ -61,9 +74,9 @@ def test_already_published_run_is_not_published_again(self): with tempfile.TemporaryDirectory() as temp: self.archive(temp) with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}), \ - patch.object(snapshot, 'registry_versions', return_value={ + patch.object(snapshot, 'registry_metadata', return_value=metadata({ '1.3.0-rc.7': {'observerSnapshotRun': '123/1'}, - }), patch.object(snapshot.subprocess, 'run') as npm: + })), patch.object(snapshot.subprocess, 'run') as npm: snapshot.publish(temp) npm.assert_not_called() @@ -71,7 +84,7 @@ def test_publish_error_is_not_treated_as_collision(self): with tempfile.TemporaryDirectory() as temp: self.archive(temp) with patch.dict('os.environ', {'SNAPSHOT_RUN_ID': '123', 'SNAPSHOT_RUN_ATTEMPT': '1'}), \ - patch.object(snapshot, 'registry_versions', return_value={}), \ + patch.object(snapshot, 'registry_metadata', return_value=metadata({})), \ patch.object(snapshot.subprocess, 'run', return_value=type('Result', (), {'returncode': 1})()): with self.assertRaisesRegex(RuntimeError, 'npm publish failed'): snapshot.publish(temp) From bdfe1672418e9e9238a31ad38fdece622155469e Mon Sep 17 00:00:00 2001 From: balazskreith Date: Fri, 16 Oct 2026 11:27:14 +0300 Subject: [PATCH 6/9] refactor: update client sink handling in Observer and ObservedCall - Replaced `createClientJsonlSink` with `createClientSink` in ObserverConfig to allow returning either a JSONL file path or a custom sink. - Updated ObservedCall to create client sinks based on the new configuration, ensuring unique sink IDs and proper handling of existing sinks. - Modified event emissions to reflect the new sink creation logic. - Adjusted tests to validate the new sink creation behavior and ensure proper functionality with existing and new clients. --- CHANGELOG.md | 65 +++++++++++----------- README.md | 57 +++++++++++++------- src/ObservedCall.ts | 36 ++++++++----- src/ObservedClient.ts | 2 +- src/ObservedMediasoupRouter.ts | 4 +- src/Observer.ts | 37 ++++--------- src/ObserverEvents.ts | 4 +- src/index.ts | 1 + src/sinks/ClientSampleSink.ts | 6 +-- src/sinks/ExtensionStatsRegistry.ts | 45 ++++++++-------- src/sinks/Sink.ts | 8 +++ src/sinks/SinkRegistry.ts | 84 +++++++++++++++++++++++++---- src/sinks/index.ts | 2 +- tests/extensionStats.spec.ts | 70 +++++++++++++++++++----- tests/sinks.spec.ts | 34 ++++++++++-- 15 files changed, 307 insertions(+), 148 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1a58d1..6c551c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,31 +33,39 @@ observedRouter.sinks.write(transport.id, await transport.getStats()); // or si - A client writes its samples to the sink under its client id in `observedCall.sinks` (`observedClient.sink` is now a getter for it) and ends it when the client closes. Client ids are therefore sink ids and must be unique across the observer's open calls. -- `createClientJsonlSink: ({ observedCall, clientId }) => string | undefined` (new `ObserverConfig` - option): return a path to write that client's samples to a `JsonlFileSink`. `client-sink-created` - announces the file (`sink: JsonlFileSink`). +- `ObserverConfig.createClientSink: ({ id, observedCall, clientId }) => SinkDestination | undefined` + decides the client's sample sink. A `SinkDestination` is exactly one of `{ jsonlPath }` — the + library creates a `JsonlFileSink` at that path under the client id — and `{ sink }` — your sink, + whose `id` must be `id` (the client id); `undefined` means none. Both is a compile error. It is called only when the id is free as a + sink id; a sink with a different id is ended and not used. `client-sink-created` announces it + (`sink: Sink`). +- `SinkRegistry.createFromDestination(id, destination)` registers what a `SinkDestination` + describes (`createClientSink` and `createExtensionStats` use it). +- `SinkRegistry.add(sink)` registers a sink constructed elsewhere under its own id, and + `owns(id)` tells whether an id was added to that registry itself rather than to a descendant. - `JsonlFileSink` and `createJsonlFileSink` take any record type. **Breaking** (see *Migrating from 1.2* below): -- `ObserverConfig.createClientSink` is removed. Use `createClientJsonlSink` for JSONL files, or - create your own sink under the client id on `client-added`. +- `ObserverConfig.createClientSink` now returns a `SinkDestination` — `{ jsonlPath }` or + `{ sink }` with the `id` it receives — instead of a sink; sinks implement the `Sink` interface + (below). - `ClientSampleSink` is no longer a class to extend: implement `Sink` (e.g. `extends EventEmitter implements ClientSampleSink`), take `SinkSettings` in the constructor, and emit `ended` from `end()`. `JsonlFileSink` and `InMemorySink` implement the interface instead of extending a base class. `JsonlFileSinkOptions` gained `id`; `new InMemorySink(samples)` is now `new InMemorySink({ id?, samples? })`. - `createJsonlFileSinkFactory`, `JsonlFileSinkFactoryOptions` and `ClientSampleSinkFactory` are - removed (`createClientJsonlSink` replaces the factory). -- `client-sink-created` now carries a `JsonlFileSink` and fires only for `createClientJsonlSink` - files. + removed: return `{ jsonlPath }` from `createClientSink`. +- `client-sink-created` carries a `Sink`. - `ObservedClient.sink` is a getter (read-only, as before). #### Migrating from 1.2 -**JSONL files per client** — `createClientSink: createJsonlFileSinkFactory(...)` becomes -`createClientJsonlSink`, which returns the path. The factory's default file name was -`${callId}__${clientId}.jsonl`; a `getFileName` option becomes the body of the callback. +**JSONL files per client** — `createClientSink: createJsonlFileSinkFactory(...)` becomes a +`createClientSink` that returns `{ jsonlPath }`. The factory's default file name was +`${callId}__${clientId}.jsonl`; a `getFileName` option becomes the body of the callback. For +`serializeSample`, return `{ sink: new JsonlFileSink({ id, path, serializeSample }) }` instead. ```ts // 1.2 @@ -65,24 +73,18 @@ new Observer({ createClientSink: createJsonlFileSinkFactory({ directory: './stat // 1.3 new Observer({ - createClientJsonlSink: ({ observedCall, clientId }) => path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`), + createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`) }), }); ``` -A `serializeSample` option has no equivalent in `createClientJsonlSink`; create the sink yourself -as below, with `sinks.create(JsonlFileSink, { id, path, serializeSample })`. - -**Any other client sink** — return it no longer from `createClientSink`; create it under the client -id when the client is added. The client writes its samples to it and ends it on close, as before. +**Any other client sink** — return it wrapped as `{ sink }`, and pass the `id` it receives to the sink. ```ts // 1.2 new Observer({ createClientSink: ({ clientId, observedCall }) => new HttpSink(urlOf(observedCall.callId, clientId)) }); // 1.3 -observer.on('client-added', ({ observedCall, observedClient }) => { - observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url: urlOf(observedCall.callId, observedClient.clientId) }); -}); +new Observer({ createClientSink: ({ id, clientId, observedCall }) => ({ sink: new HttpSink({ id, url: urlOf(observedCall.callId, clientId) }) }) }); ``` **Custom sink classes** — `class HttpSink extends ClientSampleSink` becomes an implementation of @@ -121,9 +123,8 @@ class HttpSink extends EventEmitter implements ClientSampleSink { } ``` -**`client-sink-created`** — still fires right after `client-added`, but only for the -`createClientJsonlSink` file, and its `sink` is a `JsonlFileSink` (no `instanceof` narrowing needed -to read `sink.path`). For a sink you create yourself, you already hold it. +**`client-sink-created`** — still fires right after `client-added` for the sink `createClientSink` +returned, typed `Sink`; narrow (`sink instanceof JsonlFileSink`) to read `sink.path`. **Unchanged:** `observedClient.sink` still returns the client's sink, and the client still ends it when it closes. @@ -134,17 +135,21 @@ The last data per id, for stats the library does not collect itself, written to with the same id when there is one. The same API on all three owners: ```ts -call.createExtensionStats({ id: 'call-1:sfu', createSink: [ JsonlFileSink, { path } ] }); +call.createExtensionStats({ id: 'call-1:sfu', jsonlPath: './stats/sfu.jsonl' }); // or sink: new MySink({ id: 'call-1:sfu' }) call.acceptExtensionStats('call-1:sfu', await collectSfuStats()); // the application polls call.getExtensionStats('call-1:sfu'); // the last data call.extensionStats.values; // Map ``` -- `createExtensionStats({ id, createSink? })` registers the id. `createSink` is what - `sinks.create` takes (`[SinkClass, settings]`); the sink is created under `id`, ending and - replacing a sink the owner already holds under it. Returns the sink, or `undefined` (no - `createSink`, a closed owner, an id already in `extensionStats.values` — created or accepted - before —, or the id held as a sink by another object of the observer). +- `createExtensionStats({ id, jsonlPath? | sink? })` registers the id, optionally with a sink: at + most one of `jsonlPath` (a `JsonlFileSink` is created at that path under `id`) and `sink` (your + own `Sink`, whose `id` must be `id`); both is a compile error. Either ends and replaces a sink the + owner itself holds under `id` (never a descendant's). Returns the sink, or `undefined`: neither + given, a `sink` with another id, a closed owner, an id already in `extensionStats.values` (created + or accepted before), or the id held as a sink by another object of the observer. A refused `sink` + is ended. +- `SinkDestination` — `{ jsonlPath: string } | { sink: Sink }`, exclusive — is exported; it + is what `createClientSink` returns and what `createExtensionStats` takes besides `id`. - `acceptExtensionStats(id, data)` keeps `data` in `extensionStats.values` and writes it to the sink with the same id, if any — also one created later with `sinks.create`. It does not require `createExtensionStats` first; an id accepted first gets a sink through `sinks.create`. @@ -155,7 +160,7 @@ call.extensionStats.values; // Map` and - `CreateExtensionStatsSettings` are exported. + `CreateExtensionStatsSettings` are exported. ### Other diff --git a/README.md b/README.md index aa56189..d7ed6ca 100644 --- a/README.md +++ b/README.md @@ -404,7 +404,7 @@ See [Mediasoup router observation](#mediasoup-router-observation) for the full d | Event | Extra | Fires when | |-------|-------|-----------| | `client-added` | — | a client is created | -| `client-sink-created` | `{ sink: JsonlFileSink }` | the client's JSONL sample file was created (only when `createClientJsonlSink` returns a path); fires right after `client-added` | +| `client-sink-created` | `{ sink: Sink }` | the client's sample sink was created by `createClientSink`; fires right after `client-added` | | `client-updated` | `{ sample: ClientSample, elapsedTimeInMs: number, context?: AcceptContext }` | the client processed a sample | | `client-closed` | — | the client closed | | `client-joined` | — | first `CLIENT_JOINED` event seen | @@ -480,7 +480,7 @@ type ObserverConfig> = { createCallAppData?: (p: { callId: string; observer: Observer; acceptCtx?: AcceptContext }) => Record; createClientAppData?: (p: { clientId: string; observedCall: ObservedCall; acceptCtx?: AcceptContext }) => Record; // a JSONL file per client: return its path, or undefined for none (see Sinks). - createClientJsonlSink?: (p: { clientId: string; observedCall: ObservedCall }) => string | undefined; + createClientSink?: (p: { id: string; clientId: string; observedCall: ObservedCall }) => SinkDestination | undefined; // remote-track-resolver factory — produces a call's RemoteTrackResolver (see Remote track resolution). createRemoteTrackResolver?: (observedCall: ObservedCall) => RemoteTrackResolver | undefined; }; @@ -1929,6 +1929,9 @@ that are still open, since the observer does not close mediasoup routers. | Method | Does | |--------|------| | `create(SinkClass, settings)` | constructs `new SinkClass(settings)` and registers it under `settings.id` (generated when omitted); `undefined` if the id is taken | +| `add(sink): boolean` | registers a sink constructed elsewhere under its own `id`; `false` if the id is taken (the sink stays yours to end). Prefer `create` for sinks that open a resource on construction: it checks the id first | +| `createFromDestination(id, destination)` | registers the sink a `SinkDestination` describes under `id`: `{ jsonlPath }` creates a `JsonlFileSink` (the id is checked first), `{ sink }` adds your sink (its `id` must be `id`). `undefined` if refused; a refused `sink` is ended | +| `owns(id): boolean` | whether `id` was added to this registry itself, not to a descendant | | `get(id)` | the sink, if any (own or a descendant's) | | `write(id, record): boolean` | writes to the sink; `false` if there is none, or on backpressure | | `clear()` | ends every sink of the registry and its descendants | @@ -1974,21 +1977,31 @@ its id — refuse writes after it, and emit `close` / `error` for its destinatio ### Client samples A client writes every accepted sample to the sink registered under its **client id** in its call's -registry (`observedClient.sink` returns it), and removes — ends — that sink when it closes. For the -common case, a JSONL file per client, return a path from `createClientJsonlSink`: +registry (`observedClient.sink` returns it), and ends that sink when it closes. `createClientSink` +decides it, per client, by returning a **`SinkDestination`** — exactly one of `jsonlPath` and `sink`: + +| Returns | Sink | +|---------|------| +| `{ jsonlPath }` | a JSONL file at that path: a `JsonlFileSink` created by the library under the client id | +| `{ sink }` | your `Sink`; its `id` must be `params.id`, the client id | +| `undefined` | none | ```ts const observer = new Observer({ - // return undefined for no file - createClientJsonlSink: ({ observedCall, clientId }) => `./stats/${observedCall.callId}__${clientId}.jsonl`, + createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: `./stats/${observedCall.callId}__${clientId}.jsonl` }), }); observer.on('client-sink-created', ({ observedClient, sink }) => { - sink.once('close', () => uploadFile(sink.path)); // flushed & fd closed → ready + if (sink instanceof JsonlFileSink) sink.once('close', () => uploadFile(sink.path)); // flushed & fd closed → ready }); ``` -For any other destination, create your own sink under the client id when the client is created: +`createClientSink` is called only when the client id is free as a sink id, so a clash opens no file. +A returned sink with a different id is ended and not used. Giving both `jsonlPath` and `sink` is a +compile error (and refused at runtime). For a JSONL file with a custom serializer, return +`{ sink: new JsonlFileSink({ id, path, serializeSample }) }`. + +Any other destination works the same way — return your own implementation: ```ts class HttpSink extends EventEmitter implements ClientSampleSink { @@ -2017,9 +2030,12 @@ class HttpSink extends EventEmitter implements ClientSampleSink { } } -observer.on('client-added', ({ observedCall, observedClient }) => { - observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url: `https://stats.example.com/${observedClient.clientId}` }); +new Observer({ + createClientSink: ({ id, clientId }) => ({ sink: new HttpSink({ id, url: `https://stats.example.com/${clientId}` }) }), }); + +// Or later, on `client-added`, if the client has no sink yet: +// observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url }) ``` Client ids double as sink ids, so they must be unique across the observer's open calls (client @@ -2037,8 +2053,11 @@ the call) or `observedMediasoupRouter` (a pipe transport's `getStats()`): the la optionally a sink that receives every data. The API is the same on all three; a call is shown. ```ts -// Register the id; with `createSink` (what `sinks.create` takes), a sink is created under the same id. -call.createExtensionStats({ id: `${call.callId}:sfu`, createSink: [ JsonlFileSink, { path: './stats/sfu.jsonl' } ] }); +// Register the id, with a JSONL file that receives every data accepted for it. +call.createExtensionStats({ id: `${call.callId}:sfu`, jsonlPath: './stats/sfu.jsonl' }); + +// Or with a sink of your own; its id must be the extension stats id. +call.createExtensionStats({ id: `${call.callId}:metrics`, sink: new HttpSink({ id: `${call.callId}:metrics`, url }) }); // Your own polling: keep the last data, and write it to the sink with the same id, if any. call.acceptExtensionStats(`${call.callId}:sfu`, await collectSfuStats(call.callId)); @@ -2049,7 +2068,7 @@ call.extensionStats.values; // Map | Method | Behaviour | |--------|-----------| -| `createExtensionStats({ id, createSink? })` | Registers `id`. With `createSink` — what `sinks.create` takes, `[SinkClass, settings]` — it also creates a sink under `id` (an `id` in the sink settings is ignored), ending and replacing a sink the owner already holds under `id`. Returns the created sink, or `undefined`: without `createSink`, on a closed owner, when `id` is already in `extensionStats.values` (created or accepted before), or when another object of the observer holds a sink with this id | +| `createExtensionStats({ id, jsonlPath? \| sink? })` | Registers `id`, optionally with a sink — at most one of: `jsonlPath`, the path of a JSONL file, for which a `JsonlFileSink` is created under `id`; or `sink`, a `Sink` of your own, whose `id` must be `id`. Either ends and replaces a sink the owner itself already holds under `id`. Returns the sink, or `undefined`: with neither, on a closed owner, when `id` is already in `extensionStats.values` (created or accepted before), with a `sink` whose id differs, or when another object of the observer holds a sink with this id. Giving both is a compile error (refused at runtime). A refused `sink` is ended; for `jsonlPath`, the id is checked before the file is opened | | `acceptExtensionStats(id, data)` | Keeps `data` as the last data of `id` and writes it to the owner's sink with the same id, if there is one (a sink that throws is logged). `id` does not have to be created first. Dropped once the owner is closed | | `getExtensionStats(id)` | The last data accepted for `id`, or `undefined` | | `extensionStats` | The owner's `ExtensionStatsRegistry`: `values` (`Map`), `closed`, `close()`, and `create` / `accept` / `get`, which the three methods above delegate to | @@ -2057,12 +2076,12 @@ call.extensionStats.values; // Map **Extension stats ids share the sink id namespace, so they must be unique per observer**: the sink of extension stats `x` is the sink with id `x`. Consequently: -- With `createSink`, a sink the owner already holds under the id is ended and replaced. If another - object of the observer holds it, no sink is created and the data is only kept. -- Without `createSink`, a sink you create later under the id (`sinks.create(SinkClass, { id, ... })`) - receives the data accepted from then on. -- An id accepted before it was created can no longer be created with `createSink`; give it a sink - with `sinks.create` instead. +- With `jsonlPath` or `sink`, a sink the owner itself already holds under the id is ended and replaced. A sink another object holds under it is never touched: no sink is created and the data + is only kept. +- With neither, a sink you add later under the id (`sinks.create(SinkClass, { id, ... })` or + `sinks.add(sink)`) receives the data accepted from then on. +- An id accepted before it was created can no longer be created with a sink; give it one with + `sinks.create` / `sinks.add` instead. **Lifecycle.** When the owner (the call, the router or the observer) closes, it closes its `extensionStats`: data accepted afterwards is dropped, `createExtensionStats` returns `undefined`, diff --git a/src/ObservedCall.ts b/src/ObservedCall.ts index 899ea2f..df85aee 100644 --- a/src/ObservedCall.ts +++ b/src/ObservedCall.ts @@ -2,8 +2,7 @@ import { EventEmitter } from 'node:events'; import { SinkRegistry } from './sinks/SinkRegistry'; import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; -import type { SinkConstructor } from './sinks/SinkRegistry'; -import { JsonlFileSink } from './sinks/JsonlFileSink'; +import type { Sink } from './sinks/Sink'; import type { ClientSample } from './schema/ClientSample'; import { createLogger } from './common/logger'; import { ObservedClient, ObservedClientEvents, ObservedClientSettings } from './ObservedClient'; @@ -294,7 +293,7 @@ export class ObservedCall = Record(settings: CreateExtensionStatsSettings): InstanceType | undefined { + public createExtensionStats(settings: CreateExtensionStatsSettings): Sink | undefined { return this.extensionStats.create(settings); } @@ -386,21 +385,13 @@ export class ObservedCall = Record, { id: settings.clientId, path: jsonlPath }); - } + const clientSink = this._createClientSink(settings.clientId); this.emit('newclient', result); this._notify('client-added', { ...this.eventScope, observedClient: result }); - if (jsonlSink) { - this._notify('client-sink-created', { ...this.eventScope, observedClient: result, sink: jsonlSink }); + if (clientSink) { + this._notify('client-sink-created', { ...this.eventScope, observedClient: result, sink: clientSink }); } if (wasEmpty) { @@ -466,6 +457,23 @@ export class ObservedCall = Record | undefined { + const createClientSink = this.observer.config.createClientSink; + + if (!createClientSink) return undefined; + + // Sink ids are unique per observer: checked before the factory runs, so a clash opens no file. + if (this.observer.sinks.get(clientId)) { + logger.warn('No sample sink for client %s in call %s: sink id %s is already in use', clientId, this.callId, clientId); + + return undefined; + } + + const destination = createClientSink({ id: clientId, observedCall: this, clientId }); + + return destination ? this.sinks.createFromDestination(clientId, destination) : undefined; + } + private _notify(type: K, ...args: ObserverEvents[K]): void { this.observer.emit(type, ...args); } diff --git a/src/ObservedClient.ts b/src/ObservedClient.ts index fa9ae12..c0130c1 100644 --- a/src/ObservedClient.ts +++ b/src/ObservedClient.ts @@ -263,7 +263,7 @@ export class ObservedClient = Record | undefined { return this.call.sinks.get>(this.clientId); diff --git a/src/ObservedMediasoupRouter.ts b/src/ObservedMediasoupRouter.ts index 4904fa8..d60ee2b 100644 --- a/src/ObservedMediasoupRouter.ts +++ b/src/ObservedMediasoupRouter.ts @@ -15,7 +15,7 @@ import { createLogger } from './common/logger'; import { SinkRegistry } from './sinks/SinkRegistry'; import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; -import type { SinkConstructor } from './sinks/SinkRegistry'; +import type { Sink } from './sinks/Sink'; const logger = createLogger('ObservedMediasoupRouter'); @@ -223,7 +223,7 @@ export class ObservedMediasoupRouter = R this.emit('close'); } - public createExtensionStats(settings: CreateExtensionStatsSettings): InstanceType | undefined { + public createExtensionStats(settings: CreateExtensionStatsSettings): Sink | undefined { return this.extensionStats.create(settings); } diff --git a/src/Observer.ts b/src/Observer.ts index 56e2efa..e9912ee 100644 --- a/src/Observer.ts +++ b/src/Observer.ts @@ -10,7 +10,7 @@ import { ObservedMediasoupRouter, ObservedMediasoupRouterSettings } from './Obse import { SinkRegistry } from './sinks/SinkRegistry'; import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; -import type { SinkConstructor } from './sinks/SinkRegistry'; +import type { Sink, SinkDestination } from './sinks/Sink'; import { AvailableCallScopeDetectorsConfigs, AvailableObserverScopeDetectorsConfigs, Detectors } from './detectors/Detectors'; import type { AvailableValidatorConfigs } from './validators/Validators'; import type { ValidationReport, RunningValidator } from './validators/Validator'; @@ -231,35 +231,18 @@ export type ObserverConfig = Record path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`), - * }); - * - * // before (1.2): any other sink - * new Observer({ createClientSink: ({ clientId, observedCall }) => new HttpSink(url(observedCall, clientId)) }); - * // after: create it under the client id when the client is added (HttpSink implements - * // `ClientSampleSink`, takes `SinkSettings & { ... }` and emits `ended` from `end()`) - * observer.on('client-added', ({ observedCall, observedClient }) => { - * observedCall.sinks.create(HttpSink, { id: observedClient.clientId, url: url(observedCall, observedClient.clientId) }); - * }); + * createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: `./stats/${observedCall.callId}__${clientId}.jsonl` }), * ``` + * + * Called only when the client id is free as a sink id, so a clash opens no file. */ - createClientJsonlSink?: (params: { observedCall: ObservedCall, clientId: string }) => string | undefined, + createClientSink?: (params: { id: string, observedCall: ObservedCall, clientId: string }) => SinkDestination | undefined, /** * Optional factory invoked when a call is created, producing the call's `RemoteTrackResolver` @@ -741,7 +724,7 @@ export class Observer = Record(settings: CreateExtensionStatsSettings): InstanceType | undefined { + public createExtensionStats(settings: CreateExtensionStatsSettings): Sink | undefined { return this.extensionStats.create(settings); } diff --git a/src/ObserverEvents.ts b/src/ObserverEvents.ts index 18ca6d6..2c1c688 100644 --- a/src/ObserverEvents.ts +++ b/src/ObserverEvents.ts @@ -23,7 +23,7 @@ import type { CallSummary } from './summaries/CallSummary'; import type { ValidationReport } from './validators/Validator'; import type { ResolvedActiveClientIssue } from './issues/ActiveClientIssue'; // ClientIssue doubles as the generic issue shape ({ type, payload?, timestamp? }) for call-level issues too. -import type { JsonlFileSink } from './sinks/JsonlFileSink'; +import type { Sink } from './sinks/Sink'; import { ObservedMediasoupRouter } from './ObservedMediasoupRouter'; /** @@ -99,7 +99,7 @@ export type ObserverEvents = { // client level 'client-added': [ObservedClientScope]; - 'client-sink-created': [ObservedClientScope & { sink: JsonlFileSink }]; + 'client-sink-created': [ObservedClientScope & { sink: Sink }]; 'client-updated': [ObservedClientScope & { sample: ClientSample, elapsedTimeInMs: number }]; 'client-closed': [ObservedClientScope]; 'client-joined': [ObservedClientScope]; diff --git a/src/index.ts b/src/index.ts index 9cdec25..6c4c89a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -309,6 +309,7 @@ export type { Sink, SinkEvents, SinkSettings, + SinkDestination, SinkConstructor, SinkCreateSettings, CreateExtensionStatsSettings, diff --git a/src/sinks/ClientSampleSink.ts b/src/sinks/ClientSampleSink.ts index cd31c7b..7fff4a4 100644 --- a/src/sinks/ClientSampleSink.ts +++ b/src/sinks/ClientSampleSink.ts @@ -5,9 +5,9 @@ import type { ClientSample } from '../schema/ClientSample'; export type ClientSampleSinkEvents = SinkEvents; /** - * A per-client destination for accepted samples. Create one under the client id with - * `observedCall.sinks.create(MySink, { id: clientId, ... })`, or let - * `ObserverConfig.createClientJsonlSink` create a `JsonlFileSink` there. + * A per-client destination for accepted samples. Return one from `ObserverConfig.createClientSink` + * (with the `id` it receives), or create one under the client id with + * `observedCall.sinks.create(MySink, { id: clientId, ... })`. * * The library calls `write(sample)` synchronously per accepted sample (not awaited) and `end()`s * the sink when the client closes; the registry attaches an `error` listener so a failing sink diff --git a/src/sinks/ExtensionStatsRegistry.ts b/src/sinks/ExtensionStatsRegistry.ts index 1166ff1..0c36c19 100644 --- a/src/sinks/ExtensionStatsRegistry.ts +++ b/src/sinks/ExtensionStatsRegistry.ts @@ -1,17 +1,18 @@ -import type { SinkConstructor, SinkCreateSettings, SinkRegistry } from './SinkRegistry'; +import type { SinkRegistry } from './SinkRegistry'; +import type { Sink, SinkDestination } from './Sink'; import { createLogger } from '../common/logger'; const logger = createLogger('ExtensionStats'); -/** The settings of `createExtensionStats` on an observer, a call or a mediasoup router. */ -export type CreateExtensionStatsSettings = { +/** + * The settings of `createExtensionStats` on an observer, a call or a mediasoup router: the `id`, + * and at most one of `jsonlPath` and `sink` (a `SinkDestination`). + */ +export type CreateExtensionStatsSettings = { /** Unique per observer: it is also the id of the sink that receives the stats. */ id: string, - - /** What `sinks.create` takes; the sink is created under `id` (an `id` in its settings is ignored). */ - createSink?: [SinkClass: C, settings: Omit, 'id'>], -}; +} & (SinkDestination | { jsonlPath?: never, sink?: never }); /** * The extension stats of one object — `observer.extensionStats`, `observedCall.extensionStats`, @@ -45,27 +46,29 @@ export class ExtensionStatsRegistry { } } - public create(settings: CreateExtensionStatsSettings): InstanceType | undefined { - if (this.closed) { - logger.warn('Attempted to create extension stats %s on closed %s', settings.id, this.id); - - return undefined; - } - if (this.values.has(settings.id)) { - logger.warn('Extension stats with id %s already exists on %s', settings.id, this.id); + public create(settings: CreateExtensionStatsSettings): Sink | undefined { + const { id, jsonlPath, sink } = settings as { id: string, jsonlPath?: string, sink?: Sink }; + // A given sink is the owner's from here: refused, it is ended rather than left open. + const refuse = (reason: string) => { + logger.warn('Extension stats %s on %s: %s', id, this.id, reason); + sink?.end(); return undefined; - } + }; - this.values.set(settings.id, undefined); + if (this.closed) return refuse('the owner is closed'); + if (this.values.has(id)) return refuse('the id already exists'); + if (jsonlPath !== undefined && sink) return refuse('give either jsonlPath or sink, not both'); + if (sink && sink.id !== id) return refuse(`the sink id ${sink.id} differs from the extension stats id`); - if (!settings.createSink) return undefined; + this.values.set(id, undefined); - const [ SinkClass, sinkSettings ] = settings.createSink; + if (jsonlPath === undefined && !sink) return undefined; - this._sinks.get(settings.id)?.end(); + // Replace a sink this owner holds under the id; never one of a descendant's. + if (this._sinks.owns(id)) this._sinks.get(id)?.end(); - return this._sinks.create(SinkClass, { ...sinkSettings, id: settings.id } as SinkCreateSettings); + return this._sinks.createFromDestination(id, settings as SinkDestination); } public accept(id: string, data: unknown): void { diff --git a/src/sinks/Sink.ts b/src/sinks/Sink.ts index 0c3b34a..5526b1b 100644 --- a/src/sinks/Sink.ts +++ b/src/sinks/Sink.ts @@ -60,3 +60,11 @@ export interface Sink { off(event: keyof SinkEvents, listener: (...args: any[]) => void): this; /* eslint-enable @typescript-eslint/no-explicit-any */ } + +/** + * Where records go, exactly one of: `jsonlPath`, the path of a JSONL file (a `JsonlFileSink` is + * created there), or `sink`, a sink of your own. Giving both is a compile error. + */ +export type SinkDestination = + | { jsonlPath: string, sink?: never } + | { sink: Sink, jsonlPath?: never }; diff --git a/src/sinks/SinkRegistry.ts b/src/sinks/SinkRegistry.ts index 4f7fe65..f679421 100644 --- a/src/sinks/SinkRegistry.ts +++ b/src/sinks/SinkRegistry.ts @@ -1,4 +1,5 @@ -import type { Sink } from './Sink'; +import type { Sink, SinkDestination } from './Sink'; +import { JsonlFileSink } from './JsonlFileSink'; import { v4 as uuidv4 } from 'uuid'; import { createLogger } from '../common/logger'; @@ -28,6 +29,9 @@ export class SinkRegistry { // eslint-disable-next-line @typescript-eslint/no-explicit-any private readonly _sinks = new Map>(); + // The ids of the sinks added to this registry itself (not to a descendant). + private readonly _ownIds = new Set(); + public constructor( public readonly parent?: SinkRegistry, ) { @@ -68,21 +72,61 @@ export class SinkRegistry { } const sink = new SinkClass({ ...settings, id }) as InstanceType; - // An EventEmitter 'error' with no listener throws; a failing destination must not crash the process. - const onError = (err: Error) => logger.warn('Sink %s failed: %o', id, err); - sink.on('error', onError); - sink.once('close', () => sink.off('error', onError)); - sink.once('ended', () => { - for (const registry of lineage) { - if (registry._sinks.get(id) === sink) registry._sinks.delete(id); - } - }); - for (const registry of lineage) registry._sinks.set(id, sink); + this._register(sink, lineage); + + return sink; + } + + /** + * Register a sink constructed elsewhere, under its own `id`. Returns `false`, registering nothing, + * when the id is already in use in the observer (the sink is then the caller's to end). Prefer + * `create` when the sink opens a resource on construction: it checks the id first. + */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public add(sink: Sink): boolean { + const lineage = this._lineage(); + + if (lineage[lineage.length - 1]._sinks.has(sink.id)) { + logger.warn('Sink id %s is already in use; sink ids must be unique per observer. The sink was not added', sink.id); + + return false; + } + + this._register(sink, lineage); + + return true; + } + + /** + * Register the sink a `SinkDestination` describes, under `id`: for `jsonlPath`, a `JsonlFileSink` + * is created (the id is checked before the file is opened); a `sink` is added, and its `id` must + * be `id`. Returns `undefined` when the id is taken, the ids differ, or both are given (possible + * from JavaScript); a given `sink` that is not registered is ended. + */ + public createFromDestination(id: string, destination: SinkDestination): Sink | undefined { + const { jsonlPath, sink } = destination as { jsonlPath?: string, sink?: Sink }; + const refuse = (reason: string, ...args: unknown[]) => { + logger.warn(`Sink ${id}: ${reason}`, ...args); + sink?.end(); + + return undefined; + }; + + if (jsonlPath !== undefined && sink) return refuse('give either jsonlPath or sink, not both'); + if (jsonlPath !== undefined) return this.create(JsonlFileSink, { id, path: jsonlPath }); + if (!sink) return undefined; + if (sink.id !== id) return refuse('the sink id %s must be %s; it is ended', sink.id, id); + if (!this.add(sink)) return refuse('the id is already in use; the sink is ended'); return sink; } + /** Whether the sink `id` was added to this registry itself, rather than to a descendant. */ + public owns(id: string): boolean { + return this._ownIds.has(id); + } + /** Write `record` to the sink `id`. `false` when there is no such sink, or on backpressure. */ public write(id: string, record: unknown): boolean { const sink = this._sinks.get(id); @@ -109,6 +153,24 @@ export class SinkRegistry { } /** This registry and its ancestors, up to the root. */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private _register(sink: Sink, lineage: SinkRegistry[]): void { + const id = sink.id; + // An EventEmitter 'error' with no listener throws; a failing destination must not crash the process. + const onError = (err: Error) => logger.warn('Sink %s failed: %o', id, err); + + sink.on('error', onError); + sink.once('close', () => sink.off('error', onError)); + sink.once('ended', () => { + if (this._sinks.get(id) === sink) this._ownIds.delete(id); + for (const registry of lineage) { + if (registry._sinks.get(id) === sink) registry._sinks.delete(id); + } + }); + this._ownIds.add(id); + for (const registry of lineage) registry._sinks.set(id, sink); + } + private _lineage(): SinkRegistry[] { return [ this, ...(this.parent?._lineage() ?? []) ]; } diff --git a/src/sinks/index.ts b/src/sinks/index.ts index f69bd34..5abe4eb 100644 --- a/src/sinks/index.ts +++ b/src/sinks/index.ts @@ -1,5 +1,5 @@ // Internal barrel for the sinks, re-exported from the package root. -export type { Sink, SinkEvents, SinkSettings } from './Sink'; +export type { Sink, SinkEvents, SinkSettings, SinkDestination } from './Sink'; export { SinkRegistry } from './SinkRegistry'; export type { SinkConstructor, SinkCreateSettings } from './SinkRegistry'; export type { ClientSampleSink, ClientSampleSinkEvents } from './ClientSampleSink'; diff --git a/tests/extensionStats.spec.ts b/tests/extensionStats.spec.ts index d65e2ec..4d2060b 100644 --- a/tests/extensionStats.spec.ts +++ b/tests/extensionStats.spec.ts @@ -49,7 +49,7 @@ describe('ObservedCall extension stats', () => { it('writes every accepted data to the sink created with it, which ends with the call', async () => { const { observer, call } = setup(); const file = path.join(dir, 'sfu.jsonl'); - const sink = call.createExtensionStats({ id: 'sfu', createSink: [ JsonlFileSink, { path: file } ] })!; + const sink = call.createExtensionStats({ id: 'sfu', jsonlPath: file })! as JsonlFileSink; expect(sink).toBeInstanceOf(JsonlFileSink); expect(sink.id).toBe('sfu'); @@ -71,7 +71,7 @@ describe('ObservedCall extension stats', () => { it('replaces a sink the call already holds under the id when created with createSink', () => { const { observer, call } = setup(); const existing = call.sinks.create(InMemorySink, { id: 'sfu' })!; - const created = call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })!; + const created = call.createExtensionStats({ id: 'sfu', sink: new InMemorySink({ id: 'sfu' }) })! as InMemorySink; expect(existing.ended).toBe(true); expect(call.sinks.get('sfu')).toBe(created); @@ -100,7 +100,7 @@ describe('ObservedCall extension stats', () => { const other = observer.createObservedCall({ callId: 'call-2' })!; const held = other.sinks.create(InMemorySink, { id: 'sfu' })!; - expect(call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(call.createExtensionStats({ id: 'sfu', sink: new InMemorySink({ id: 'sfu' }) })).toBeUndefined(); expect(held.ended).toBe(false); // The data is still kept on the call; it just has no sink here. @@ -109,7 +109,7 @@ describe('ObservedCall extension stats', () => { expect(records(held)).toEqual([]); call.close(); - expect(call.createExtensionStats({ id: 'late', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(call.createExtensionStats({ id: 'late', sink: new InMemorySink({ id: 'late' }) })).toBeUndefined(); call.acceptExtensionStats('sfu', { bitrate: 2 }); expect(call.getExtensionStats('sfu')).toEqual({ bitrate: 1 }); observer.close(); @@ -118,7 +118,7 @@ describe('ObservedCall extension stats', () => { it('keeps a client sample sink apart from extension stats with a different id', () => { const { observer, call } = setup(); - call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] }); + call.createExtensionStats({ id: 'sfu', sink: new InMemorySink({ id: 'sfu' }) }); observer.on('client-added', ({ observedCall, observedClient }) => { observedCall.sinks.create(InMemorySink, { id: observedClient.clientId }); }); @@ -136,11 +136,11 @@ describe('extension stats ids', () => { const { observer, call } = setup(); call.createExtensionStats({ id: 'sfu' }); - expect(call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(call.createExtensionStats({ id: 'sfu', sink: new InMemorySink({ id: 'sfu' }) })).toBeUndefined(); expect(call.sinks.get('sfu')).toBeUndefined(); call.acceptExtensionStats('accepted-first', { bitrate: 1 }); - expect(call.createExtensionStats({ id: 'accepted-first', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(call.createExtensionStats({ id: 'accepted-first', sink: new InMemorySink({ id: 'accepted-first' }) })).toBeUndefined(); // The way to give it a sink afterwards. const sink = call.sinks.create(InMemorySink, { id: 'accepted-first' })!; @@ -158,7 +158,7 @@ describe('ObservedMediasoupRouter extension stats', () => { const observer = new Observer(); const router = fakeRouter(); const observedRouter = observer.createObservedMediasoupRouter({ router })!; - const sink = observedRouter.createExtensionStats({ id: 'pipe-1', createSink: [ InMemorySink, {} ] })!; + const sink = observedRouter.createExtensionStats({ id: 'pipe-1', sink: new InMemorySink({ id: 'pipe-1' }) })! as InMemorySink; observedRouter.acceptExtensionStats('pipe-1', [ { bytesSent: 1 } ]); observedRouter.acceptExtensionStats('pipe-1', [ { bytesSent: 2 } ]); @@ -170,7 +170,7 @@ describe('ObservedMediasoupRouter extension stats', () => { // Unique per observer: the call cannot take the router's id. const call = observer.createObservedCall({ callId: 'call-1' })!; - expect(call.createExtensionStats({ id: 'pipe-1', createSink: [ InMemorySink, {} ] })).toBeUndefined(); + expect(call.createExtensionStats({ id: 'pipe-1', sink: new InMemorySink({ id: 'pipe-1' }) })).toBeUndefined(); expect(sink.ended).toBe(false); router.observer.emit('close'); @@ -185,7 +185,7 @@ describe('ObservedMediasoupRouter extension stats', () => { describe('Observer extension stats', () => { it('keeps the last data, writes it to its sink, and ends the sink when the observer closes', () => { const observer = new Observer(); - const sink = observer.createExtensionStats({ id: 'worker-1', createSink: [ InMemorySink, {} ] })!; + const sink = observer.createExtensionStats({ id: 'worker-1', sink: new InMemorySink({ id: 'worker-1' }) })! as InMemorySink; observer.acceptExtensionStats('worker-1', { cpu: 12 }); @@ -204,9 +204,9 @@ describe('ExtensionStatsRegistry.close', () => { it('ends only the extension stats sinks of its owner', () => { const observer = new Observer({ closeCallIfEmptyForMs: undefined }); const call = observer.createObservedCall({ callId: 'call-1' })!; - const statsSink = observer.createExtensionStats({ id: 'worker-1', createSink: [ InMemorySink, {} ] })!; + const statsSink = observer.createExtensionStats({ id: 'worker-1', sink: new InMemorySink({ id: 'worker-1' }) })! as InMemorySink; const otherSink = observer.sinks.create(InMemorySink, { id: 'other' })!; - const callSink = call.createExtensionStats({ id: 'sfu', createSink: [ InMemorySink, {} ] })!; + const callSink = call.createExtensionStats({ id: 'sfu', sink: new InMemorySink({ id: 'sfu' }) })! as InMemorySink; observer.extensionStats.close(); @@ -217,3 +217,49 @@ describe('ExtensionStatsRegistry.close', () => { expect([ otherSink.ended, callSink.ended ]).toEqual([ true, true ]); }); }); + +describe('createExtensionStats settings', () => { + it('refuses a sink whose id differs, ending it, and opens no file for a path whose id is taken', () => { + const { observer, call } = setup(); + const mismatched = new InMemorySink({ id: 'not-b' }); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'extension-stats-')); + const other = observer.createObservedCall({ callId: 'call-2' })!; + + other.sinks.create(InMemorySink, { id: 'taken' }); + expect(call.createExtensionStats({ id: 'taken', jsonlPath: path.join(dir, 'taken.jsonl') })).toBeUndefined(); + expect(fs.existsSync(path.join(dir, 'taken.jsonl'))).toBe(false); + fs.rmSync(dir, { recursive: true, force: true }); + + expect(call.createExtensionStats({ id: 'b', sink: mismatched })).toBeUndefined(); + expect(mismatched.ended).toBe(true); + expect(call.sinks.get('not-b')).toBeUndefined(); + observer.close(); + }); + + it('refuses jsonlPath and sink together (from JavaScript), ending the given sink and opening no file', () => { + const { observer, call } = setup(); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'extension-stats-')); + const given = new InMemorySink({ id: 'both' }); + // Only reachable without the type check: the settings type makes both a compile error. + const settings = { id: 'both', jsonlPath: path.join(dir, 'both.jsonl'), sink: given } as unknown as { id: string, sink: InMemorySink }; + + expect(call.createExtensionStats(settings)).toBeUndefined(); + expect(given.ended).toBe(true); + expect(fs.existsSync(path.join(dir, 'both.jsonl'))).toBe(false); + expect(call.extensionStats.values.has('both')).toBe(false); + fs.rmSync(dir, { recursive: true, force: true }); + observer.close(); + }); + + it('never ends a descendant\'s sink: the observer cannot replace a call\'s sink under the same id', () => { + const { observer, call } = setup(); + const callSink = call.sinks.create(InMemorySink, { id: 'shared' })!; + const given = new InMemorySink({ id: 'shared' }); + + expect(observer.createExtensionStats({ id: 'shared', sink: given })).toBeUndefined(); + expect(callSink.ended).toBe(false); + expect(given.ended).toBe(true); + expect(call.sinks.get('shared')).toBe(callSink); + observer.close(); + }); +}); diff --git a/tests/sinks.spec.ts b/tests/sinks.spec.ts index 9837b0d..ce05d5c 100644 --- a/tests/sinks.spec.ts +++ b/tests/sinks.spec.ts @@ -72,13 +72,13 @@ describe('JsonlFileSink', () => { expect(fs.readFileSync(filePath, 'utf8').trim()).toBe('id=zed'); }); - it('createClientJsonlSink writes each client\'s samples to the path it returns, announced by client-sink-created', async () => { + it('createClientSink returning { jsonlPath }: each client writes its samples to that JSONL file, announced by client-sink-created', async () => { const observer = new Observer({ - createClientJsonlSink: ({ observedCall, clientId }) => (clientId === 'skip' ? undefined : path.join(dir, `${observedCall.callId}__${clientId}.jsonl`)), + createClientSink: ({ observedCall, clientId }) => (clientId === 'skip' ? undefined : { jsonlPath: path.join(dir, `${observedCall.callId}__${clientId}.jsonl`) }), }); const created: JsonlFileSink[] = []; - observer.on('client-sink-created', ({ sink }) => created.push(sink)); + observer.on('client-sink-created', ({ sink }) => created.push(sink as JsonlFileSink)); observer.accept(makeSample({ callId: 'cid', clientId: 'cl' })); observer.accept(makeSample({ callId: 'cid', clientId: 'skip' })); @@ -99,9 +99,33 @@ describe('JsonlFileSink', () => { expect(JSON.parse(lines[0]).clientId).toBe('cl'); }); - it('createClientJsonlSink opens no file for a client whose id is already a sink id in another call', () => { + it('createClientSink returning { sink }: uses it when its id is the client id, ends it otherwise', () => { + const sinks: InMemorySink[] = []; const observer = new Observer({ - createClientJsonlSink: ({ observedCall, clientId }) => path.join(dir, `${observedCall.callId}__${clientId}.jsonl`), + createClientSink: ({ id, clientId }) => { + const sink = new InMemorySink({ id: clientId === 'wrong' ? 'not-the-client-id' : id }); + + sinks.push(sink); + + return { sink }; + }, + }); + + observer.accept(makeSample({ callId: 'c', clientId: 'ok' })); + observer.accept(makeSample({ callId: 'c', clientId: 'wrong' })); + + const call = observer.getObservedCall('c')!; + + expect(call.getObservedClient('ok')?.sink).toBe(sinks[0]); + expect(sinks[0].samples).toHaveLength(1); + expect(call.getObservedClient('wrong')?.sink).toBeUndefined(); + expect(sinks[1].ended).toBe(true); + observer.close(); + }); + + it('createClientSink is not called for a client whose id is already a sink id in another call', () => { + const observer = new Observer({ + createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: path.join(dir, `${observedCall.callId}__${clientId}.jsonl`) }), }); observer.accept(makeSample({ callId: 'c1', clientId: 'dup' })); From 5fbb534a8761d51033329e09c2fbc1d22b5e5cfe Mon Sep 17 00:00:00 2001 From: balazskreith Date: Fri, 16 Oct 2026 12:48:25 +0300 Subject: [PATCH 7/9] feat: refactor client sink management to use saveClientSamples for per-client sample handling --- CHANGELOG.md | 55 +++++++++++++++++++++-------------- README.md | 44 ++++++++++++++++++---------- src/ObservedCall.ts | 28 ++++++++++++++---- src/ObservedClient.ts | 2 +- src/Observer.ts | 24 ++++++++------- src/index.ts | 3 +- src/sinks/ClientSampleSink.ts | 2 +- src/sinks/Sink.ts | 3 ++ src/sinks/SinkRegistry.ts | 5 ++-- src/sinks/index.ts | 2 +- tests/sinkRegistry.spec.ts | 20 +++++++++++++ tests/sinks.spec.ts | 41 +++++++++++++++++++++----- 12 files changed, 160 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c551c1..605234c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,39 +33,45 @@ observedRouter.sinks.write(transport.id, await transport.getStats()); // or si - A client writes its samples to the sink under its client id in `observedCall.sinks` (`observedClient.sink` is now a getter for it) and ends it when the client closes. Client ids are therefore sink ids and must be unique across the observer's open calls. -- `ObserverConfig.createClientSink: ({ id, observedCall, clientId }) => SinkDestination | undefined` - decides the client's sample sink. A `SinkDestination` is exactly one of `{ jsonlPath }` — the - library creates a `JsonlFileSink` at that path under the client id — and `{ sink }` — your sink, - whose `id` must be `id` (the client id); `undefined` means none. Both is a compile error. It is called only when the id is free as a - sink id; a sink with a different id is ended and not used. `client-sink-created` announces it - (`sink: Sink`). +- `ObserverConfig.saveClientSamples` decides where each client's samples are saved — exactly one of + two per-client callbacks, called with `{ id, observedCall, clientId }` (the same `jsonlPath` / + `sink` pair as `createExtensionStats`): + - `{ jsonlPath: (p) => string | undefined }`: the library creates a `JsonlFileSink` at the + returned path under the client id; + - `{ sink: (p) => Sink | undefined }`: your sink, whose `id` must be `p.id`. + + **A client's sample sink id is its client id** (`p.id`); `jsonlPath` uses it for you. Sink ids + are unique per observer, so client ids must be unique across the observer's open calls (client + monitors generate uuids); a client whose id is already a sink id gets no sink, with a warning. + `undefined` gives that client no sink; both is a compile error. The callback is called only when + the id is free; a sink with a different id is ended and not used. + `client-sink-created` announces it (`sink: Sink`). - `SinkRegistry.createFromDestination(id, destination)` registers what a `SinkDestination` - describes (`createClientSink` and `createExtensionStats` use it). + describes (`saveClientSamples` and `createExtensionStats` use it). - `SinkRegistry.add(sink)` registers a sink constructed elsewhere under its own id, and `owns(id)` tells whether an id was added to that registry itself rather than to a descendant. - `JsonlFileSink` and `createJsonlFileSink` take any record type. **Breaking** (see *Migrating from 1.2* below): -- `ObserverConfig.createClientSink` now returns a `SinkDestination` — `{ jsonlPath }` or - `{ sink }` with the `id` it receives — instead of a sink; sinks implement the `Sink` interface - (below). +- `ObserverConfig.createClientSink` is replaced by `saveClientSamples` (`{ jsonlPath }` or + `{ sink }`, each a per-client callback); sinks implement the `Sink` interface (below). - `ClientSampleSink` is no longer a class to extend: implement `Sink` (e.g. `extends EventEmitter implements ClientSampleSink`), take `SinkSettings` in the constructor, and emit `ended` from `end()`. `JsonlFileSink` and `InMemorySink` implement the interface instead of extending a base class. `JsonlFileSinkOptions` gained `id`; `new InMemorySink(samples)` is now `new InMemorySink({ id?, samples? })`. - `createJsonlFileSinkFactory`, `JsonlFileSinkFactoryOptions` and `ClientSampleSinkFactory` are - removed: return `{ jsonlPath }` from `createClientSink`. + removed: use `saveClientSamples: { jsonlPath }`. - `client-sink-created` carries a `Sink`. - `ObservedClient.sink` is a getter (read-only, as before). #### Migrating from 1.2 -**JSONL files per client** — `createClientSink: createJsonlFileSinkFactory(...)` becomes a -`createClientSink` that returns `{ jsonlPath }`. The factory's default file name was -`${callId}__${clientId}.jsonl`; a `getFileName` option becomes the body of the callback. For -`serializeSample`, return `{ sink: new JsonlFileSink({ id, path, serializeSample }) }` instead. +**JSONL files per client** — `createClientSink: createJsonlFileSinkFactory(...)` becomes +`saveClientSamples: { jsonlPath }`, a callback returning the file path. The factory's default file +name was `${callId}__${clientId}.jsonl`; a `getFileName` option becomes the body of the callback. +For `serializeSample`, use `saveClientSamples: { sink: ({ id }) => new JsonlFileSink({ id, path, serializeSample }) }`. ```ts // 1.2 @@ -73,22 +79,26 @@ new Observer({ createClientSink: createJsonlFileSinkFactory({ directory: './stat // 1.3 new Observer({ - createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`) }), + saveClientSamples: { + jsonlPath: ({ observedCall, clientId }) => path.join('./stats', `${observedCall.callId}__${clientId}.jsonl`), + }, }); ``` -**Any other client sink** — return it wrapped as `{ sink }`, and pass the `id` it receives to the sink. +**Any other client sink** — `createClientSink` becomes `saveClientSamples: { sink }`; pass the `id` it receives (the client id) to the sink. ```ts // 1.2 new Observer({ createClientSink: ({ clientId, observedCall }) => new HttpSink(urlOf(observedCall.callId, clientId)) }); // 1.3 -new Observer({ createClientSink: ({ id, clientId, observedCall }) => ({ sink: new HttpSink({ id, url: urlOf(observedCall.callId, clientId) }) }) }); +new Observer({ + saveClientSamples: { sink: ({ id, clientId, observedCall }) => new HttpSink({ id, url: urlOf(observedCall.callId, clientId) }) }, +}); ``` **Custom sink classes** — `class HttpSink extends ClientSampleSink` becomes an implementation of -the interface. The constructor receives the settings object passed to `create`, including `id`: +the interface. The constructor receives the settings object, including `id`: ```ts // 1.2 @@ -123,8 +133,8 @@ class HttpSink extends EventEmitter implements ClientSampleSink { } ``` -**`client-sink-created`** — still fires right after `client-added` for the sink `createClientSink` -returned, typed `Sink`; narrow (`sink instanceof JsonlFileSink`) to read `sink.path`. +**`client-sink-created`** — still fires right after `client-added` for the sink created through +`saveClientSamples`, typed `Sink`; narrow (`sink instanceof JsonlFileSink`) to read `sink.path`. **Unchanged:** `observedClient.sink` still returns the client's sink, and the client still ends it when it closes. @@ -149,7 +159,8 @@ call.extensionStats.values; // Map` — `{ jsonlPath: string } | { sink: Sink }`, exclusive — is exported; it - is what `createClientSink` returns and what `createExtensionStats` takes besides `id`. + is what `createExtensionStats` takes besides `id`. `SaveClientSamplesConfig` and + `ClientSinkParams` (the callbacks' parameter) are exported too. - `acceptExtensionStats(id, data)` keeps `data` in `extensionStats.values` and writes it to the sink with the same id, if any — also one created later with `sinks.create`. It does not require `createExtensionStats` first; an id accepted first gets a sink through `sinks.create`. diff --git a/README.md b/README.md index d7ed6ca..79681dc 100644 --- a/README.md +++ b/README.md @@ -404,7 +404,7 @@ See [Mediasoup router observation](#mediasoup-router-observation) for the full d | Event | Extra | Fires when | |-------|-------|-----------| | `client-added` | — | a client is created | -| `client-sink-created` | `{ sink: Sink }` | the client's sample sink was created by `createClientSink`; fires right after `client-added` | +| `client-sink-created` | `{ sink: Sink }` | the client's sample sink was created through `saveClientSamples`; fires right after `client-added` | | `client-updated` | `{ sample: ClientSample, elapsedTimeInMs: number, context?: AcceptContext }` | the client processed a sample | | `client-closed` | — | the client closed | | `client-joined` | — | first `CLIENT_JOINED` event seen | @@ -480,7 +480,10 @@ type ObserverConfig> = { createCallAppData?: (p: { callId: string; observer: Observer; acceptCtx?: AcceptContext }) => Record; createClientAppData?: (p: { clientId: string; observedCall: ObservedCall; acceptCtx?: AcceptContext }) => Record; // a JSONL file per client: return its path, or undefined for none (see Sinks). - createClientSink?: (p: { id: string; clientId: string; observedCall: ObservedCall }) => SinkDestination | undefined; + // where each client's samples are saved: exactly one of jsonlPath / sink, each a per-client callback (see Sinks). + saveClientSamples?: + | { jsonlPath: (p: { id: string; clientId: string; observedCall: ObservedCall }) => string | undefined } + | { sink: (p: { id: string; clientId: string; observedCall: ObservedCall }) => Sink | undefined }; // remote-track-resolver factory — produces a call's RemoteTrackResolver (see Remote track resolution). createRemoteTrackResolver?: (observedCall: ObservedCall) => RemoteTrackResolver | undefined; }; @@ -1947,7 +1950,7 @@ and never crashes the process. import type { Sink, SinkSettings } from '@observertc/observer-js'; interface Sink { // typically: class X extends EventEmitter implements Sink - readonly id: string; + readonly id: string; // unique per observer write(record: T): boolean; // `false` once ended (nothing written), or on backpressure end(): void; // idempotent; MUST emit `ended` synchronously, then flush and emit `close` on / once / off(event, listener); // the events below @@ -1977,18 +1980,20 @@ its id — refuse writes after it, and emit `close` / `error` for its destinatio ### Client samples A client writes every accepted sample to the sink registered under its **client id** in its call's -registry (`observedClient.sink` returns it), and ends that sink when it closes. `createClientSink` -decides it, per client, by returning a **`SinkDestination`** — exactly one of `jsonlPath` and `sink`: +registry (`observedClient.sink` returns it), and ends that sink when it closes. The +**`saveClientSamples`** config option decides it — exactly one of two per-client callbacks, the same +`jsonlPath` / `sink` pair as `createExtensionStats` takes: -| Returns | Sink | -|---------|------| -| `{ jsonlPath }` | a JSONL file at that path: a `JsonlFileSink` created by the library under the client id | -| `{ sink }` | your `Sink`; its `id` must be `params.id`, the client id | -| `undefined` | none | +| `saveClientSamples` | Called per client with `{ id, observedCall, clientId }`, returns | Sink | +|---------------------|------------------------------------------------------------------|------| +| `{ jsonlPath }` | the path of the client's file, or `undefined` | a `JsonlFileSink` at that path, created by the library under the client id | +| `{ sink }` | your `Sink`, or `undefined` | that sink; its `id` must be `id` | ```ts const observer = new Observer({ - createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: `./stats/${observedCall.callId}__${clientId}.jsonl` }), + saveClientSamples: { + jsonlPath: ({ observedCall, clientId }) => `./stats/${observedCall.callId}__${clientId}.jsonl`, + }, }); observer.on('client-sink-created', ({ observedClient, sink }) => { @@ -1996,10 +2001,15 @@ observer.on('client-sink-created', ({ observedClient, sink }) => { }); ``` -`createClientSink` is called only when the client id is free as a sink id, so a clash opens no file. -A returned sink with a different id is ended and not used. Giving both `jsonlPath` and `sink` is a -compile error (and refused at runtime). For a JSONL file with a custom serializer, return -`{ sink: new JsonlFileSink({ id, path, serializeSample }) }`. +**A client's sample sink id is its client id.** The `id` the callbacks receive is that id, and +with `jsonlPath` the library uses it for you. Sink ids are unique per observer, so **client ids +must be unique across the observer's open calls** — client monitors generate uuids, so this +normally holds. A client whose id is already a sink id gets no sink, and a warning is logged. + +The callback is called only when the id is free, so a clash opens no file; `undefined` gives that +client no sink. A returned sink with a different id is ended and not used. Giving both `jsonlPath` +and `sink` is a compile error (and refused at runtime). For a JSONL file with a custom serializer, +use `sink: ({ id }) => new JsonlFileSink({ id, path, serializeSample })`. Any other destination works the same way — return your own implementation: @@ -2031,7 +2041,9 @@ class HttpSink extends EventEmitter implements ClientSampleSink { } new Observer({ - createClientSink: ({ id, clientId }) => ({ sink: new HttpSink({ id, url: `https://stats.example.com/${clientId}` }) }), + saveClientSamples: { + sink: ({ id, clientId }) => new HttpSink({ id, url: `https://stats.example.com/${clientId}` }), // id: the client id + }, }); // Or later, on `client-added`, if the client has no sink yet: diff --git a/src/ObservedCall.ts b/src/ObservedCall.ts index df85aee..f8bdfec 100644 --- a/src/ObservedCall.ts +++ b/src/ObservedCall.ts @@ -2,7 +2,7 @@ import { EventEmitter } from 'node:events'; import { SinkRegistry } from './sinks/SinkRegistry'; import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; -import type { Sink } from './sinks/Sink'; +import type { Sink, ClientSinkParams } from './sinks/Sink'; import type { ClientSample } from './schema/ClientSample'; import { createLogger } from './common/logger'; import { ObservedClient, ObservedClientEvents, ObservedClientSettings } from './ObservedClient'; @@ -456,11 +456,18 @@ export class ObservedCall = Record | undefined { - const createClientSink = this.observer.config.createClientSink; + const config = this.observer.config.saveClientSamples as { + jsonlPath?: (params: ClientSinkParams) => string | undefined, + sink?: (params: ClientSinkParams) => Sink | undefined, + } | undefined; - if (!createClientSink) return undefined; + if (!config || (!config.jsonlPath && !config.sink)) return undefined; + if (config.jsonlPath && config.sink) { + logger.warn('saveClientSamples: give either jsonlPath or sink, not both; no client samples are saved'); + + return undefined; + } // Sink ids are unique per observer: checked before the factory runs, so a clash opens no file. if (this.observer.sinks.get(clientId)) { @@ -469,11 +476,20 @@ export class ObservedCall = Record = { id: clientId, observedCall: this, clientId }; + + if (config.jsonlPath) { + const jsonlPath = config.jsonlPath(params); - return destination ? this.sinks.createFromDestination(clientId, destination) : undefined; + return jsonlPath === undefined ? undefined : this.sinks.createFromDestination(clientId, { jsonlPath }); + } + + const sink = config.sink?.(params); + + return sink ? this.sinks.createFromDestination(clientId, { sink }) : undefined; } + /** Emit an Observer-bus event scoped to this call. */ private _notify(type: K, ...args: ObserverEvents[K]): void { this.observer.emit(type, ...args); } diff --git a/src/ObservedClient.ts b/src/ObservedClient.ts index c0130c1..1114989 100644 --- a/src/ObservedClient.ts +++ b/src/ObservedClient.ts @@ -263,7 +263,7 @@ export class ObservedClient = Record | undefined { return this.call.sinks.get>(this.clientId); diff --git a/src/Observer.ts b/src/Observer.ts index e9912ee..6a5c434 100644 --- a/src/Observer.ts +++ b/src/Observer.ts @@ -10,7 +10,7 @@ import { ObservedMediasoupRouter, ObservedMediasoupRouterSettings } from './Obse import { SinkRegistry } from './sinks/SinkRegistry'; import { ExtensionStatsRegistry } from './sinks/ExtensionStatsRegistry'; import type { CreateExtensionStatsSettings } from './sinks/ExtensionStatsRegistry'; -import type { Sink, SinkDestination } from './sinks/Sink'; +import type { Sink, ClientSinkParams } from './sinks/Sink'; import { AvailableCallScopeDetectorsConfigs, AvailableObserverScopeDetectorsConfigs, Detectors } from './detectors/Detectors'; import type { AvailableValidatorConfigs } from './validators/Validators'; import type { ValidationReport, RunningValidator } from './validators/Validator'; @@ -61,6 +61,11 @@ export type CallAppDataFactory = (params: { callId: string, observer: Observer, /** Produces the initial `appData` for a client created without an explicit `appData`. */ export type ClientAppDataFactory = (params: { clientId: string, observedCall: ObservedCall, acceptCtx?: AcceptContext }) => Record; +/** `ObserverConfig.saveClientSamples`: exactly one of the two per-client callbacks. */ +export type SaveClientSamplesConfig = + | { jsonlPath: (params: ClientSinkParams) => string | undefined, sink?: never } + | { sink: (params: ClientSinkParams) => Sink | undefined, jsonlPath?: never }; + export type ObserverConfig = Record> = { /** @@ -231,18 +236,15 @@ export type ObserverConfig = Record ({ jsonlPath: `./stats/${observedCall.callId}__${clientId}.jsonl` }), - * ``` + * Where each client's samples are saved: exactly one of `jsonlPath` (return the path of the + * client's JSONL file) and `sink` (return a sink whose `id` is `params.id`), each called per + * client; `undefined` gives that client no sink. See the README's *Sinks* section. * - * Called only when the client id is free as a sink id, so a clash opens no file. + * A client's sample sink id is its client id (`params.id`). Sink ids are unique per observer, so + * client ids must be unique across the observer's open calls; a client whose id is already a sink + * id gets no sink, and a warning is logged. */ - createClientSink?: (params: { id: string, observedCall: ObservedCall, clientId: string }) => SinkDestination | undefined, + saveClientSamples?: SaveClientSamplesConfig, /** * Optional factory invoked when a call is created, producing the call's `RemoteTrackResolver` diff --git a/src/index.ts b/src/index.ts index 6c4c89a..e423843 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -export type { ObserverEvents, SampleRejectedReason, AcceptContext, CallAppDataFactory, ClientAppDataFactory, AcceptMiddleware, AcceptMiddlewarePayload } from './Observer'; +export type { ObserverEvents, SampleRejectedReason, AcceptContext, CallAppDataFactory, ClientAppDataFactory, AcceptMiddleware, AcceptMiddlewarePayload, SaveClientSamplesConfig } from './Observer'; export type { ObserverEventBase, ObservedCallScope, @@ -310,6 +310,7 @@ export type { SinkEvents, SinkSettings, SinkDestination, + ClientSinkParams, SinkConstructor, SinkCreateSettings, CreateExtensionStatsSettings, diff --git a/src/sinks/ClientSampleSink.ts b/src/sinks/ClientSampleSink.ts index 7fff4a4..8bc209b 100644 --- a/src/sinks/ClientSampleSink.ts +++ b/src/sinks/ClientSampleSink.ts @@ -5,7 +5,7 @@ import type { ClientSample } from '../schema/ClientSample'; export type ClientSampleSinkEvents = SinkEvents; /** - * A per-client destination for accepted samples. Return one from `ObserverConfig.createClientSink` + * A per-client destination for accepted samples. Return one from `ObserverConfig.saveClientSamples.sink` * (with the `id` it receives), or create one under the client id with * `observedCall.sinks.create(MySink, { id: clientId, ... })`. * diff --git a/src/sinks/Sink.ts b/src/sinks/Sink.ts index 5526b1b..c3a5047 100644 --- a/src/sinks/Sink.ts +++ b/src/sinks/Sink.ts @@ -68,3 +68,6 @@ export interface Sink { export type SinkDestination = | { jsonlPath: string, sink?: never } | { sink: Sink, jsonlPath?: never }; + +/** What the `saveClientSamples` callbacks receive: `id` is the client's sample sink id, the client id. */ +export type ClientSinkParams = { id: string, observedCall: Call, clientId: string }; diff --git a/src/sinks/SinkRegistry.ts b/src/sinks/SinkRegistry.ts index f679421..ee54f10 100644 --- a/src/sinks/SinkRegistry.ts +++ b/src/sinks/SinkRegistry.ts @@ -143,11 +143,11 @@ export class SinkRegistry { /** End every sink of this registry and of its descendants. */ public clear(): void { // Snapshot first: each `end()` removes its sink from the map. - for (const sink of [ ...this._sinks.values() ]) { + for (const [ id, sink ] of [ ...this._sinks.entries() ]) { try { sink.end(); } catch (err) { - logger.warn('Sink %s failed to end: %o', sink.id, err); + logger.warn('Sink %s failed to end: %o', id, err); } } } @@ -156,6 +156,7 @@ export class SinkRegistry { // eslint-disable-next-line @typescript-eslint/no-explicit-any private _register(sink: Sink, lineage: SinkRegistry[]): void { const id = sink.id; + // An EventEmitter 'error' with no listener throws; a failing destination must not crash the process. const onError = (err: Error) => logger.warn('Sink %s failed: %o', id, err); diff --git a/src/sinks/index.ts b/src/sinks/index.ts index 5abe4eb..5d1b76b 100644 --- a/src/sinks/index.ts +++ b/src/sinks/index.ts @@ -1,5 +1,5 @@ // Internal barrel for the sinks, re-exported from the package root. -export type { Sink, SinkEvents, SinkSettings, SinkDestination } from './Sink'; +export type { Sink, SinkEvents, SinkSettings, SinkDestination, ClientSinkParams } from './Sink'; export { SinkRegistry } from './SinkRegistry'; export type { SinkConstructor, SinkCreateSettings } from './SinkRegistry'; export type { ClientSampleSink, ClientSampleSinkEvents } from './ClientSampleSink'; diff --git a/tests/sinkRegistry.spec.ts b/tests/sinkRegistry.spec.ts index f7e56b6..9dd3326 100644 --- a/tests/sinkRegistry.spec.ts +++ b/tests/sinkRegistry.spec.ts @@ -127,6 +127,26 @@ describe('SinkRegistry', () => { }); }); +describe('SinkRegistry.add', () => { + it('registers a sink constructed elsewhere under its own id, and refuses a taken id', () => { + const root = new SinkRegistry(); + const call = new SinkRegistry(root); + const first = new InMemorySink({ id: 'mine' }); + const second = new InMemorySink({ id: 'mine' }); + + expect(call.add(first)).toBe(true); + expect(root.get('mine')).toBe(first); + expect(root.add(second)).toBe(false); + expect(second.ended).toBe(false); + + first.end(); + expect(call.get('mine')).toBeUndefined(); + expect(root.add(second)).toBe(true); + root.clear(); + expect(second.ended).toBe(true); + }); +}); + describe('sinks on the observed objects', () => { let dir: string; diff --git a/tests/sinks.spec.ts b/tests/sinks.spec.ts index ce05d5c..ce45fdf 100644 --- a/tests/sinks.spec.ts +++ b/tests/sinks.spec.ts @@ -72,9 +72,11 @@ describe('JsonlFileSink', () => { expect(fs.readFileSync(filePath, 'utf8').trim()).toBe('id=zed'); }); - it('createClientSink returning { jsonlPath }: each client writes its samples to that JSONL file, announced by client-sink-created', async () => { + it('saveClientSamples.jsonlPath: each client writes its samples to the JSONL file at the returned path, announced by client-sink-created', async () => { const observer = new Observer({ - createClientSink: ({ observedCall, clientId }) => (clientId === 'skip' ? undefined : { jsonlPath: path.join(dir, `${observedCall.callId}__${clientId}.jsonl`) }), + saveClientSamples: { + jsonlPath: ({ observedCall, clientId }) => (clientId === 'skip' ? undefined : path.join(dir, `${observedCall.callId}__${clientId}.jsonl`)), + }, }); const created: JsonlFileSink[] = []; @@ -99,16 +101,18 @@ describe('JsonlFileSink', () => { expect(JSON.parse(lines[0]).clientId).toBe('cl'); }); - it('createClientSink returning { sink }: uses it when its id is the client id, ends it otherwise', () => { + it('saveClientSamples.sink: uses the returned sink when its id is the given id (the client id), ends it otherwise', () => { const sinks: InMemorySink[] = []; + const ids: string[] = []; const observer = new Observer({ - createClientSink: ({ id, clientId }) => { + saveClientSamples: { sink: ({ id, clientId }) => { + ids.push(id); const sink = new InMemorySink({ id: clientId === 'wrong' ? 'not-the-client-id' : id }); sinks.push(sink); - return { sink }; - }, + return sink; + } }, }); observer.accept(makeSample({ callId: 'c', clientId: 'ok' })); @@ -117,15 +121,36 @@ describe('JsonlFileSink', () => { const call = observer.getObservedCall('c')!; expect(call.getObservedClient('ok')?.sink).toBe(sinks[0]); + expect(call.sinks.get('ok')).toBe(sinks[0]); + expect(ids).toEqual([ 'ok', 'wrong' ]); expect(sinks[0].samples).toHaveLength(1); expect(call.getObservedClient('wrong')?.sink).toBeUndefined(); expect(sinks[1].ended).toBe(true); observer.close(); }); - it('createClientSink is not called for a client whose id is already a sink id in another call', () => { + it('saveClientSamples with both jsonlPath and sink (from JavaScript) saves nothing', () => { + let called = 0; + const config = { + jsonlPath: () => { ++called; return path.join(dir, 'both.jsonl'); }, + sink: ({ id }: { id: string }) => { ++called; return new InMemorySink({ id }); }, + }; + // Only reachable without the type check: the config type makes both a compile error. + const observer = new Observer({ saveClientSamples: config as unknown as { jsonlPath: () => string } }); + + observer.accept(makeSample({ callId: 'c', clientId: 'x' })); + + expect(observer.getObservedCall('c')?.getObservedClient('x')?.sink).toBeUndefined(); + expect(called).toBe(0); + expect(fs.existsSync(path.join(dir, 'both.jsonl'))).toBe(false); + observer.close(); + }); + + it('saveClientSamples is not called for a client whose id is already a sink id in another call', () => { const observer = new Observer({ - createClientSink: ({ observedCall, clientId }) => ({ jsonlPath: path.join(dir, `${observedCall.callId}__${clientId}.jsonl`) }), + saveClientSamples: { + jsonlPath: ({ observedCall, clientId }) => path.join(dir, `${observedCall.callId}__${clientId}.jsonl`), + }, }); observer.accept(makeSample({ callId: 'c1', clientId: 'dup' })); From 524b80b709e2b2a00b542bbca67e0fbbfeca0e44 Mon Sep 17 00:00:00 2001 From: balazskreith Date: Fri, 16 Oct 2026 22:58:37 +0300 Subject: [PATCH 8/9] fix: replace the outbound track when it is republished on the same MediaStreamTrack When a producer is recreated with the same MediaStreamTrack (e.g. after a reconnection, or after toggling audio processing), the new RTCRtpSender reports a new media-source stat and new outbound-rtp stats with the same trackIdentifier. The client keeps reporting the old ones too, with frozen counters. ObservedOutboundTrack is keyed by track id and captured its media source and outbound RTPs once, at creation. After a republication it therefore stayed linked to the old, frozen stats for the rest of the call: - bitrate, deltaPacketsSent, remoteFractionLost, remoteRttInMs and the degradation reasons were computed from the frozen RTPs (typically reporting no-packets-sent), and rmsAudioLevel from the frozen source; - UnconsumedTrackDetector and the P2P remote track resolver read getOutboundRtps(), i.e. the old SSRCs; - the remote track resolver kept the track indexed under its original producerId, so consumers of the new producer never linked to it. Re-linking the media source and RTPs on every update would fix the stats but silently change what an ObservedOutboundTrack stands for: its producerId / SSRC key would change under an existing pairing, and its detectors would span two publications. Pairing is bound to the producer, not to time, so a republication is treated as a new track object instead. Changes: - ObservedPeerConnection: on each outbound track update, take the newest media source with the track's trackIdentifier (the last one added; Maps keep insertion order). If it differs from the track's media source, the existing object is removed (outbound-track-removed) and a new ObservedOutboundTrack with the same id, bound to the new media source, is added (outbound-track-added). Removal goes through one helper shared with the sweep of tracks the client stopped reporting. - ObservedOutboundTrack: the media source is fixed for the object's life; getOutboundRtps() looks up the RTPs of that media source on every call, so RTPs that appear after the track (e.g. a simulcast layer starting later) are included too. - RemoteTrackResolver: store the publisher id each outbound track was indexed under and remove by it, instead of re-resolving at removal. A track whose producerId changed in place no longer leaves its old entry behind. Behaviour change: after a republication, a reference held to the old ObservedOutboundTrack points at a removed object. Look tracks up by id. The stale media source and RTPs stay in the peer connection's maps while the client reports them, but no track links to them any more. Tests (tests/outboundTrackRepublish.spec.ts): - republication yields a new object with the new media source and SSRC, computes deltaPacketsSent from the live RTP, and emits one removed and one added event; - no replacement while the media source stays the same, and a later RTP of the same source is included; - a first media source appearing after the track replaces the object; - consumers re-pair by producerId: P1 is released, P2 links to the new object; - removal releases the indexed publisher id even after the producerId changed in place (fails without the resolver change). Reported-by: Alessio Bazzica --- CHANGELOG.md | 10 ++ src/ObservedOutboundTrack.ts | 12 +- src/ObservedPeerConnection.ts | 24 ++-- src/resolvers/RemoteTrackResolver.ts | 7 +- tests/outboundTrackRepublish.spec.ts | 173 +++++++++++++++++++++++++++ 5 files changed, 213 insertions(+), 13 deletions(-) create mode 100644 tests/outboundTrackRepublish.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 605234c..e45aaa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,16 @@ call.extensionStats.values; // Map` and `CreateExtensionStatsSettings` are exported. +### Fixed: outbound track republished on the same `MediaStreamTrack` + +A producer recreated on the same track (reconnection, toggling audio processing) left the +`ObservedOutboundTrack` linked to the old media source and RTPs, so its stats, the detectors and +the remote track resolver read stale data. Now a new media source for the same track id replaces +the track object (`outbound-track-removed`, then `outbound-track-added`, same `id`), and +`getOutboundRtps()` is looked up on every call. + +Thanks to Alessio Bazzica for reporting it. + ### Other - `percentileOfHistogram` narrows the bucket holding the observed `min` / `max` to the observed diff --git a/src/ObservedOutboundTrack.ts b/src/ObservedOutboundTrack.ts index d74a303..a8225ef 100644 --- a/src/ObservedOutboundTrack.ts +++ b/src/ObservedOutboundTrack.ts @@ -84,7 +84,6 @@ export class ObservedOutboundTrack implements OutboundTrackSample { public readonly id: string, public readonly kind: MediaKind, private readonly _peerConnection: ObservedPeerConnection, - private readonly _outboundRtps?: ObservedOutboundRtp[], private readonly _mediaSource?: ObservedMediaSource, ) { // no-op @@ -110,8 +109,13 @@ export class ObservedOutboundTrack implements OutboundTrackSample { return this._peerConnection; } - public getOutboundRtps() { - return this._outboundRtps; + /** The outbound RTPs of this track's media source, looked up on every call. */ + public getOutboundRtps(): ObservedOutboundRtp[] | undefined { + const mediaSource = this._mediaSource; + + if (!mediaSource) return undefined; + + return [ ...this._peerConnection.observedOutboundRtps.values() ].filter((rtp) => rtp.mediaSourceId === mediaSource.id); } public getMediaSource() { @@ -137,7 +141,7 @@ export class ObservedOutboundTrack implements OutboundTrackSample { this.remoteRttInMs = undefined; this.qualityLimitationReason = undefined; - for (const rtp of this._outboundRtps ?? []) { + for (const rtp of this.getOutboundRtps() ?? []) { this.bitrate = (this.bitrate ?? 0) + rtp.bitrate; this.deltaPacketsSent = (this.deltaPacketsSent ?? 0) + rtp.deltaPacketsSent; diff --git a/src/ObservedPeerConnection.ts b/src/ObservedPeerConnection.ts index 4c45a0a..666ba63 100644 --- a/src/ObservedPeerConnection.ts +++ b/src/ObservedPeerConnection.ts @@ -743,9 +743,7 @@ export class ObservedPeerConnection extends EventEmitter { for (const observedOutboundTrack of [ ...this.observedOutboundTracks.values() ]) { if (observedOutboundTrack.visited) continue; - this.observedOutboundTracks.delete(observedOutboundTrack.id); - this.emit('removed-outbound-track', observedOutboundTrack); - this._notify('outbound-track-removed', { ...this.eventScope, observedOutboundTrack }); + this._removeOutboundTrack(observedOutboundTrack); } for (const observedPeerConnectionTransport of [ ...this.observedPeerConnectionTransports.values() ]) { if (observedPeerConnectionTransport.visited) continue; @@ -1078,6 +1076,14 @@ export class ObservedPeerConnection extends EventEmitter { private _updateOutboundTrackSample(stats: OutboundTrackSample) { let observedOutboundTrack = this.observedOutboundTracks.get(stats.id); + // The newest media source of this track (Maps keep insertion order). + const mediaSource = [ ...this.observedMediaSources.values() ].filter((source) => source.trackIdentifier === stats.id).pop(); + + // A new media source for the same track id means a republication: replace the track object. + if (observedOutboundTrack && observedOutboundTrack.getMediaSource() !== mediaSource) { + this._removeOutboundTrack(observedOutboundTrack); + observedOutboundTrack = undefined; + } if (!observedOutboundTrack) { if (!stats.timestamp || !stats.id || !stats.kind) { @@ -1086,17 +1092,13 @@ export class ObservedPeerConnection extends EventEmitter { stats ); } - const observedMediaSource = [ ...this.observedMediaSources.values() ].find((mediaSource) => mediaSource.trackIdentifier === stats.id); - const outboundRtps = observedMediaSource - ? [ ...this.observedOutboundRtps.values() ].filter((outboundRtp) => outboundRtp.mediaSourceId === observedMediaSource?.id) : undefined; observedOutboundTrack = new ObservedOutboundTrack( stats.timestamp, stats.id, stats.kind as MediaKind, this, - outboundRtps, - observedMediaSource, + mediaSource, ); observedOutboundTrack.update(stats); @@ -1111,6 +1113,12 @@ export class ObservedPeerConnection extends EventEmitter { return observedOutboundTrack; } + private _removeOutboundTrack(observedOutboundTrack: ObservedOutboundTrack) { + this.observedOutboundTracks.delete(observedOutboundTrack.id); + this.emit('removed-outbound-track', observedOutboundTrack); + this._notify('outbound-track-removed', { ...this.eventScope, observedOutboundTrack }); + } + private _updatePeerConnectionTransportStats(stats: PeerConnectionTransportStats) { let observedPeerConnectionTransport = this.observedPeerConnectionTransports.get(stats.id); diff --git a/src/resolvers/RemoteTrackResolver.ts b/src/resolvers/RemoteTrackResolver.ts index 7b1647e..66cbc4b 100644 --- a/src/resolvers/RemoteTrackResolver.ts +++ b/src/resolvers/RemoteTrackResolver.ts @@ -37,6 +37,8 @@ export type RemoteTrackResolvers = { */ export class RemoteTrackResolver { private readonly _publisherIdToOutboundTrack = new Map(); + // The publisher id each outbound track was indexed under, so removal does not depend on its current attachments. + private readonly _outboundTrackPublisherIds = new Map(); private readonly _subscriberIdToInboundTrack = new Map(); /** @@ -96,6 +98,7 @@ export class RemoteTrackResolver { observer.off('outbound-track-updated', onOutboundUpdated); this._pendingInboundTracks.clear(); this._pendingOutboundTracks.clear(); + this._outboundTrackPublisherIds.clear(); }); observer.on('inbound-track-added', onInboundAdded); @@ -190,6 +193,7 @@ export class RemoteTrackResolver { this._pendingOutboundTracks.delete(outboundTrack); this._publisherIdToOutboundTrack.set(publisherId, outboundTrack); + this._outboundTrackPublisherIds.set(outboundTrack, publisherId); // Unconsumed until a subscriber links below — a new publisher legitimately starts this way. this.observedCall.unconsumedOutboundTracks.add(outboundTrack); @@ -210,8 +214,9 @@ export class RemoteTrackResolver { private _removeOutboundTrack(outboundTrack: ObservedOutboundTrack) { this._pendingOutboundTracks.delete(outboundTrack); - const publisherId = this.resolvers.resolveOutboundTrackPublisherId(outboundTrack); + const publisherId = this._outboundTrackPublisherIds.get(outboundTrack); + this._outboundTrackPublisherIds.delete(outboundTrack); // Only clear the map entry if it still points at this track (guards republish races). if (publisherId && this._publisherIdToOutboundTrack.get(publisherId) === outboundTrack) { this._publisherIdToOutboundTrack.delete(publisherId); diff --git a/tests/outboundTrackRepublish.spec.ts b/tests/outboundTrackRepublish.spec.ts new file mode 100644 index 0000000..894f90a --- /dev/null +++ b/tests/outboundTrackRepublish.spec.ts @@ -0,0 +1,173 @@ +import { Observer } from '../src/Observer'; +import { createDefaultMediasoupRemoteTrackResolverFactory } from '../src/resolvers/RemoteTrackResolverFactories'; +import type { ObservedOutboundTrack } from '../src/ObservedOutboundTrack'; +import type { ClientSample, PeerConnectionSample } from '../src/schema/ClientSample'; +import { makeSample, nextTimestamp } from './helpers/samples'; + +/** + * A producer recreated with the same MediaStreamTrack (reconnection, toggling audio processing): the + * new sender reports a new media source and new outbound RTPs with the same `trackIdentifier`, while + * the client keeps reporting the old ones with frozen counters. + */ + +const TRACK_ID = 'mic-track'; + +type Publication = { mediaSourceId: string, ssrc: number, producerId: string, packetsSent: number }; + +function publisherSample(publications: Publication[], current: Publication): ClientSample { + const timestamp = nextTimestamp(); + const pc = { + peerConnectionId: 'pub-pc', + mediaSources: publications.map((p) => ({ timestamp, id: p.mediaSourceId, kind: 'audio', trackIdentifier: TRACK_ID })), + outboundRtps: publications.map((p) => ({ + timestamp, id: `rtp-${p.ssrc}`, ssrc: p.ssrc, kind: 'audio', mediaSourceId: p.mediaSourceId, + packetsSent: p.packetsSent, bytesSent: p.packetsSent * 100, + })), + outboundTracks: [ { timestamp, id: TRACK_ID, kind: 'audio', attachments: { producerId: current.producerId } } ], + } as unknown as PeerConnectionSample; + + return { timestamp, callId: 'call-1', clientId: 'publisher', peerConnections: [ pc ] }; +} + +const consumerSample = (producerId: string) => makeSample({ + clientId: 'consumer', + peerConnections: [ { peerConnectionId: 'sub-pc', inbound: [ { trackId: `in-${producerId}`, attachments: { producerId, consumerId: `c-${producerId}` } } ] } ], +}); + +function setup() { + const observer = new Observer({ createRemoteTrackResolver: createDefaultMediasoupRemoteTrackResolverFactory(), closeCallIfEmptyForMs: undefined }); + const added: ObservedOutboundTrack[] = []; + const removed: ObservedOutboundTrack[] = []; + + observer.on('outbound-track-added', ({ observedOutboundTrack }) => added.push(observedOutboundTrack)); + observer.on('outbound-track-removed', ({ observedOutboundTrack }) => removed.push(observedOutboundTrack)); + + const track = () => observer.getObservedCall('call-1')!.getObservedClient('publisher')! + .observedPeerConnections.get('pub-pc')!.observedOutboundTracks.get(TRACK_ID)!; + + return { observer, added, removed, track }; +} + +describe('outbound track republished on the same MediaStreamTrack', () => { + it('gets a new object bound to the new media source and RTPs, while the old one is removed', () => { + const { observer, added, removed, track } = setup(); + const p1: Publication = { mediaSourceId: 'ms-1', ssrc: 1, producerId: 'P1', packetsSent: 100 }; + + observer.accept(publisherSample([ p1 ], p1)); + observer.accept(publisherSample([ { ...p1, packetsSent: 200 } ], p1)); + + const first = track(); + + expect(first.getMediaSource()?.id).toBe('ms-1'); + expect(first.getOutboundRtps()?.map((rtp) => rtp.ssrc)).toEqual([ 1 ]); + + // Republished: ms-2 / ssrc 2 / P2 appear; ms-1 / ssrc 1 stay reported, frozen. + const p2: Publication = { mediaSourceId: 'ms-2', ssrc: 2, producerId: 'P2', packetsSent: 10 }; + + observer.accept(publisherSample([ { ...p1, packetsSent: 200 }, p2 ], p2)); + observer.accept(publisherSample([ { ...p1, packetsSent: 200 }, { ...p2, packetsSent: 60 } ], p2)); + + const second = track(); + + expect(second).not.toBe(first); + expect(second.id).toBe(TRACK_ID); + expect(second.getMediaSource()?.id).toBe('ms-2'); + expect(second.getOutboundRtps()?.map((rtp) => rtp.ssrc)).toEqual([ 2 ]); + expect(second.deltaPacketsSent).toBe(50); + expect(added).toEqual([ first, second ]); + expect(removed).toEqual([ first ]); + + // The old object keeps its own publication. + expect(first.getMediaSource()?.id).toBe('ms-1'); + observer.close(); + }); + + it('does not rotate while the media source stays the same, and includes RTPs that appear later', () => { + const { observer, added, removed, track } = setup(); + const p1: Publication = { mediaSourceId: 'ms-1', ssrc: 1, producerId: 'P1', packetsSent: 100 }; + const layer: Publication = { mediaSourceId: 'ms-1', ssrc: 11, producerId: 'P1', packetsSent: 5 }; + + observer.accept(publisherSample([ p1 ], p1)); + observer.accept(publisherSample([ p1, layer ], p1)); + + expect(track().getOutboundRtps()?.map((rtp) => rtp.ssrc)).toEqual([ 1, 11 ]); + expect(added).toHaveLength(1); + expect(removed).toHaveLength(0); + observer.close(); + }); + + it('replaces the track object when its first media source appears after it', () => { + const { observer, added, removed, track } = setup(); + const timestamp = nextTimestamp(); + + observer.accept({ + timestamp, callId: 'call-1', clientId: 'publisher', + peerConnections: [ { peerConnectionId: 'pub-pc', outboundTracks: [ { timestamp, id: TRACK_ID, kind: 'audio', attachments: { producerId: 'P1' } } ] } as unknown as PeerConnectionSample ], + }); + + const first = track(); + + expect(first.getMediaSource()).toBeUndefined(); + + const p1: Publication = { mediaSourceId: 'ms-1', ssrc: 1, producerId: 'P1', packetsSent: 100 }; + + observer.accept(publisherSample([ p1 ], p1)); + + expect(track()).not.toBe(first); + expect(track().getMediaSource()?.id).toBe('ms-1'); + expect(added).toHaveLength(2); + expect(removed).toEqual([ first ]); + observer.close(); + }); + + it('re-pairs consumers by producerId: P1 consumers unlink, P2 consumers link to the new object', () => { + const { observer, track } = setup(); + const p1: Publication = { mediaSourceId: 'ms-1', ssrc: 1, producerId: 'P1', packetsSent: 100 }; + + observer.accept(publisherSample([ p1 ], p1)); + observer.accept(consumerSample('P1')); + + const first = track(); + const call = observer.getObservedCall('call-1')!; + const resolver = call.remoteTrackResolver!; + + expect(first.remoteInboundTracks.size).toBe(1); + expect(resolver.getOutboundTrackByPublisherId('P1')).toBe(first); + + const p2: Publication = { mediaSourceId: 'ms-2', ssrc: 2, producerId: 'P2', packetsSent: 10 }; + + observer.accept(publisherSample([ p1, p2 ], p2)); + + const second = track(); + + // The old producer's entry is released, even though the track's attachments now say P2. + expect(resolver.getOutboundTrackByPublisherId('P1')).toBeUndefined(); + expect(resolver.getOutboundTrackByPublisherId('P2')).toBe(second); + expect(first.remoteInboundTracks.size).toBe(0); + + observer.accept(consumerSample('P2')); + expect(second.remoteInboundTracks.size).toBe(1); + observer.close(); + }); + + it('releases the publisher id the track was indexed under when it is removed, even if its producerId changed since', () => { + const { observer, track } = setup(); + const p1: Publication = { mediaSourceId: 'ms-1', ssrc: 1, producerId: 'P1', packetsSent: 100 }; + + observer.accept(publisherSample([ p1 ], p1)); + + const resolver = observer.getObservedCall('call-1')!.remoteTrackResolver!; + const first = track(); + + expect(resolver.getOutboundTrackByPublisherId('P1')).toBe(first); + + // Same media source, but the attachments now report another producer. + observer.accept(publisherSample([ p1 ], { ...p1, producerId: 'P-other' })); + expect(track()).toBe(first); + + // The publisher stops reporting the track: it is removed, and the P1 entry with it. + observer.accept({ timestamp: nextTimestamp(), callId: 'call-1', clientId: 'publisher', peerConnections: [ { peerConnectionId: 'pub-pc' } as unknown as PeerConnectionSample ] }); + expect(resolver.getOutboundTrackByPublisherId('P1')).toBeUndefined(); + observer.close(); + }); +}); From 29af30e8654c188b8f0f1462fb93bc64d490035f Mon Sep 17 00:00:00 2001 From: balazskreith Date: Tue, 20 Oct 2026 09:42:28 +0300 Subject: [PATCH 9/9] feat: add RollingJsonlFileSink for managing long-lived JSONL file outputs --- CHANGELOG.md | 10 ++ README.md | 38 +++++++ src/index.ts | 6 +- src/sinks/RollingJsonlFileSink.ts | 160 +++++++++++++++++++++++++++++ src/sinks/index.ts | 2 + tests/rollingJsonlFileSink.spec.ts | 127 +++++++++++++++++++++++ 6 files changed, 342 insertions(+), 1 deletion(-) create mode 100644 src/sinks/RollingJsonlFileSink.ts create mode 100644 tests/rollingJsonlFileSink.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e45aaa6..5dfdf1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,16 @@ call.extensionStats.values; // Map` and `CreateExtensionStatsSettings` are exported. +### `RollingJsonlFileSink` (new) + +A JSONL sink for long-lived owners, such as an observer's extension stats over days: it writes to a +sequence of files and rolls to the next before a write that would exceed `maxBytes`, once the +current file has been open for `maxAgeMs`, or on `roll()`. `path({ index, openedAt })` names each +file, and `file-closed` (`path`, `index`) announces each finished one in order; `close` fires after +the last. Limits are checked on write, so no timers run and an empty file is never rolled. +`JsonlFileSink` is unchanged. Exported with `RollingJsonlFileSinkOptions`, +`RollingJsonlFileSinkEvents` and `RollingJsonlFileInfo`. + ### Fixed: outbound track republished on the same `MediaStreamTrack` A producer recreated on the same track (reconnection, toggling audio processing) left the diff --git a/README.md b/README.md index 79681dc..268a8d9 100644 --- a/README.md +++ b/README.md @@ -1975,6 +1975,7 @@ its id — refuse writes after it, and emit `close` / `error` for its destinatio | Export | Notes | |--------|-------| | `JsonlFileSink` / `createJsonlFileSink({ path, serializeSample? })` | appends one JSON line per record; wraps `fs.WriteStream` and re-emits its events; exposes `readonly path`. The directory **must exist** | +| `RollingJsonlFileSink` | JSONL over a sequence of files for long-lived owners: rolls on `maxBytes`, `maxAgeMs` or `roll()`; `file-closed` per finished file. See [Rolling files](#rolling-files) | | `InMemorySink` / `createInMemorySink(samples?)` | collects client samples into `.samples`; emits `close` on `end()` | ### Client samples @@ -2101,6 +2102,43 @@ and the extension stats' sinks are ended. The owner's other sinks end with its ` usual. `extensionStats.close()` can also be called directly; it ends only the extension stats' sinks. +### Rolling files + +`JsonlFileSink` writes one file for the sink's whole life — right for calls and clients, which come +and go. An owner that lives for days or months, such as the observer itself, needs its records cut +into several files. `RollingJsonlFileSink` does that: it writes JSONL to a sequence of files, closing +the current one and opening the next (*rolling*) before a write that would exceed `maxBytes`, once +the current file has been open for `maxAgeMs`, or when `roll()` is called. + +```ts +const sink = new RollingJsonlFileSink({ + id: 'sfu-cpu', + path: ({ index, openedAt }) => `./stats/sfu-cpu-${openedAt}-${index}.jsonl`, + maxBytes: 100 * 1024 * 1024, + maxAgeMs: 60 * 60 * 1000, +}); + +sink.on('file-closed', (path, index) => uploadFile(path)); // flushed & fd closed → ready + +observer.createExtensionStats({ id: 'sfu-cpu', sink }); +setInterval(async () => observer.acceptExtensionStats('sfu-cpu', await readCpu()), 5_000).unref(); +``` + +| Option / member | Meaning | +|-----------------|---------| +| `path({ index, openedAt })` | the path of each file, called when it is opened (`index` 0, 1, 2, …; `openedAt` = `Date.now()`). Must give a new path per `index` | +| `maxBytes?`, `maxAgeMs?` | roll before a write that would make the file larger than `maxBytes`, or once it has been open for `maxAgeMs` | +| `serializeSample?` | custom serializer; defaults to `JSON.stringify` | +| `roll()` | close the current file and continue in a new one | +| `path`, `index` | the file currently written to | + +- Nothing is dropped: each record goes to exactly one file. +- The limits are checked on `write`, so the sink runs no timers: a file is not rolled while nothing + is written to it, and an empty file is never rolled (`roll()` on one is a no-op). +- `file-closed` (`path`, `index`) fires for every finished file, in index order, the last one + included. `close` fires once, after `end()` has closed every file. The owner ends the sink as + usual — here when the observer closes. + ### Other records: e.g. mediasoup pipe transport stats ```ts diff --git a/src/index.ts b/src/index.ts index e423843..b933554 100644 --- a/src/index.ts +++ b/src/index.ts @@ -296,12 +296,13 @@ export type { TrendTesterConfig } from './utils/TrendTester'; export { createLogger, setObserverLogger } from './common/logger'; export type { Logger, ObserverLogger } from './common/logger'; // Sinks. `Sink` (and `ClientSampleSink = Sink`) is the interface to implement for a custom destination; -// `JsonlFileSink` / `InMemorySink` are the built-ins; `SinkRegistry` owns their lifecycle. +// `JsonlFileSink` / `RollingJsonlFileSink` / `InMemorySink` are the built-ins; `SinkRegistry` owns their lifecycle. export { SinkRegistry, ExtensionStatsRegistry, JsonlFileSink, createJsonlFileSink, + RollingJsonlFileSink, InMemorySink, createInMemorySink, } from './sinks'; @@ -317,6 +318,9 @@ export type { ClientSampleSink, ClientSampleSinkEvents, JsonlFileSinkOptions, + RollingJsonlFileSinkOptions, + RollingJsonlFileSinkEvents, + RollingJsonlFileInfo, } from './sinks'; export { Middleware } from './common/Middleware'; // `Observer.acceptMiddlewares` is a public field of this type, so the type has to be nameable. diff --git a/src/sinks/RollingJsonlFileSink.ts b/src/sinks/RollingJsonlFileSink.ts new file mode 100644 index 0000000..b2c4456 --- /dev/null +++ b/src/sinks/RollingJsonlFileSink.ts @@ -0,0 +1,160 @@ +import * as fs from 'fs'; +import { EventEmitter } from 'node:events'; +import type { Sink, SinkEvents, SinkSettings } from './Sink'; + +/** The file a `RollingJsonlFileSink` is about to open: the sequence number and when it opens. */ +export type RollingJsonlFileInfo = { + + /** 0 for the first file, then 1, 2, … */ + index: number, + + /** `Date.now()` when the file is opened. */ + openedAt: number, +}; + +export type RollingJsonlFileSinkOptions = Partial & { + + /** The path of each file, called when it is opened. Must give a new path per `index`. */ + path: (file: RollingJsonlFileInfo) => string, + + /** Roll before a write that would make the current file larger than this, in bytes. */ + maxBytes?: number, + + /** Roll before a write once the current file has been open for this long, in ms. */ + maxAgeMs?: number, + + /** Optional custom serializer; defaults to `JSON.stringify`. */ + serializeSample?: (record: T) => string, +}; + +export type RollingJsonlFileSinkEvents = { + ended: SinkEvents['ended']; + close: SinkEvents['close']; + error: SinkEvents['error']; + finish: SinkEvents['finish']; + drain: SinkEvents['drain']; + + /** A file is complete: flushed and its descriptor closed. Fires for every file, the last one included. */ + 'file-closed': [path: string, index: number], +}; + +export declare interface RollingJsonlFileSink { + on(event: E, listener: (...args: RollingJsonlFileSinkEvents[E]) => void): this; + once(event: E, listener: (...args: RollingJsonlFileSinkEvents[E]) => void): this; + off(event: E, listener: (...args: RollingJsonlFileSinkEvents[E]) => void): this; + emit(event: E, ...args: RollingJsonlFileSinkEvents[E]): boolean; +} + +/** + * A JSONL file sink for long-lived owners (e.g. an observer's extension stats over days): it writes + * to a sequence of files, closing the current one and opening the next — *rolling* — when it would + * exceed `maxBytes`, when it has been open for `maxAgeMs`, or when `roll()` is called. Nothing is + * dropped: each record goes to exactly one file. + * + * The limits are checked on `write`, so a file is never rolled while nothing is written to it, and + * an empty file is never rolled. `file-closed` announces each finished file (ready to upload or + * move), in index order; `close` fires once, after every file is closed by `end()`. + */ +export class RollingJsonlFileSink extends EventEmitter implements Sink { + public readonly id: string; + + private readonly _options: RollingJsonlFileSinkOptions; + private readonly _serializeSample: (record: T) => string; + private _stream!: fs.WriteStream; + private _path!: string; + private _index = -1; + private _openedAt = 0; + private _bytes = 0; + private _ended = false; + // Resolves once every file closed so far has closed and been announced, in index order: the + // streams close independently, so a later file could otherwise be announced before an earlier one. + private _closing: Promise = Promise.resolve(); + + public constructor(options: RollingJsonlFileSinkOptions) { + super(); + this._options = options; + this._serializeSample = options.serializeSample ?? ((record) => JSON.stringify(record)); + this._open(); + // A registry always passes an id; a standalone sink falls back to its first path. + this.id = options.id ?? this._path; + } + + /** The file currently written to. */ + public get path(): string { + return this._path; + } + + /** The index of the file currently written to. */ + public get index(): number { + return this._index; + } + + public get ended(): boolean { + return this._ended; + } + + public write(record: T): boolean { + if (this._ended) return false; + + const line = `${this._serializeSample(record)}\n`; + const bytes = Buffer.byteLength(line); + const { maxBytes, maxAgeMs } = this._options; + + if (maxAgeMs !== undefined && maxAgeMs <= Date.now() - this._openedAt) this.roll(); + if (maxBytes !== undefined && maxBytes < this._bytes + bytes) this.roll(); + + this._bytes += bytes; + + return this._stream.write(line); + } + + /** Close the current file and continue in a new one. A no-op on an empty file or an ended sink. */ + public roll(): void { + if (this._ended || this._bytes === 0) return; + + this._closeCurrent(); + this._open(); + } + + /** No more writes: emits `ended`, closes the current file (`file-closed`), then emits `close`. */ + public end(): void { + if (this._ended) return; + + this._ended = true; + this.emit('ended'); + this._closeCurrent(); + void this._closing.then(() => { + this.emit('finish'); + this.emit('close'); + }); + } + + private _open(): void { + ++this._index; + this._openedAt = Date.now(); + this._bytes = 0; + this._path = this._options.path({ index: this._index, openedAt: this._openedAt }); + + const stream = fs.createWriteStream(this._path, { flags: 'a' }); + + stream.on('drain', () => this.emit('drain')); + stream.on('error', (err) => this.emit('error', err)); + this._stream = stream; + } + + private _closeCurrent(): void { + const stream = this._stream; + const path = this._path; + const index = this._index; + // 'error' also ends the wait: a failed file is reported through 'error' and still announced. + const closed = new Promise((resolve) => { + stream.once('close', () => resolve()); + stream.once('error', () => resolve()); + }); + + this._closing = Promise.all([ this._closing, closed ]).then(() => { + this.emit('file-closed', path, index); + }); + stream.end(); + } +} diff --git a/src/sinks/index.ts b/src/sinks/index.ts index 5d1b76b..e8d3448 100644 --- a/src/sinks/index.ts +++ b/src/sinks/index.ts @@ -5,6 +5,8 @@ export type { SinkConstructor, SinkCreateSettings } from './SinkRegistry'; export type { ClientSampleSink, ClientSampleSinkEvents } from './ClientSampleSink'; export { JsonlFileSink, createJsonlFileSink } from './JsonlFileSink'; export type { JsonlFileSinkOptions } from './JsonlFileSink'; +export { RollingJsonlFileSink } from './RollingJsonlFileSink'; +export type { RollingJsonlFileSinkOptions, RollingJsonlFileSinkEvents, RollingJsonlFileInfo } from './RollingJsonlFileSink'; export { InMemorySink, createInMemorySink } from './InMemorySink'; export { ExtensionStatsRegistry } from './ExtensionStatsRegistry'; export type { CreateExtensionStatsSettings } from './ExtensionStatsRegistry'; diff --git a/tests/rollingJsonlFileSink.spec.ts b/tests/rollingJsonlFileSink.spec.ts new file mode 100644 index 0000000..928ef90 --- /dev/null +++ b/tests/rollingJsonlFileSink.spec.ts @@ -0,0 +1,127 @@ +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { Observer } from '../src/Observer'; +import { RollingJsonlFileSink } from '../src/sinks/RollingJsonlFileSink'; + +let dir: string; + +beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'rolling-')); +}); +afterEach(() => { + jest.restoreAllMocks(); + fs.rmSync(dir, { recursive: true, force: true }); +}); + +const fileOf = (index: number) => path.join(dir, `stats-${index}.jsonl`); +const readLines = (file: string) => fs.readFileSync(file, 'utf8').trim().split('\n').filter(Boolean).map((line) => JSON.parse(line)); + +/** End the sink and collect every `file-closed`, resolving on `close` (after the last file). */ +function endAndCollect(sink: RollingJsonlFileSink, closed: { path: string, index: number }[]) { + return new Promise((resolve) => { + sink.once('close', () => resolve()); + sink.end(); + }).then(() => closed); +} + +describe('RollingJsonlFileSink', () => { + it('rolls before a write that would exceed maxBytes, keeping every record in exactly one file', async () => { + const closed: { path: string, index: number }[] = []; + // Each line is `{"n":}\n` = 8 bytes: two lines per 16-byte file. + const sink = new RollingJsonlFileSink<{ n: number }>({ path: ({ index }) => fileOf(index), maxBytes: 16 }); + + sink.on('file-closed', (path, index) => { + closed.push({ path, index }); + }); + for (let n = 0; n < 5; ++n) expect(sink.write({ n })).toBe(true); + + expect(sink.index).toBe(2); + await endAndCollect(sink, closed); + + expect(closed).toEqual([ { path: fileOf(0), index: 0 }, { path: fileOf(1), index: 1 }, { path: fileOf(2), index: 2 } ]); + expect(readLines(fileOf(0))).toEqual([ { n: 0 }, { n: 1 } ]); + expect(readLines(fileOf(1))).toEqual([ { n: 2 }, { n: 3 } ]); + expect(readLines(fileOf(2))).toEqual([ { n: 4 } ]); + }); + + it('rolls on write once the file has been open for maxAgeMs, never while idle', async () => { + let now = 1_000_000; + + jest.spyOn(Date, 'now').mockImplementation(() => now); + + const opened: number[] = []; + const sink = new RollingJsonlFileSink({ + path: ({ index, openedAt }) => { + opened.push(openedAt); + + return fileOf(index); + }, + maxAgeMs: 60_000, + }); + + sink.write({ n: 1 }); + now += 59_999; + sink.write({ n: 2 }); + expect(sink.index).toBe(0); + + // An hour of silence: no file is opened until something is written. + now += 3_600_000; + expect(sink.index).toBe(0); + sink.write({ n: 3 }); + expect(sink.index).toBe(1); + expect(opened).toEqual([ 1_000_000, 1_000_000 + 59_999 + 3_600_000 ]); + + await endAndCollect(sink, []); + expect(readLines(fileOf(0))).toEqual([ { n: 1 }, { n: 2 } ]); + expect(readLines(fileOf(1))).toEqual([ { n: 3 } ]); + }); + + it('rolls on roll(), which is a no-op on an empty file; refuses writes once ended', async () => { + const closed: { path: string, index: number }[] = []; + const sink = new RollingJsonlFileSink({ path: ({ index }) => fileOf(index) }); + + sink.on('file-closed', (path, index) => { + closed.push({ path, index }); + }); + sink.roll(); + expect(sink.index).toBe(0); + + sink.write({ n: 1 }); + sink.roll(); + expect(sink.index).toBe(1); + expect(sink.path).toBe(fileOf(1)); + sink.write({ n: 2 }); + + await endAndCollect(sink, closed); + + expect(sink.ended).toBe(true); + expect(sink.write({ n: 3 })).toBe(false); + sink.roll(); + expect(closed.map((file) => file.index)).toEqual([ 0, 1 ]); + expect(readLines(fileOf(1))).toEqual([ { n: 2 } ]); + }); + + it('works as an observer extension stats sink, ended when the observer closes', async () => { + const observer = new Observer(); + const sink = new RollingJsonlFileSink({ id: 'sfu-cpu', path: ({ index }) => fileOf(index), maxBytes: 30 }); + + expect(observer.createExtensionStats({ id: 'sfu-cpu', sink })).toBe(sink); + + const closed: number[] = []; + + sink.on('file-closed', (_path, index) => { + closed.push(index); + }); + + for (let cpu = 10; cpu < 13; ++cpu) observer.acceptExtensionStats('sfu-cpu', { cpu }); + + const done = new Promise((resolve) => sink.once('close', () => resolve())); + + observer.close(); + await done; + + expect(closed).toEqual([ 0, 1 ]); + expect([ ...readLines(fileOf(0)), ...readLines(fileOf(1)) ]).toEqual([ { cpu: 10 }, { cpu: 11 }, { cpu: 12 } ]); + }); +});