Skip to content

Commit

Permalink
pull out logic to find name resolution delay
Browse files Browse the repository at this point in the history
  • Loading branch information
aranjans committed Dec 20, 2024
1 parent b6503f7 commit b2831f1
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 38 deletions.
11 changes: 3 additions & 8 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,9 @@ func (dcs *defaultConfigSelector) SelectConfig(rpcInfo iresolver.RPCInfo) (*ires
// function.
func NewClient(target string, opts ...DialOption) (conn *ClientConn, err error) {
cc := &ClientConn{
target: target,
conns: make(map[*addrConn]struct{}),
dopts: defaultDialOptions(),
nameResolutionDelayed: false,
target: target,
conns: make(map[*addrConn]struct{}),
dopts: defaultDialOptions(),
}

cc.retryThrottler.Store((*retryThrottler)(nil))
Expand Down Expand Up @@ -605,10 +604,6 @@ type ClientConn struct {
idlenessMgr *idle.Manager
metricsRecorderList *stats.MetricsRecorderList

// To track if there was a delay in name resolution, helping to track
// latency issues in gRPC connection setup.
nameResolutionDelayed bool

// The following provide their own synchronization, and therefore don't
// require cc.mu to be held to access them.
csMgr *connectivityStateManager
Expand Down
5 changes: 0 additions & 5 deletions stats/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ type RPCTagInfo struct {
// FailFast indicates if this RPC is failfast.
// This field is only valid on client side, it's always false on server side.
FailFast bool
// NameResolutionDelay indicates whether there was a delay in name
// resolution.
//
// This field is only valid on client side, it's always false on server side.
NameResolutionDelay bool
}

// Handler defines the interface for the related stats handling (e.g., RPCs, connections).
Expand Down
6 changes: 1 addition & 5 deletions stats/opentelemetry/client_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,7 @@ func (h *clientStatsHandler) TagRPC(ctx context.Context, info *stats.RPCTagInfo)
ai := &attemptInfo{}
startTime := time.Now()
if !isTracingDisabled(h.options.TraceOptions) {
callSpan := trace.SpanFromContext(ctx)
if info.NameResolutionDelay {
callSpan.AddEvent("Delayed name resolution complete")
}
ctx, ai = h.traceTagRPC(trace.ContextWithSpan(ctx, callSpan), info)
ctx, ai = h.traceTagRPC(ctx, info)
}
ai.startTime = startTime
ai.xdsLabels = labels.TelemetryLabels
Expand Down
36 changes: 16 additions & 20 deletions stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,12 @@ func newClientStream(ctx context.Context, desc *StreamDesc, cc *ClientConn, meth
// Provide an opportunity for the first RPC to see the first service config
// provided by the resolver.
if err := cc.waitForResolvedAddrs(ctx); err != nil {
cc.nameResolutionDelayed = true
return nil, err
}
var mc serviceconfig.MethodConfig
var onCommit func()
newStream := func(ctx context.Context, done func()) (iresolver.ClientStream, error) {
return newClientStreamWithParams(ctx, desc, cc, method, mc, onCommit, done, cc.nameResolutionDelayed, opts...)
return newClientStreamWithParams(ctx, desc, cc, method, mc, onCommit, done, opts...)
}

rpcInfo := iresolver.RPCInfo{Context: ctx, Method: method}
Expand Down Expand Up @@ -257,7 +256,7 @@ func newClientStream(ctx context.Context, desc *StreamDesc, cc *ClientConn, meth
return newStream(ctx, func() {})
}

func newClientStreamWithParams(ctx context.Context, desc *StreamDesc, cc *ClientConn, method string, mc serviceconfig.MethodConfig, onCommit, doneFunc func(), nameResolutionDelayed bool, opts ...CallOption) (_ iresolver.ClientStream, err error) {
func newClientStreamWithParams(ctx context.Context, desc *StreamDesc, cc *ClientConn, method string, mc serviceconfig.MethodConfig, onCommit, doneFunc func(), opts ...CallOption) (_ iresolver.ClientStream, err error) {
c := defaultCallInfo()
if mc.WaitForReady != nil {
c.failFast = !*mc.WaitForReady
Expand Down Expand Up @@ -321,20 +320,19 @@ func newClientStreamWithParams(ctx context.Context, desc *StreamDesc, cc *Client
}

cs := &clientStream{
callHdr: callHdr,
ctx: ctx,
methodConfig: &mc,
opts: opts,
callInfo: c,
cc: cc,
desc: desc,
codec: c.codec,
cp: cp,
comp: comp,
cancel: cancel,
firstAttempt: true,
onCommit: onCommit,
nameResolutionDelayed: nameResolutionDelayed,
callHdr: callHdr,
ctx: ctx,
methodConfig: &mc,
opts: opts,
callInfo: c,
cc: cc,
desc: desc,
codec: c.codec,
cp: cp,
comp: comp,
cancel: cancel,
firstAttempt: true,
onCommit: onCommit,
}
if !cc.dopts.disableRetry {
cs.retryThrottler = cc.retryThrottler.Load().(*retryThrottler)
Expand Down Expand Up @@ -418,7 +416,7 @@ func (cs *clientStream) newAttemptLocked(isTransparent bool) (*csAttempt, error)
var beginTime time.Time
shs := cs.cc.dopts.copts.StatsHandlers
for _, sh := range shs {
ctx = sh.TagRPC(ctx, &stats.RPCTagInfo{FullMethodName: method, FailFast: cs.callInfo.failFast, NameResolutionDelay: cs.nameResolutionDelayed})
ctx = sh.TagRPC(ctx, &stats.RPCTagInfo{FullMethodName: method, FailFast: cs.callInfo.failFast})
beginTime = time.Now()
begin := &stats.Begin{
Client: true,
Expand Down Expand Up @@ -556,8 +554,6 @@ type clientStream struct {
// synchronized.
serverHeaderBinlogged bool

nameResolutionDelayed bool

mu sync.Mutex
firstAttempt bool // if true, transparent retry is valid
numRetries int // exclusive of transparent retry attempt(s)
Expand Down

0 comments on commit b2831f1

Please sign in to comment.