[REA-6142] Export the transport statistics of a live WebRTC wire

[douglaseel]

The peer has sampled the wire every two seconds all along and nothing read
the result: a sample reached a log line when it carried a discard and was
dropped otherwise. A viewer reporting a stuttering picture had no number
behind it, because the metrics that used to carry one went away with the
2.8.x gauges.

Reading those samples properly meant taking reactor-webrtc 0.16.0, whose
report carries what 0.14.0 could not. The receiver's RTCP reports say what it
never got and how long the stream took to reach it, so outbound loss and the
round trip media actually travelled are measurements now rather than
guesses — and that round trip is not the one ICE takes, because connectivity
checks keep succeeding on a path whose media queue has grown. Congestion
control says what it believes the path will hold, which is the figure that
separates a model that stopped producing frames from a network that stopped
accepting them. The encoder says how many frames reached the wire, so the
frame rate a viewer saw is measured rather than inferred from the rate the
model emitted. ICE also marks the nominated pair, so the path's own round
trip and its estimate come from the pair carrying media instead of from the
first one that happened to succeed.

Bitrate is deliberately not an instrument of its own. Bytes sent and bytes
received are counted, and a rate over either is the bitrate the track
achieved — which is the number worth having, where the encoder's target
bitrate only ever said what it was aiming at.

Every count arrives as a total for the life of the wire, so a counter cannot
take it straight. One recorder per connection holds the previous sample and
moves each counter by what the window cost, then dies with its connection
while the instruments stay on the shared registry. That keeps the series a
process holds fixed however many connections it serves, and it is the
opposite of the 2.8.x shape, where a track's last value was republished on
every scrape until the process died and a flat plateau read as a measurement.
Loss shares one counter across both directions because the direction is what
tells the two apart, and the loss fraction the receiver reports rides a
histogram beside it, so a query reads the share of a stream that went missing
without dividing two counters. A zero fraction counts: dropping it would
leave the histogram holding only the bad windows. What the receiver has not
reported on yet is absent instead, since libwebrtc holds that round trip at
zero until the first report lands and a zero there is silence, not a clean
path.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Signed-off-by: Douglas Ferreira douglaseel@gmail.com

[douglaseel]

• [REA-6142] Export the transport statistics of a live WebRTC wire #179 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[ggoldens]

LGTM, please check the unit tests since they are failing.

[github-actions]

[codex-review] - [P1] acceptor.py: Client-controlled track labels allow unbounded Prometheus series growth.

• [P2] metrics.py: Differencing keys conflate same-named inbound and outbound counters.

Scope: full (f01fd6f..4de314b).

View workflow run.

[Dere-Wah]

The measurement design here is the strong part: differencing per connection while the instruments stay on the shared registry is the right answer to counters that arrive as lifetime totals, and separating the ICE round trip from the one the receiver measures on the stream is the distinction that makes the data actionable. Coverage is real too — test_serve asserting the families reach /metrics through the assembly catches the failure mode a registry-by-hand test cannot.

One structural comment on where the new module sits, and two smaller ones inline. None of them block; approving on the assumption they get a look before merge.

On the two earlier codex-review findings, both read as resolved at this head: allowed_tracks/label() bounds the label values (P1), and _advance's key now carries the direction labels (P2). No action needed.

[Dere-Wah]

The dependency arrow between these two modules now points both ways: metrics imports transport.webrtc.metrics for the re-exports, that module imports the registry back, and runtime_metrics.py exists to break the cycle that creates. So a new root module is paying for a re-export rather than for a layering decision — and metrics.py / runtime_metrics.py sitting side by side under near-identical names is hard to navigate.

Worth noting nothing in src/ reads either symbol off this module. router.py and acceptor.py both import WebRtcMetrics from transport.webrtc.metrics directly. The only two consumers are tests/unit/transport/webrtc/test_acceptor.py:9 and test_webrtc_metrics.py:6.

