Skip to content

rpc: report psrpc request delivery, expiry and claim outcomes - #1687

Open
u9g wants to merge 3 commits into
mainfrom
r1-psrpc-request-observer
Open

rpc: report psrpc request delivery, expiry and claim outcomes#1687
u9g wants to merge 3 commits into
mainfrom
r1-psrpc-request-observer

Conversation

@u9g

@u9g u9g commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

psrpc has three server-side lifecycle events the interceptor chain can't see, because the handler is never invoked: a request read off the bus, one dropped past its expiry, and settlement of the claim handshake. The last is why a caller's ErrNoResponse leaves no record on either side today — "never arrived" and "a server bid too late" look identical.

PSRPCMetricsObserver implements psrpc.RequestObserver (livekit/psrpc#118, released as v0.7.3), installed via psrpc.WithServerObserver in WithServerObservability, so services already using that pick it up unchanged:

livekit_psrpc_requests_received_total{service,method}
livekit_psrpc_requests_expired_total{service,method}
livekit_psrpc_claim_total{service,method,outcome}       # granted | lost_to_peer | timed_out
livekit_psrpc_claim_wait_time_ms{service,method,outcome}

timed_out claims and pre-dispatch expiry also log at warn. lost_to_peer is expected on broadcast RPCs; on a queue RPC it means two members got the same request.

@changeset-bot

changeset-bot Bot commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 489cc78

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@u9g
u9g force-pushed the r1-psrpc-request-observer branch 2 times, most recently from 63f1e75 to f48303a Compare July 29, 2026 16:04
@u9g
u9g marked this pull request as ready for review July 29, 2026 17:15
PSRPCMetricsObserver implements psrpc.RequestObserver, installed via
psrpc.WithServerObserver inside WithServerObservability. Reports the three
server-side lifecycle events the interceptor chain cannot see, because in
each case the handler is never invoked: a request read off the bus, a
request dropped past its expiry, and settlement of the claim handshake.
@u9g
u9g force-pushed the r1-psrpc-request-observer branch from f48303a to 661e964 Compare July 29, 2026 17:21
github-actions Bot and others added 2 commits July 29, 2026 17:23
The six middleware.MetricsObserver methods had no coverage. Extend the
gather helper to filter by service and read gauges, so tests in this
package stay independent of each other and of registry ordering.
Comment thread rpc/metrics.go
Comment on lines +297 to +298
logger.Warnw("psrpc request dropped: expired before dispatch", nil,
"service", info.Service, "method", info.Method, "lateBy", lateBy)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the metric captures most of the same information - let's skip logging here.

Comment thread rpc/metrics.go
m.claimTotal.WithLabelValues(info.Service, info.Method, outcome.String()).Inc()
m.claimWaitTime.WithLabelValues(info.Service, info.Method, outcome.String()).Observe(float64(wait.Milliseconds()))

if outcome == psrpc.ClaimTimedOut {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here

Comment thread rpc/metrics.go

func (o PSRPCMetricsObserver) OnClaim(info psrpc.RPCInfo, outcome psrpc.ClaimOutcome, wait time.Duration) {
m := metrics.Load()
m.claimTotal.WithLabelValues(info.Service, info.Method, outcome.String()).Inc()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the histogram will have a _count already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants