From b2831f1b962dc944799bc6a367ad4f3e35ce1d2b Mon Sep 17 00:00:00 2001 From: Abhishek Ranjan Date: Fri, 20 Dec 2024 23:16:36 +0530 Subject: [PATCH] pull out logic to find name resolution delay --- clientconn.go | 11 +++----- stats/handlers.go | 5 ---- stats/opentelemetry/client_metrics.go | 6 +---- stream.go | 36 ++++++++++++--------------- 4 files changed, 20 insertions(+), 38 deletions(-) diff --git a/clientconn.go b/clientconn.go index 5f326647e11f..4f57b55434f9 100644 --- a/clientconn.go +++ b/clientconn.go @@ -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)) @@ -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 diff --git a/stats/handlers.go b/stats/handlers.go index c1dd190c1609..dc03731e45ef 100644 --- a/stats/handlers.go +++ b/stats/handlers.go @@ -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). diff --git a/stats/opentelemetry/client_metrics.go b/stats/opentelemetry/client_metrics.go index cb13182d7b9a..fc17c9be55be 100644 --- a/stats/opentelemetry/client_metrics.go +++ b/stats/opentelemetry/client_metrics.go @@ -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 diff --git a/stream.go b/stream.go index 3406a7d9744f..521472925a78 100644 --- a/stream.go +++ b/stream.go @@ -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} @@ -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 @@ -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) @@ -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, @@ -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)