Skip to content

balancer: expose parent ClientConn via Unwrap() on all embedding wrappers - #9363

Closed
sushanb wants to merge 1 commit into
grpc:masterfrom
sushanb:add/balancer-clientconn-unwrap
Closed

balancer: expose parent ClientConn via Unwrap() on all embedding wrappers#9363
sushanb wants to merge 1 commit into
grpc:masterfrom
sushanb:add/balancer-clientconn-unwrap

Conversation

@sushanb

@sushanb sushanb commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Several types in the balancer tree wrap a balancer.ClientConn by anonymously embedding it. This PR adds an Unwrap() balancer.ClientConn method to each of them so that gRPC-internal callers can walk up the wrapping chain to reach the underlying parent ClientConn. This is the mirror-image contract of what errors.Unwrap provides for error trees.

The method is a one-liner returning the embedded field, and doesn't change any existing behavior. It exists so a follow-up change can rely on it to inherit parent-channel telemetry (stats handlers and interceptors) onto balancer-owned control channels (e.g. the RLS control channel) without resorting to reflection over private fields.

Wrappers touched

  • internal/balancer/gracefulswitch.balancerWrapper
  • internal/balancergroup.subBalancerWrapper
  • internal/xds/balancer/clusterimpl.clusterImplBalancer
  • internal/xds/balancer/outlierdetection.outlierDetectionBalancer
  • internal/xds/balancer/priority.ignoreResolveNowClientConn
  • balancer/grpclb.lbCacheClientConn
  • balancer/ringhash.ringhashBalancer

Test plan

  • Existing tests pass (no behavior change)

RELEASE NOTES: none

…pers

Several types in the balancer tree wrap a balancer.ClientConn by
anonymously embedding it. Add an Unwrap() balancer.ClientConn method to
each of them so that gRPC-internal callers can walk up the wrapping
chain to reach the underlying parent ClientConn. This is the
mirror-image contract of what errors.Unwrap provides for error trees.

The method is a one-liner returning the embedded field, and doesn't
change any existing behavior. It exists so a follow-up change can rely
on it to inherit parent-channel telemetry (stats handlers and
interceptors) onto balancer-owned control channels (e.g. the RLS
control channel) without resorting to reflection over private fields.

Wrappers touched:
  internal/balancer/gracefulswitch.balancerWrapper
  internal/balancergroup.subBalancerWrapper
  internal/xds/balancer/clusterimpl.clusterImplBalancer
  internal/xds/balancer/outlierdetection.outlierDetectionBalancer
  internal/xds/balancer/priority.ignoreResolveNowClientConn
  balancer/grpclb.lbCacheClientConn
  balancer/ringhash.ringhashBalancer

RELEASE NOTES: none
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.43%. Comparing base (9d1988d) to head (3e994fa).

Files with missing lines Patch % Lines
balancer/grpclb/grpclb_util.go 0.00% 1 Missing ⚠️
balancer/ringhash/ringhash.go 0.00% 1 Missing ⚠️
internal/balancer/gracefulswitch/gracefulswitch.go 0.00% 1 Missing ⚠️
internal/balancergroup/balancergroup.go 0.00% 1 Missing ⚠️
internal/xds/balancer/clusterimpl/clusterimpl.go 0.00% 1 Missing ⚠️
internal/xds/balancer/outlierdetection/balancer.go 0.00% 1 Missing ⚠️
...ternal/xds/balancer/priority/ignore_resolve_now.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9363      +/-   ##
==========================================
- Coverage   87.45%   87.43%   -0.03%     
==========================================
  Files         425      425              
  Lines       30297    30304       +7     
==========================================
- Hits        26496    26495       -1     
- Misses       3801     3809       +8     
Files with missing lines Coverage Δ
balancer/grpclb/grpclb_util.go 81.35% <0.00%> (-4.86%) ⬇️
balancer/ringhash/ringhash.go 95.55% <0.00%> (-0.72%) ⬇️
internal/balancer/gracefulswitch/gracefulswitch.go 89.47% <0.00%> (-0.60%) ⬇️
internal/balancergroup/balancergroup.go 88.42% <0.00%> (-1.53%) ⬇️
internal/xds/balancer/clusterimpl/clusterimpl.go 92.13% <0.00%> (-0.35%) ⬇️
internal/xds/balancer/outlierdetection/balancer.go 92.30% <0.00%> (-0.26%) ⬇️
...ternal/xds/balancer/priority/ignore_resolve_now.go 87.50% <0.00%> (-12.50%) ⬇️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sushanb

sushanb commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Converting to draft while I re-implement the fix to conform to gRFC A110 (Child Channel Options). This PR's approach (auto-inheriting the parent channel's stats handlers + interceptors via an internal walk) violates A110's explicit rule that O_child be opaque to P, and doesn't add the public grpc.WithChildChannelOptions/balancer.BuildOptions.ChildChannelOptions API that A110 mandates. Replacement PR incoming — will link here.

@eshitachandwani

Copy link
Copy Markdown
Member

We generally do not prefer keeping draft PRs, please re-open the PR once the implementation is done and is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants