server: observe request delivery, pre-dispatch expiry, and claim outcomes - #118
Open
u9g wants to merge 9 commits into
Open
server: observe request delivery, pre-dispatch expiry, and claim outcomes#118u9g wants to merge 9 commits into
u9g wants to merge 9 commits into
Conversation
…omes
Three server-side request lifecycle events are currently unobservable, and
they are exactly the ones that matter when a client reports a failure:
1. A request read off the bus is not counted anywhere.
2. A request that arrives after its expiry is dropped with no counter, no
log and no else branch.
3. A server that bids and is never granted the claim records nothing. The
client has already returned ErrNoResponse to its caller, so the request
exists in no log, metric or trace on either side.
Interceptors cannot cover these: they wrap the handler, which in all three
cases is never invoked.
Adds RequestObserver, installed via WithServerObserver, with events for
delivery, pre-dispatch expiry, and claim settlement (granted / lost_to_peer /
abandoned) carrying the time spent waiting on the client's decision. psrpc has
no metrics dependency, so this is a callback interface for the caller to
publish; no logging is added here, keeping levels the caller's concern.
The abandoned outcome makes two previously indistinguishable failures
distinguishable: a client ErrNoResponse with a matching abandoned claim means
the request was delivered and bid for, while one with no delivery event means
it never arrived. Also lets a queue RPC detect lost_to_peer, which implies
more than one member received the same request.
Tests cover the granted and abandoned paths, and assert that an abandoned
claim leaves the handler un-invoked -- the property that makes retrying
ErrNoResponse safe.
u9g
force-pushed
the
r1-claim-observability
branch
from
July 27, 2026 20:17
d68224c to
a493586
Compare
Lifecycle events fire outside the interceptor chain, so the observer has to reach the server itself. Requiring a separate WithServerObserver call meant every service had to opt in individually, while already passing an observer to WithServerMetrics. WithServerMetrics now type-asserts RequestObserver on the observer it is given and wires it when present. Callers using WithServerMetrics -- or a wrapper such as protocol's WithServerObservability -- get delivery, expiry and claim events by implementing three methods on their existing observer, with no call-site change. RequestObserver stays optional: a metrics-only observer is unaffected. WithServerObserver remains for callers that want the events without metrics.
paulwe
approved these changes
Jul 27, 2026
paulwe
left a comment
paulwe
left a comment
Contributor
There was a problem hiding this comment.
mostly ok. fwiw this probably isn't where dispatch is failing
Contributor
|
need to bump version in |
…rver Drop the RequestObserver type assertion in WithServerMetrics: it installed an unrelated feature invisibly from the option's signature, and let option order silently decide which observer won. Collapse the observer tests into one that covers delivery, both claim outcomes and handler suppression.
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
2 participants
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.
Three server-side request lifecycle events are unobservable today, and they are the ones that matter when a client reports a failure:
elsebranch.ErrNoResponseupstream, so the request exists in no log, metric or trace on either side.Interceptors can't cover these: they wrap the handler, which in all three cases is never invoked.
Change
Adds
RequestObserver, installed withWithServerObserver, with events for delivery, pre-dispatch expiry, and claim settlement (granted/lost_to_peer/abandoned) carrying the time spent waiting on the client's decision.A callback interface rather than metrics, since psrpc has no metrics dependency — publishing stays the caller's concern. No logging added, so log levels also stay the caller's concern.
Nil observer is the default and every call site is nil-checked, so this is inert unless opted into. Additive to
ServerOpts;MetricsObserveris untouched, so existing implementers are unaffected.Why
abandonedis the useful oneIt makes two previously identical failures distinguishable. A client
ErrNoResponsewith a matchingabandonedclaim means the request was delivered and bid for; one with no delivery event means it never arrived. Those have different causes and different fixes, and currently look the same from every vantage point.lost_to_peeris expected on broadcast RPCs, but on a queue RPC it means more than one member received the same request — also worth surfacing.Tests
internal/test/observer_test.gocovers the granted and abandoned paths, using a slow affinity function to delay the bid past the client's selection window deterministically. The abandoned test also asserts the handler was never invoked — the property that makes retryingErrNoResponsesafe, which had no regression test before.Note:
internal/test/my_servicefailsgo vetonmainalready (undefined: MyServiceServer, needs codegen); unrelated to this change.