Skip to content

Commit a25e93d

Browse files
authored
feat(bigtable): modularize Direct Access compatibility check (#19987)
## Summary - Extract the Direct Access compatibility decision out of `BigtableChannelPool` behind a new `DirectAccessChecker` interface so each channel pool factory can plug in its own strategy. - Today's only implementation, `pingAndWarmDirectAccessChecker`, preserves current behavior (PingAndWarm probe + ALTS check + async failure investigation + `direct_access/compatible` metric). The upcoming session pool factory will plug in a `GetClientConfiguration`-based checker (driven by `ClientConfigurationManager`) without touching the pool. - A small `disabledDirectAccessChecker` keeps the `direct_access/compatible{reason=manually_disabled}` reading when the env/config disables Direct Access — the env var check moves up to the factory so the pool no longer re-reads `CBT_ENABLE_DIRECTPATH`. ## Changes - **New** `internal/transport/direct_access_checker.go`: `DirectAccessChecker` interface, `pingAndWarmDirectAccessChecker`, `disabledDirectAccessChecker`, and the moved helpers (`xdsCdsURITemplate`, `checkIPPlumbing`, `checkKernelRoutes`, `probeSingleEndpoint`). - `internal/transport/connpool.go`: remove `directAccessDialer`, `directAccessFeatureFlagsMD`, `daEligibleGauge` fields, the `WithDirectAccessDialer` / `WithDirectAccessFeatureFlagsMetadata` options, the in-pool probe + investigation chain, the duplicate `CBT_ENABLE_DIRECTPATH` check, and the metric reporting helpers. Add `WithDirectAccessChecker`. `NewBigtableChannelPool` now just delegates to the checker. - `internal/transport/channel_pool_factory.go`: when `isDirectAccessEnabled(config)` is true, wire a `pingAndWarmDirectAccessChecker`; otherwise wire a `disabledDirectAccessChecker`. Both passed via `WithDirectAccessChecker`. - `internal/transport/connpool_test.go`: six `TestDirectAccessLogic` subtests updated to construct the appropriate checker. The env-disabled subtest becomes `DirectAccess_DisabledChecker` since the env-var gate has moved to the factory layer. ## Test plan - [x] `cd bigtable && go build ./...` - [x] `cd bigtable && go vet ./...` - [x] `cd bigtable && go test ./internal/transport/... -count=1` (full transport suite, including all six `TestDirectAccessLogic` subtests, `TestCreateAndStartManagedChannelPool*`, and `TestManagedChannelPool_Close`)
1 parent 0bbd701 commit a25e93d

6 files changed

Lines changed: 508 additions & 339 deletions

File tree

bigtable/internal/transport/channel_pool_factory.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,22 @@ func CreateBigtableChannelPool(
153153

154154
fullInstanceName := fmt.Sprintf("projects/%s/instances/%s", project, instance)
155155

156+
// directAccessMD is the feature-flag metadata used for priming on both
157+
// the direct-access and standard-path connection factories — the pool
158+
// holds it via WithFeatureFlagsMetadata and applies it to every Prime().
156159
poolOpts := []BigtableChannelPoolOption{
157160
WithInstanceName(fullInstanceName),
158161
WithAppProfile(config.AppProfile),
159-
WithFeatureFlagsMetadata(directAccessMD),
160162
WithMetricsReporterConfig(btopt.DefaultMetricsReporterConfig()),
161163
WithMeterProvider(otelMeterProvider),
162-
WithDirectAccessFeatureFlagsMetadata(directAccessMD),
164+
WithFeatureFlagsMetadata(directAccessMD),
163165
}
164166

167+
// Pluggable Direct Access strategy: the classic channel pool factory uses
168+
// a PingAndWarm probe; when the env/config disables Direct Access we still
169+
// wire a "disabled" checker so the direct_access/compatible metric keeps
170+
// surfacing the off state with reason=manually_disabled. The future
171+
// session pool factory will swap in a GetClientConfiguration-based checker.
165172
if isDirectAccessEnabled(config) {
166173
directAccessDialerOptions := make([]option.ClientOption, len(o))
167174
copy(directAccessDialerOptions, o)
@@ -175,7 +182,17 @@ func CreateBigtableChannelPool(
175182
}
176183
return NewBigtableConn(grpcConn), nil
177184
}
178-
poolOpts = append(poolOpts, WithDirectAccessDialer(directAccessDialer))
185+
checker := newPingAndWarmDirectAccessChecker(
186+
directAccessDialer,
187+
fullInstanceName,
188+
config.AppProfile,
189+
directAccessMD,
190+
otelMeterProvider,
191+
nil, // logger plumbed by callers once available
192+
)
193+
poolOpts = append(poolOpts, WithDirectAccessChecker(checker))
194+
} else {
195+
poolOpts = append(poolOpts, WithDirectAccessChecker(newDisabledDirectAccessChecker(otelMeterProvider, nil)))
179196
}
180197

181198
return NewBigtableChannelPool(ctx,

bigtable/internal/transport/conn_recycler_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestConnectionRecycler_CheckRecycle(t *testing.T) {
3838
MaxJitter: 0,
3939
}
4040

41-
pool, err := NewBigtableChannelPool(ctx, 1, btopt.RoundRobin, dialFunc, time.Now())
41+
pool, err := NewBigtableChannelPool(ctx, 1, btopt.RoundRobin, dialFunc, time.Now(), WithDirectAccessChecker(newDisabledDirectAccessChecker(nil, nil)))
4242
if err != nil {
4343
t.Fatalf("Failed to create pool: %v", err)
4444
}
@@ -70,7 +70,7 @@ func TestConnectionRecycler_CheckRecycle(t *testing.T) {
7070
MaxJitter: 0,
7171
}
7272

73-
pool, err := NewBigtableChannelPool(ctx, 1, btopt.RoundRobin, dialFunc, time.Now())
73+
pool, err := NewBigtableChannelPool(ctx, 1, btopt.RoundRobin, dialFunc, time.Now(), WithDirectAccessChecker(newDisabledDirectAccessChecker(nil, nil)))
7474
if err != nil {
7575
t.Fatalf("Failed to create pool: %v", err)
7676
}
@@ -99,7 +99,7 @@ func TestConnectionRecycler_CheckRecycle(t *testing.T) {
9999
}
100100
// 5 conns
101101
poolSize := 5
102-
pool, err := NewBigtableChannelPool(ctx, poolSize, btopt.RoundRobin, dialFunc, time.Now())
102+
pool, err := NewBigtableChannelPool(ctx, poolSize, btopt.RoundRobin, dialFunc, time.Now(), WithDirectAccessChecker(newDisabledDirectAccessChecker(nil, nil)))
103103
if err != nil {
104104
t.Fatalf("Failed to create pool: %v", err)
105105
}

0 commit comments

Comments
 (0)