cockroachdb / cockroachdb/cockroach

rpc: add DRPC interceptor equivalents for tests using gRPC-only ContextTestingKnobs interceptors

Open
#170,096 0 comments 0 reactions 0 assignees View on GitHub
C-enhancement T-db-server
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

## Problem

The `ContextTestingKnobs` struct (`pkg/rpc/context_testutils.go`) has parallel interceptor fields for both gRPC and DRPC:

```go
// gRPC
StreamClientInterceptor func(target string, class rpcbase.ConnectionClass) grpc.StreamClientInterceptor
UnaryClientInterceptor func(target string, class rpcbase.ConnectionClass) grpc.UnaryClientInterceptor

// DRPC
UnaryClientInterceptorDRPC func(target string, class rpcbase.ConnectionClass) drpcclient.UnaryClientInterceptor
StreamClientInterceptorDRPC func(target string, class rpcbase.ConnectionClass) drpcclient.StreamClientInterceptor
```

Several tests set only the gRPC interceptors. When DRPC is randomly enabled (via `TestDRPCEnabledRandomly`), the interceptor-based test logic is **silently bypassed** — DRPC connections don't go through gRPC interceptors. This means:
- Error injection doesn't fire → tests may pass vacuously or fail unexpectedly
- Latency measurement/logging is skipped → test observations are incomplete
- Interceptor validation doesn't run → coverage gap

## Affected Tests

### 1. `pkg/kv/kvclient/kvcoord/dist_sender_rangefeed_test.go` — `TestMuxRangeFeedDoesNotStallOnError`
- **Line ~356**: Sets `StreamClientInterceptor` to inject errors on `MuxRangeFeed` calls
- **Current workaround**: DRPC is disabled via `DefaultDRPCOption: base.TestDRPCDisabled`
- **Needs**: `StreamClientInterceptorDRPC` equivalent so the test works with DRPC enabled

### 2. `pkg/ccl/multiregionccl/cold_start_latency_test.go` — `TestColdStartLatency`
- **Line ~89**: `UnaryClientInterceptor` for host cluster node tracing/latency measurement
- **Line ~201**: `StreamClientInterceptor` for tenant tracing/latency measurement
- **Line ~233**: `UnaryClientInterceptor` for tenant-to-node latency measurement
- **Needs**: Both `UnaryClientInterceptorDRPC` and `StreamClientInterceptorDRPC` equivalents

### 3. `pkg/rpc/context_test.go` — `TestTestingKnobs`
- **Line ~1702**: Sets `StreamClientInterceptor` to record all stream RPC calls for verification
- **Needs**: `StreamClientInterceptorDRPC` equivalent

## Reference Pattern

`pkg/kv/kvserver/client_raft_test.go` — `TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic` properly sets both gRPC and DRPC interceptors (gRPC at line ~5048, DRPC at line ~5069) and can serve as the reference implementation.

## Desired Solution

For each affected test, add a corresponding DRPC interceptor that mirrors the gRPC interceptor's behavior:
1. Add the `StreamClientInterceptorDRPC` / `UnaryClientInterceptorDRPC` field alongside the existing gRPC interceptor in `ContextTestingKnobs`
2. Implement equivalent interception logic using `drpcclient.StreamClientInterceptor` / `drpcclient.UnaryClientInterceptor` types
3. Remove any `DefaultDRPCOption: base.TestDRPCDisabled` workarounds once the DRPC interceptors are in place

Epic: none

Jira issue: CRDB-63824

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.