Suggest pointing those two test imports at the real module, dropping lines 47-52, and folding RuntimeMetrics back in here. The arrow then runs one way — transport.webrtc.metrics → metrics — which is the direction main already has, and the cycle-breaker module goes away. I tried it locally: net −5 lines, one module fewer, ruff check/ruff format clean, 1442 unit tests passing.

If the registry is meant to be its own layer rather than an accident of the cycle, that is a fine call to make — but then it is worth making it explicitly and giving it a name and a docstring that say so, since a metrics package with the registry in __init__ and one module per subsystem is the shape that scales to a second transport without another root module.

[Dere-Wah]

ConnectionStatsRecorder reads twenty private attributes off WebRtcMetrics (group._packets_sent, self._metrics._rtt, and so on). It works because the two share a module, but it makes them one unit with a seam drawn through the middle, and SLF is not in the ruff select list, so nothing will flag it if the pattern spreads.

Consider holding the instruments in a small frozen dataclass that WebRtcMetrics builds once and passes to the recorder. The twenty _ reads become ordinary attribute access on a value object the recorder legitimately owns, and the split between "declares instruments" and "differences totals" becomes one a reader can see rather than one the module boundary hides.

[Dere-Wah]

track_names is annotated Callable[[], Iterable[str]] and receives runner.track_map, which returns Mapping[str, Any]. It type-checks and yields the right values, but only because iterating a mapping gives its keys — the parameter asks for names and the caller supplies a manifest. Passing lambda: runner.track_map().keys() at the router.py call site would make the intent legible without changing behaviour.

[Dere-Wah]

Following up on the layering comment with the point that matters more than the cycle itself: where this leaves the package root. Two inline notes proposing a metrics/ package.

[Dere-Wah]

Worth separating this from the cycle argument, because it outlives it: this file sets a precedent for a surface that grows a root module per subsystem.

Metrics now live in three places — metrics.py at 460 lines, this at 36, and transport/webrtc/metrics.py at 426 — and nothing in the two root filenames tells a reader which to open. The next subsystem that needs the registry and cannot import metrics.py gets a fourth. The package root already carries eleven single modules against seven packages, so it is the part of the tree least able to absorb that.

Every other subsystem here that outgrew one file became a package instead: core, http, interface, protocol, recording, runner, transport, each with an __init__ that re-exports its surface behind an __all__. Metrics is the one that grew and split sideways.

The shape that matches the rest of the tree:

src/reactor_runtime/metrics/
    __init__.py    # CONTENT_TYPE, RuntimeMetrics, MetricsRecorder, CommandMetrics, ModelMetrics
    registry.py    # RuntimeMetrics — this file, correctly placed
    session.py     # MetricsRecorder, _reason_label, the session buckets
    command.py     # CommandMetrics, UNKNOWN_COMMAND
    model.py       # ModelMetrics

Three things make it cheap. The import path reactor_runtime.metrics is unchanged, so all fourteen existing call sites across serve, runner, http/*, and the tests stay as they are — it is a pure move. The cycle cannot come back, because the three recorder modules import metrics.registry while __init__ imports the four, and nothing in the package imports transport. And the bucket constants stop being a single undifferentiated block at the top of a 460-line file: each one sits with the instrument it shapes, where the comment explaining its boundaries is actually readable.

It also answers the scaling question directly. A second transport, or a metering or recording instrument group, adds a module beside its own code and imports the registry — the root stops growing a module per subsystem, which is the failure this file starts.

[Dere-Wah]

To be clear about scope: under the metrics/ package layout this file does not move. Instruments belong beside the code that observes them, and main already had transport/webrtc/{router,acceptor}.py importing the metrics module, so putting the WebRTC group here is the right call and the arrow direction is unchanged.

The only edit here is this line becoming from reactor_runtime.metrics import RuntimeMetrics — which is what router.py and acceptor.py already wrote before this PR. The problem to fix is at the root, not in this directory.

[douglaseel]

Merge activity

• Sep 11, 7:10 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Sep 11, 7:10 PM UTC: @douglaseel merged this pull request with Graphite.