Repository navigation
Conversation
pblazej
marked this pull request as ready for review
October 23, 2026 06:15
Contributor
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
pblazej
marked this pull request as draft
October 23, 2026 06:27
Move the protocol submodule to 003613b7, which adds otel/: the Weaver registry of telemetry names and the Rust constants generated from it. generate_proto.sh now copies otel/gen/rust/semconv.rs into src/, and the crate exposes it as `livekit_protocol::semconv`. The protobuf changes between the two submodule commits add field options only; generate_proto.sh produces the same livekit.rs and livekit.serde.rs as before. The generated file is left out of rustfmt and of the license header check, like the other generated sources.
Every event, span, checkpoint and attribute name the core sends, and every enum value with a registered member set, now comes from `livekit_protocol::semconv` instead of a string literal. Enum values are explicit matches on the generated constants, so a renamed variant no longer changes what ships. The core now sends the registry's spelling for the keys the registry renamed, the spelling Agents already sends: - lk.room.sid -> lk.room_sid - lk.participant.sid -> lk.participant_id - lk.track.sid -> lk.track_sid - lk.room.name -> lk.pii.room_name - lk.participant.identity -> lk.pii.participant_identity - lk.participant.remote_identity -> lk.pii.remote_participant_identity - lk.track.source -> lk.track_source, with the RTC FFI TrackSource names (SOURCE_CAMERA, SOURCE_MICROPHONE, SOURCE_SCREENSHARE, SOURCE_SCREENSHARE_AUDIO, SOURCE_UNKNOWN) - lk.disconnect.reason -> lk.disconnect_reason, with the protobuf DisconnectReason names (CLIENT_INITIATED, ..., UNKNOWN_REASON) and RECONNECT_FAILED The lk.room.disconnected body keeps the snake_case display line. SPEC.md and the tests follow the new names. Names the registry does not define stay literals: the unsent `unknown` thermal state, the `vpn`, `bluetooth` and `other` connection types, the `timed_out` error type, the `livekit-client` service name, the `custom.` event prefix, the `attempt <n> <mode>` checkpoint pattern and the audio route outputs.
One test runs a pipeline through one of every record, span and checkpoint the core produces, and through every variant of each enum it sends, decodes the OTLP it exports, and compares one line per signal with a frozen list: the keys of each record and span, its severities and checkpoints, the resource keys, the session keys, and the values of every enum-valued key. A registry change that renames a name or a value now fails this test.
pblazej
force-pushed
the
blaze/telemetry-stack/9-semconv
branch
from
October 23, 2026 09:56
692941c to
f562db5
Compare
pblazej
marked this pull request as ready for review
October 23, 2026 11:47
pblazej
requested review from
1egoman,
davidliu,
hiroshihorie,
lukasIO and
xianshijing-lk
October 23, 2026 11:48
lukasIO
approved these changes
Oct 27, 2026
lukasIO
left a comment
Contributor
There was a problem hiding this comment.
nice!
I haven't done any tests myself but only reviewed the diff by eye.
Contributor
There was a problem hiding this comment.
with the current setup generated output lives both in the protocol repo and here. I know it's only a copy, but it doesn't feel great.
Do you think creating an own crate for it in protocol would be over kill?
Contributor
Author
There was a problem hiding this comment.
really good point, any other benefits that would justify the infra?
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Rust telemetry core now takes every name it sends from the semantic conventions registry in livekit/protocol#1866. Semantic conventions are the agreed names, types and units of telemetry data. Every event name, span name, checkpoint name and attribute key comes from a generated constant, and so does every enum value that the registry lists. The core sends no metrics, so no metric name is involved.
The core also sends the spelling that the registry chose for eight keys. Agents already sends this spelling. Two of the keys also change their values: the track source and the disconnect reason now use the enum names that Agents sends.
A pin test runs one of every record, span and checkpoint through the pipeline and compares what goes out with a frozen list. A registry change that renames a name or a value fails that test.
This PR is stacked on #1396 (stage 8 of the telemetry stack).
Protocol change
The
livekit-protocol/protocolsubmodule moves from3fbe5098to003613b7, the head of livekit/protocol#1866. That PR is a draft and must merge first. After it merges, move the submodule to the merged commit and regenerate as described below.Between the two commits the protobufs change only by field options (
logger.sensitivity).generate_proto.shproduces the samelivekit.rsandlivekit.serde.rsas before.generate_proto.shnow also copiesprotocol/otel/gen/rust/semconv.rstolivekit-protocol/src/semconv.rs, and the crate exposes it aslivekit_protocol::semconv.livekit-telemetrydepends onlivekit-protocolfor it.Why a dependency and not a vendored copy
livekit-protocollivekit-telemetrylivekit-ffilinkslivekit-protocolin every build mode alreadycargo release publish --workspacepublishes in dependency order, and knope bumps the workspace dependencylivekit-telemetryalonelivekit-protocol,pbjson,serde,chrono)livekit-ffilivekit-protocolalreadyThe extra compile time hits only a project that builds
livekit-telemetryon its own. The only shipped consumer,livekit-ffi, already pays it. So the dependency is not clearly worse, and one generated copy is easier to keep correct.Renames
Each old key and old value is recorded in the registry as
renamed_fromon the new one.Keys
lk.room.sidlk.room_sidlk.room.namelk.pii.room_namelk.participant.sidlk.participant_idlk.participant.identitylk.pii.participant_identitylk.participant.remote_identitylk.pii.remote_participant_identitylk.subscribelk.track.sidlk.track_sidlk.publish,lk.subscribe,lk.rtc.stats.samplelk.track.sourcelk.track_sourcelk.publish,lk.subscribelk.disconnect.reasonlk.disconnect_reasonlk.room.disconnectedValues of
lk.track_source(the RTC FFITrackSourcenames)cameraSOURCE_CAMERAmicrophoneSOURCE_MICROPHONEscreen_shareSOURCE_SCREENSHAREscreen_share_audioSOURCE_SCREENSHARE_AUDIOunknownSOURCE_UNKNOWNValues of
lk.disconnect_reason(the protobufDisconnectReasonnames)client_initiated…agent_error(16 values)CLIENT_INITIATEDunknownUNKNOWN_REASONreconnect_failedRECONNECT_FAILED, the one value that only clients sendThe severity rule does not change. If the client hung up, the severity is
info. For every other reason it iswarn. The display body keeps snake case, for exampledisconnected: client_initiated.Names the registry does not define, which stay literals
unknownthermal statevpn,bluetooth,otherfornetwork.connection.typetimed_outforerror.typeerror.typeis open, and the registry gives it as an example onlylivekit-clientandlivekit-client-<sdk>forservice.namecustom.event prefixcustomnamespace, not the names in itattempt <n> <mode>checkpointlk.device.audio_route.outputsvalueslk.,otel.,code.INFO,WARN, …)livekit-ffihas no telemetry name literal in its code, so nothing there changes. Its doc comments name onlylk.outcome,lk.connect,lk.subscribeandlk.room.disconnected, which keep their names.How to regenerate
To change a name, change it in livekit/protocol first, as
otel/README.mdthere describes. Then do these steps:livekit-protocol/protocolsubmodule to the new protocol commit.livekit-protocol, run./generate_proto.sh. It needsprotoc,protoc-gen-prost@0.5.0andprotoc-gen-prost-serde@0.4.0. It regenerates the protobuf code and copiessemconv.rs.cargo test -p livekit-telemetry. If a name that the core sends changed,wire_tests::every_name_and_value_on_the_wire_is_pinnedfails. Update the pin andSPEC.mdin the same PR.Do not edit
src/semconv.rsby hand. The next run ofgenerate_proto.shoverwrites it, and theGenerate protocolworkflow runs that script on pull requests tomainthat touchlivekit-protocol/.Testing
livekit-telemetryhas a new test,wire_tests::every_name_and_value_on_the_wire_is_pinned. It drives one pipeline through every device state value and device event, log records from each source, and a self-telemetry report with every counter. A room then gets every typed span, checkpoint, reconnect reason, track source and outcome, and four RTC windows with every counter and gauge. All 18 disconnect reasons follow. Then it decodes the OTLP that the pipeline exports and compares one line per signal with a frozen list. The lines hold the record and span keys, severities, checkpoints, resource and session keys, and the values of 20 enum-valued keys. To prove that it catches a registry change, I changed one generated value (SOURCE_SCREENSHARE_AUDIO) by hand. The test failed. The other tests now use the new names.Gates at
f562db58Test counts are collected test cases: unit tests, then doc tests.
cargo fmt --checkcargo clippy -p livekit-telemetry --no-deps --all-targets -- -D warnings, also with--all-featuresand as acargo hackfeature powerset at depth 2cargo test -p livekit-telemetryRUSTDOCFLAGS="-D warnings" cargo doc -p livekit-telemetry --no-depscargo test -p livekit-ffi --no-default-features --features core-moduleslivekit-fficore-modules clippycargo clippy -p telemetry_ping --no-deps --all-targets -- -D warningscargo make --profile release android-packageinlivekit-ffiassembleReleasesucceeds. The size check fails as it does on the base branch (see Follow-ups)./gradlew compileReleaseKotlin --rerun-tasksinlivekit-ffi/support/androidcargo make dart-package, thendart testinlivekit-ffi/packages/dart--no-depsmatters now. Without it,cargo clippy -p livekit-telemetryalso lints its new path dependencylivekit-protocoland reports 687 errors. All of them are in the generated protobuf files (livekit.serde.rs,livekit.agent.serde.rs,livekit.rs,logger.serde.rs), which this PR does not change. No CI job runs clippy.Live-check and size
otel/live-check.sh(Weaver 0.28.0) on the pin test's capture: 68 log records, 17 spans, 25 resourcesinformationnotes for values that the registry does not define:network.connection.type=vpn,bluetooth,other, anderror.type=timed_outarm64-v8aliblivekit_ffi.so, releaseWeaver is not part of rust-sdks CI.
Follow-ups
livekit-protocol, so thatcargo clippy -p livekit-telemetrypasses without--no-deps.ANDROID_SIZE_LIMIT_BYTESbefore this PR, and this PR adds 168 bytes.network.connection.typevaluesvpn,bluetoothandother, and theerror.typevaluetimed_out.error.typeis an open value in OpenTelemetry, so the registry has to decide whether to listtimed_outas a value or leave the key open. Until then, the core sends these four as literals.Platform sites that name a renamed key or value
blaze/telemetryat96be2956Tests/LiveKitCoreTests/Telemetry/TelemetryTests.swift:218, 270blaze/telemetryat3da1b731livekit-android-test/.../telemetry/TelemetryMockE2ETest.kt:191-193, 196, 205, 215, 216, 223blaze/telemetryat8755fbetest/telemetry/telemetry_e2e_test.dart:245, 249, 279, 280, 284, 293blaze/telemetryatc00ade11src/telemetry/scope.ts:104-105, 276-279, 501-504, 651, 670-671,src/room/participant/LocalParticipant.ts:657, 662, 663,src/room/CoreRoom.ts:1879src/telemetry/telemetry.test.ts:704, 720, 779, 972, 1349, 1566,src/telemetry/telemetry.browser.test.ts:81, 184-185, 189, 194, 216-217, 223, 231In JS,
CoreRoom.ts:1879passes one value as the disconnect reason and as theerror.typeof the connect and reconnect spans. Only the disconnect reason changes toRECONNECT_FAILED.