Skip to content

rpc: gate the claim skip process-wide for every server - #1729

Merged
erikhortsch merged 3 commits into
mainfrom
erik/server-skip-claim
Aug 24, 2026
Merged

rpc: gate the claim skip process-wide for every server#1729
erikhortsch merged 3 commits into
mainfrom
erik/server-skip-claim

Conversation

@erikhortsch

@erikhortsch erikhortsch commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Wires psrpc's new WithServerSkipClaim (livekit/psrpc#122) into
WithServerObservability.

WithServerObservability is the one seam every server constructor shares:
WithDefaultServerOptions calls it, and the constructors that take only a logger
reach it too. In cloud that is 33 server sites — 23 via WithDefaultServerOptions
and 10 via WithServerObservability directly — covered by a single edit.

The setting is process-wide rather than per-service because the claim is a
transport policy, and it is read per request, so callers may set it before or
after their servers are constructed and revoke it at runtime without a redeploy.

Why not per-config

WithDefaultServerOptions already receives PSRPCConfig, so a field there was
the obvious home — but PSRPCConfig is passed by value at construction, which
would lose the runtime kill switch that motivated the client-side version
(livekit/cloud#4567). It also would not reach the 10 logger-only sites. A
process-wide setter keeps both properties.

Verification

  • go build ./... — clean
  • go test -race ./rpc/... ./utils/... — passes
  • New TestServerSkipClaim covers the three properties the call sites depend on:
    unset means claim, the func is consulted per call rather than captured, and it
    stays revocable.

Depends on

livekit/psrpc#122 — merged and released as v0.7.5, which go.mod now pins.
Ready to land.

Note the merged psrpc PR evolved past the revision this description was first
written against: the client option was removed in favor of an always-on
advertisement (new field number, old one reserved), so the server election wired
here is the only switch.

🤖 Generated with Claude Code

Wires psrpc's WithServerSkipClaim into WithServerObservability, which is the one
seam every server constructor shares -- WithDefaultServerOptions calls it, and
the constructors that take only a logger reach it too.

The setting is process-wide because the claim is a transport policy rather than a
per-service one, and it is read per request so callers may set it before or after
their servers exist and revoke it without a redeploy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 7674583

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@erikhortsch
erikhortsch marked this pull request as ready for review August 24, 2026 16:22
@erikhortsch
erikhortsch requested a review from a team August 24, 2026 16:22
Comment thread rpc/typed_api.go Outdated
return p.Bus, psrpc.WithClientOptions(p.Options()...)
}

var serverSkipClaim atomic.Pointer[func() bool]

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.

maybe rename this to psrpcServerSkipClaim. I don't think it is super clear even then, but there are so many references to server and client everywhere, hoping it makes at least a bit clearer. And the function names change to PsRPCSetServerSkipClaim etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 7674583 — went with SetPSRPCServerSkipClaim / psrpcServerSkipClaim rather than PsRPCSetServerSkipClaim: this package already spells the initialism PSRPC (PSRPCConfig, PSRPCMetricsObserver), and the verb-first setter matches Go convention. Cloud's call site updated in its PR.

Server and client mean too many things in this package for a bare
SetServerSkipClaim to read clearly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@erikhortsch
erikhortsch merged commit 14f581a into main Aug 24, 2026
9 checks passed
@erikhortsch
erikhortsch deleted the erik/server-skip-claim branch August 24, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants