add default deadline interceptor for gRPC clients#551
add default deadline interceptor for gRPC clients#551k4leung4 wants to merge 2 commits intochainguard-dev:mainfrom
Conversation
Signed-off-by: Kenny Leung <kleung@chainguard.dev>
- Fix RespectsExistingDeadline: server delay (200ms) now exceeds the interceptor timeout (100ms) so the test actually proves the caller's deadline governs. Previously the 50ms delay would succeed either way. - Extract dialWithDeadlineInterceptor helper to deduplicate server/client setup across all 3 tests (~50 lines removed). - Use t.Cleanup instead of defer for server/conn lifecycle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
eslerm
left a comment
eslerm
left a comment
There was a problem hiding this comment.
🤖: Good fix — defaultDeadlineInterceptor is well-placed (outermost, so deadline envelopes all retries) and the guard conditions are clean (!ok + timeout > 0). Tests are solid.
| grpc.WithStatsHandler(trace.PreserveTraceParentHandler), | ||
| grpc.WithChainUnaryInterceptor(clientid.UnaryClientInterceptor(), state().clientMetrics.UnaryClientInterceptor(), grpc_retry.UnaryClientInterceptor(retryOpts...)), | ||
| grpc.WithChainUnaryInterceptor(defaultDeadlineInterceptor(state().env.GrpcClientDefaultTimeout), clientid.UnaryClientInterceptor(), state().clientMetrics.UnaryClientInterceptor(), grpc_retry.UnaryClientInterceptor(retryOpts...)), | ||
| grpc.WithChainStreamInterceptor(clientid.StreamClientInterceptor(), state().clientMetrics.StreamClientInterceptor(), grpc_retry.StreamClientInterceptor(retryOpts...)), |
There was a problem hiding this comment.
🤖: WaitForReady(true) applies to both unary and streaming calls, but the deadline interceptor is only in the unary chain. A streaming RPC without a caller-provided deadline is still exposed to the same indefinite-blocking scenario the PR describes. Worth tracking as a follow-up — or a comment here noting streaming is intentionally out of scope.
| } | ||
| } | ||
|
|
||
| // defaultDeadlineInterceptor returns a unary client interceptor that sets a |
There was a problem hiding this comment.
🤖: Worth noting in the godoc that the 30s deadline is the total budget shared across all retry attempts (since this interceptor is outermost and grpc_retry is innermost). An operator tuning both GRPC_CLIENT_DEFAULT_TIMEOUT and GRPC_CLIENT_MAX_RETRY might not realize retries don't each get a fresh window.
2 participants
Summary
Add a unary client interceptor that sets a default 30s deadline on gRPC
calls that don't already have one. This prevents
WaitForReady(true)fromblocking indefinitely when a downstream is unavailable.
Motivation
Addresses CUS-238 /
CUS-172 /
chainguard-dev/internal-dev#18988.
WaitForReady(true)is set globally on all gRPC clients. When a downstreamis unavailable, RPCs block indefinitely waiting for the connection to become
ready, tying up goroutines until Cloud Run's per-instance concurrency limit
(50) is exhausted. During INC-44, this caused API instances to accumulate
blocked goroutines faster than Cloud Run could scale new instances.
Changes
GRPC_CLIENT_DEFAULT_TIMEOUT(default30s)defaultDeadlineInterceptorplaced first in the unary interceptorchain — if the caller's context has no deadline, one is applied
blanket timeout)
Behavior
GRPC_CLIENT_DEFAULT_TIMEOUT=0GRPC_CLIENT_DEFAULT_TIMEOUT=10sRisk
Services with intentionally long RPCs (e.g., apkoaas builds, image
assembly) should set an explicit context deadline or override the env var.
The 30s default is conservative — the API server's request timeout is 60
minutes for the build use case, but the API server always sets a context
deadline from the incoming request, so the interceptor won't override it.
Tests
TestDefaultDeadlineInterceptor_AppliesTimeout— slow server + shorttimeout →
DeadlineExceededTestDefaultDeadlineInterceptor_RespectsExistingDeadline— caller'slonger deadline is preserved, call succeeds
TestDefaultDeadlineInterceptor_DisabledWithZero— timeout=0 disablesthe interceptor, call succeeds
Test plan
go test ./pkg/options/ -v— 5/5 passgolangci-lint run ./pkg/options/— 0 issuesDeadlineExceedederrors🤖 Generated with Claude Code