rpc: add error_code label to psrpc error_total metric - #1699
Merged
Conversation
Failed RPCs were counted with only role/kind/service/method, so a spike in livekit_psrpc_error_total gave no indication of how the RPCs failed -- unavailable, deadline_exceeded and permission_denied were indistinguishable without a log dive. psrpc already carries a bounded error taxonomy that was being discarded at the metrics boundary. Label values come from psrpc.GetErrorCode, so the domain is exactly psrpc's ErrorCode constants; err.Error() is never used as a label value. Multi requests report "unknown" because middleware.MetricsObserver.OnMultiRequest has no error parameter. Label slices are built with slices.Concat rather than append: append(labels, ...) only reallocates today because maps.Keys returns len == cap, so a second append onto labels would let errorLabels and bytesLabels write the same index of a shared backing array.
🦋 Changeset detectedLatest commit: 80890e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
boks1971
approved these changes
Aug 4, 2026
milos-lk
approved these changes
Aug 4, 2026
This was referenced Aug 4, 2026
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.
Why
livekit_psrpc_error_totalcounted failed RPCs with onlyrole, kind, service, method. When it climbed there was no way to tell how the RPCs failed — a burst ofunavailable(load/affinity),deadline_exceeded(timeouts) andpermission_denied(caller misconfiguration) were indistinguishable, so every spike needed a log dive to triage. psrpc already carries a bounded error taxonomy (psrpc.ErrorCode, ~23 constants) that was being thrown away at the metrics boundary.What
error_codeadded as a trailing label onerror_totalonly; the other five collectors are untouched.errorCodeLabelhelper wrapping psrpc's existingGetErrorCode, so the label domain is exactly psrpc'sErrorCodeconstants —err.Error()is never used as a label value, keeping cardinality bounded.psrpc.OK(the empty string) and unresolvable errors both collapse tounknown.slices.Concatinstead ofappend. This is a correctness prerequisite, not cleanup:bytesLabels := append(labels, …)only reallocates today becausemaps.Keyshappens to returnlen == cap. Adding a secondappend(labels, …)forerrorLabelswould make both slices write indexlen(labels)of the same backing array wheneverlabelshad spare capacity, silently registering one collector with the other's label name.Known limitation
Multi requests always report
error_code="unknown". psrpc'smiddleware.MetricsObserver.OnMultiRequesthas no error parameter andmultiRPCMetricsInterceptor.Recvdrops the error, so the code isn't available. These rows stay separable via the existingkind="multirpc". Surfacing real codes would need anerrparam on that psrpc interface — breaking, though the only implementors anywhere are the two observers in this same file.Bare
context.Canceled/context.DeadlineExceededalso land inunknown, since psrpc doesn't special-case them. Left alone deliberately — ifkind="stream"turns out to be dominated byunknownin practice, that's a two-case pre-check to add later.Rollout
Adding a label changes the series identity of
livekit_psrpc_error_total. Aggregating queries are unaffected — the production alert rules usesum(rate(livekit_psrpc_error_total{...})). Expect a one-scrape discontinuity as old series are replaced by new ones (normal counter-reset handling inrate()). Only queries matching the full label set exactly would need updating.Test plan
go build ./...,go vet ./rpc/, full suite green (30 packages).rpc/metrics_test.godrives all four error paths and gathers from the default registry. Deliberately avoidsprometheus/testutil— itskylelemons/godebugdep is absent fromgo.sumand importing it would force ago.modchange.errorCodeLabelto return"", confirmed the test fails, reverted.nats_servercurry label), since that's where theWithLabelValuesarity risk lives:error_code=deadline_exceededlanded alongsidenats_server=nats-a.go buildin cloud-protocol / cloud-io / cloud-egress / egress. cloud-io, cloud-egress and egress fail, but on a stalebackend-common/observability/gatewayobsimplgenerated reporter and an egress embed pattern — confirmed identical failures with this change stashed. None touch rpc metrics.