Skip to content

Commit 535e619

Browse files
easwarseaswars
committed
some more cleanups
1 parent a480380 commit 535e619

File tree

5 files changed

+146
-99
lines changed

5 files changed

+146
-99
lines changed

‎channelz/service/service_test.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424
"net"
2525
"strconv"
26+
"strings"
2627
"testing"
2728
"time"
2829

@@ -477,13 +478,15 @@ func (s) TestGetServerSocketsNonZeroStartID(t *testing.T) {
477478
func (s) TestGetChannel(t *testing.T) {
478479
czCleanup := channelz.NewChannelzStorageForTesting()
479480
defer cleanupWrapper(czCleanup, t)
481+
480482
refNames := []string{"top channel 1", "nested channel 1", "sub channel 2", "nested channel 3"}
481483
ids := make([]*channelz.Identifier, 4)
482484
ids[0] = channelz.RegisterChannel(&dummyChannel{}, nil, refNames[0])
483485
channelz.AddTraceEvent(logger, ids[0], 0, &channelz.TraceEventDesc{
484486
Desc: "Channel Created",
485487
Severity: channelz.CtInfo,
486488
})
489+
487490
ids[1] = channelz.RegisterChannel(&dummyChannel{}, ids[0], refNames[1])
488491
channelz.AddTraceEvent(logger, ids[1], 0, &channelz.TraceEventDesc{
489492
Desc: "Channel Created",
@@ -507,6 +510,7 @@ func (s) TestGetChannel(t *testing.T) {
507510
Severity: channelz.CtInfo,
508511
},
509512
})
513+
510514
ids[3] = channelz.RegisterChannel(&dummyChannel{}, ids[1], refNames[3])
511515
channelz.AddTraceEvent(logger, ids[3], 0, &channelz.TraceEventDesc{
512516
Desc: "Channel Created",
@@ -524,9 +528,11 @@ func (s) TestGetChannel(t *testing.T) {
524528
Desc: "Resolver returns an empty address list",
525529
Severity: channelz.CtWarning,
526530
})
531+
527532
for _, id := range ids {
528533
defer channelz.RemoveEntry(id)
529534
}
535+
530536
svr := newCZServer()
531537
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
532538
defer cancel()
@@ -555,7 +561,7 @@ func (s) TestGetChannel(t *testing.T) {
555561
}
556562

557563
for i, e := range trace.Events {
558-
if e.GetDescription() != want[i].desc {
564+
if !strings.Contains(e.GetDescription(), want[i].desc) {
559565
t.Fatalf("trace: GetDescription want %#v, got %#v", want[i].desc, e.GetDescription())
560566
}
561567
if e.GetSeverity() != want[i].severity {

‎clientconn.go‎

Lines changed: 50 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -159,23 +159,20 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
159159
}
160160
}()
161161

162-
if channelz.IsOn() {
163-
if cc.dopts.channelzParentID != nil {
164-
cc.channelzID = channelz.RegisterChannel(&channelzChannel{cc}, cc.dopts.channelzParentID, target)
165-
channelz.AddTraceEvent(logger, cc.channelzID, 0, &channelz.TraceEventDesc{
166-
Desc: "Channel Created",
167-
Severity: channelz.CtInfo,
168-
Parent: &channelz.TraceEventDesc{
169-
Desc: fmt.Sprintf("Nested Channel(id:%d) created", cc.channelzID.Int()),
170-
Severity: channelz.CtInfo,
171-
},
172-
})
173-
} else {
174-
cc.channelzID = channelz.RegisterChannel(&channelzChannel{cc}, nil, target)
175-
channelz.Info(logger, cc.channelzID, "Channel Created")
162+
pid := cc.dopts.channelzParentID
163+
cc.channelzID = channelz.RegisterChannel(&channelzChannel{cc}, pid, target)
164+
ted := &channelz.TraceEventDesc{
165+
Desc: fmt.Sprintf("Channel(id:%d) created", cc.channelzID.Int()),
166+
Severity: channelz.CtInfo,
167+
}
168+
if cc.dopts.channelzParentID != nil {
169+
ted.Parent = &channelz.TraceEventDesc{
170+
Desc: fmt.Sprintf("Nested Channel(id:%d) created", cc.channelzID.Int()),
171+
Severity: channelz.CtInfo,
176172
}
177-
cc.csMgr.channelzID = cc.channelzID
178173
}
174+
channelz.AddTraceEvent(logger, cc.channelzID, 1, ted)
175+
cc.csMgr.channelzID = cc.channelzID
179176

180177
if cc.dopts.copts.TransportCredentials == nil && cc.dopts.copts.CredsBundle == nil {
181178
return nil, errNoTransportSecurity
@@ -768,21 +765,21 @@ func (cc *ClientConn) newAddrConn(addrs []resolver.Address, opts balancer.NewSub
768765
cc.mu.Unlock()
769766
return nil, ErrClientConnClosing
770767
}
771-
if channelz.IsOn() {
772-
var err error
773-
ac.channelzID, err = channelz.RegisterSubChannel(ac, cc.channelzID, "")
774-
if err != nil {
775-
return nil, err
776-
}
777-
channelz.AddTraceEvent(logger, ac.channelzID, 0, &channelz.TraceEventDesc{
778-
Desc: "Subchannel Created",
779-
Severity: channelz.CtInfo,
780-
Parent: &channelz.TraceEventDesc{
781-
Desc: fmt.Sprintf("Subchannel(id:%d) created", ac.channelzID.Int()),
782-
Severity: channelz.CtInfo,
783-
},
784-
})
768+
769+
var err error
770+
ac.channelzID, err = channelz.RegisterSubChannel(ac, cc.channelzID, "")
771+
if err != nil {
772+
return nil, err
785773
}
774+
channelz.AddTraceEvent(logger, ac.channelzID, 0, &channelz.TraceEventDesc{
775+
Desc: fmt.Sprintf("Subchannel(id:%d) created", ac.channelzID.Int()),
776+
Severity: channelz.CtInfo,
777+
Parent: &channelz.TraceEventDesc{
778+
Desc: fmt.Sprintf("Subchannel(id:%d) created", ac.channelzID.Int()),
779+
Severity: channelz.CtInfo,
780+
},
781+
})
782+
786783
cc.conns[ac] = struct{}{}
787784
cc.mu.Unlock()
788785
return ac, nil
@@ -1089,22 +1086,22 @@ func (cc *ClientConn) Close() error {
10891086
for ac := range conns {
10901087
ac.tearDown(ErrClientConnClosing)
10911088
}
1092-
if channelz.IsOn() {
1093-
ted := &channelz.TraceEventDesc{
1094-
Desc: "Channel Deleted",
1089+
ted := &channelz.TraceEventDesc{
1090+
Desc: fmt.Sprintf("Channel(id:%d) deleted", cc.channelzID.Int()),
1091+
Severity: channelz.CtInfo,
1092+
}
1093+
if cc.dopts.channelzParentID != nil {
1094+
ted.Parent = &channelz.TraceEventDesc{
1095+
Desc: fmt.Sprintf("Nested channel(id:%d) deleted", cc.channelzID.Int()),
10951096
Severity: channelz.CtInfo,
10961097
}
1097-
if cc.dopts.channelzParentID != nil {
1098-
ted.Parent = &channelz.TraceEventDesc{
1099-
Desc: fmt.Sprintf("Nested channel(id:%d) deleted", cc.channelzID.Int()),
1100-
Severity: channelz.CtInfo,
1101-
}
1102-
}
1103-
channelz.AddTraceEvent(logger, cc.channelzID, 0, ted)
1104-
// TraceEvent needs to be called before RemoveEntry, as TraceEvent may add trace reference to
1105-
// the entity being deleted, and thus prevent it from being deleted right away.
1106-
channelz.RemoveEntry(cc.channelzID)
11071098
}
1099+
channelz.AddTraceEvent(logger, cc.channelzID, 0, ted)
1100+
// TraceEvent needs to be called before RemoveEntry, as TraceEvent may add
1101+
// trace reference to the entity being deleted, and thus prevent it from being
1102+
// deleted right away.
1103+
channelz.RemoveEntry(cc.channelzID)
1104+
11081105
return nil
11091106
}
11101107

@@ -1501,19 +1498,18 @@ func (ac *addrConn) tearDown(err error) {
15011498
curTr.GracefulClose()
15021499
ac.mu.Lock()
15031500
}
1504-
if channelz.IsOn() {
1505-
channelz.AddTraceEvent(logger, ac.channelzID, 0, &channelz.TraceEventDesc{
1506-
Desc: "Subchannel Deleted",
1501+
channelz.AddTraceEvent(logger, ac.channelzID, 0, &channelz.TraceEventDesc{
1502+
Desc: fmt.Sprintf("Subchannel(id:%d) deleted", ac.channelzID.Int()),
1503+
Severity: channelz.CtInfo,
1504+
Parent: &channelz.TraceEventDesc{
1505+
Desc: fmt.Sprintf("Subchannel(id:%d) deleted", ac.channelzID.Int()),
15071506
Severity: channelz.CtInfo,
1508-
Parent: &channelz.TraceEventDesc{
1509-
Desc: fmt.Sprintf("Subchanel(id:%d) deleted", ac.channelzID.Int()),
1510-
Severity: channelz.CtInfo,
1511-
},
1512-
})
1513-
// TraceEvent needs to be called before RemoveEntry, as TraceEvent may add trace reference to
1514-
// the entity being deleted, and thus prevent it from being deleted right away.
1515-
channelz.RemoveEntry(ac.channelzID)
1516-
}
1507+
},
1508+
})
1509+
// TraceEvent needs to be called before RemoveEntry, as TraceEvent may add
1510+
// trace reference to the entity being deleted, and thus prevent it from
1511+
// being deleted right away.
1512+
channelz.RemoveEntry(ac.channelzID)
15171513
ac.mu.Unlock()
15181514
}
15191515

‎internal/channelz/funcs.go‎

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,14 @@ func GetServer(id int64) *ServerMetric {
190190
// (identified by pid). pid == nil means no parent.
191191
//
192192
// Returns a unique channelz identifier assigned to this channel.
193+
//
194+
// If channelz is not turned ON, this function is a no-op.
193195
func RegisterChannel(c Channel, pid *Identifier, ref string) *Identifier {
194-
id := idGen.genID()
196+
if !IsOn() {
197+
return nil
198+
}
195199

200+
id := idGen.genID()
196201
parent := int64(0)
197202
isTopChannel := true
198203
if pid != nil {
@@ -218,7 +223,13 @@ func RegisterChannel(c Channel, pid *Identifier, ref string) *Identifier {
218223
// (identified by pid).
219224
//
220225
// Returns a unique channelz identifier assigned to this subChannel.
226+
//
227+
// If channelz is not turned ON, this function is a no-op.
221228
func RegisterSubChannel(c Channel, pid *Identifier, ref string) (*Identifier, error) {
229+
if !IsOn() {
230+
return nil, nil
231+
}
232+
222233
if pid == nil {
223234
return nil, errors.New("a SubChannel's parent id cannot be nil")
224235
}
@@ -237,7 +248,13 @@ func RegisterSubChannel(c Channel, pid *Identifier, ref string) (*Identifier, er
237248

238249
// RegisterServer registers the given server s in channelz database. It returns
239250
// the unique channelz tracking id assigned to this server.
251+
//
252+
// If channelz is not turned ON, this function is a no-op.
240253
func RegisterServer(s Server, ref string) *Identifier {
254+
if !IsOn() {
255+
return nil
256+
}
257+
241258
id := idGen.genID()
242259
svr := &server{
243260
refName: ref,
@@ -254,7 +271,13 @@ func RegisterServer(s Server, ref string) *Identifier {
254271
// with ref as its reference name, and add it to the child list of its parent
255272
// (identified by pid). It returns the unique channelz tracking id assigned to
256273
// this listen socket.
274+
//
275+
// If channelz is not turned ON, this function is a no-op.
257276
func RegisterListenSocket(s Socket, pid *Identifier, ref string) (*Identifier, error) {
277+
if !IsOn() {
278+
return nil, nil
279+
}
280+
258281
if pid == nil {
259282
return nil, errors.New("a ListenSocket's parent id cannot be 0")
260283
}
@@ -268,7 +291,13 @@ func RegisterListenSocket(s Socket, pid *Identifier, ref string) (*Identifier, e
268291
// with ref as its reference name, and adds it to the child list of its parent
269292
// (identified by pid). It returns the unique channelz tracking id assigned to
270293
// this normal socket.
294+
//
295+
// If channelz is not turned ON, this function is a no-op.
271296
func RegisterNormalSocket(s Socket, pid *Identifier, ref string) (*Identifier, error) {
297+
if !IsOn() {
298+
return nil, nil
299+
}
300+
272301
if pid == nil {
273302
return nil, errors.New("a NormalSocket's parent id cannot be 0")
274303
}
@@ -280,21 +309,30 @@ func RegisterNormalSocket(s Socket, pid *Identifier, ref string) (*Identifier, e
280309

281310
// RemoveEntry removes an entry with unique channelz tracking id to be id from
282311
// channelz database.
312+
//
313+
// If channelz is not turned ON, this function is a no-op.
283314
func RemoveEntry(id *Identifier) {
315+
if !IsOn() {
316+
return
317+
}
284318
db.get().removeEntry(id.Int())
285319
}
286320

287-
// TraceEventDesc is what the caller of AddTraceEvent should provide to describe the event to be added
288-
// to the channel trace.
289-
// The Parent field is optional. It is used for event that will be recorded in the entity's parent
290-
// trace also.
321+
// TraceEventDesc is what the caller of AddTraceEvent should provide to describe
322+
// the event to be added to the channel trace.
323+
//
324+
// The Parent field is optional. It is used for an event that will be recorded
325+
// in the entity's parent trace.
291326
type TraceEventDesc struct {
292327
Desc string
293328
Severity Severity
294329
Parent *TraceEventDesc
295330
}
296331

297-
// AddTraceEvent adds trace related to the entity with specified id, using the provided TraceEventDesc.
332+
// AddTraceEvent adds trace related to the entity with specified id, using the
333+
// provided TraceEventDesc.
334+
//
335+
// If channelz is not turned ON, this will simply log the event descriptions.
298336
func AddTraceEvent(l grpclog.DepthLoggerV2, id *Identifier, depth int, desc *TraceEventDesc) {
299337
for d := desc; d != nil; d = d.Parent {
300338
switch d.Severity {
@@ -309,7 +347,9 @@ func AddTraceEvent(l grpclog.DepthLoggerV2, id *Identifier, depth int, desc *Tra
309347
if getMaxTraceEntry() == 0 {
310348
return
311349
}
312-
db.get().traceEvent(id.Int(), desc)
350+
if IsOn() {
351+
db.get().traceEvent(id.Int(), desc)
352+
}
313353
}
314354

315355
// channelMap is the storage data structure for channelz.

‎resolver_conn_wrapper.go‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@
1919
package grpc
2020

2121
import (
22-
"fmt"
2322
"strings"
2423
"sync"
2524

2625
"google.golang.org/grpc/balancer"
2726
"google.golang.org/grpc/credentials"
2827
"google.golang.org/grpc/internal/channelz"
2928
"google.golang.org/grpc/internal/grpcsync"
29+
"google.golang.org/grpc/internal/pretty"
3030
"google.golang.org/grpc/resolver"
3131
"google.golang.org/grpc/serviceconfig"
3232
)
@@ -97,10 +97,7 @@ func (ccr *ccResolverWrapper) UpdateState(s resolver.State) error {
9797
if ccr.done.HasFired() {
9898
return nil
9999
}
100-
channelz.Infof(logger, ccr.cc.channelzID, "ccResolverWrapper: sending update to cc: %v", s)
101-
if channelz.IsOn() {
102-
ccr.addChannelzTraceEvent(s)
103-
}
100+
ccr.addChannelzTraceEvent(s)
104101
ccr.curState = s
105102
if err := ccr.cc.updateResolverState(ccr.curState, nil); err == balancer.ErrBadResolverState {
106103
return balancer.ErrBadResolverState
@@ -125,10 +122,7 @@ func (ccr *ccResolverWrapper) NewAddress(addrs []resolver.Address) {
125122
if ccr.done.HasFired() {
126123
return
127124
}
128-
channelz.Infof(logger, ccr.cc.channelzID, "ccResolverWrapper: sending new addresses to cc: %v", addrs)
129-
if channelz.IsOn() {
130-
ccr.addChannelzTraceEvent(resolver.State{Addresses: addrs, ServiceConfig: ccr.curState.ServiceConfig})
131-
}
125+
ccr.addChannelzTraceEvent(resolver.State{Addresses: addrs, ServiceConfig: ccr.curState.ServiceConfig})
132126
ccr.curState.Addresses = addrs
133127
ccr.cc.updateResolverState(ccr.curState, nil)
134128
}
@@ -141,7 +135,7 @@ func (ccr *ccResolverWrapper) NewServiceConfig(sc string) {
141135
if ccr.done.HasFired() {
142136
return
143137
}
144-
channelz.Infof(logger, ccr.cc.channelzID, "ccResolverWrapper: got new service config: %v", sc)
138+
channelz.Infof(logger, ccr.cc.channelzID, "ccResolverWrapper: got new service config: %s", sc)
145139
if ccr.cc.dopts.disableServiceConfig {
146140
channelz.Info(logger, ccr.cc.channelzID, "Service config lookups disabled; ignoring config")
147141
return
@@ -151,9 +145,7 @@ func (ccr *ccResolverWrapper) NewServiceConfig(sc string) {
151145
channelz.Warningf(logger, ccr.cc.channelzID, "ccResolverWrapper: error parsing service config: %v", scpr.Err)
152146
return
153147
}
154-
if channelz.IsOn() {
155-
ccr.addChannelzTraceEvent(resolver.State{Addresses: ccr.curState.Addresses, ServiceConfig: scpr})
156-
}
148+
ccr.addChannelzTraceEvent(resolver.State{Addresses: ccr.curState.Addresses, ServiceConfig: scpr})
157149
ccr.curState.ServiceConfig = scpr
158150
ccr.cc.updateResolverState(ccr.curState, nil)
159151
}
@@ -180,8 +172,5 @@ func (ccr *ccResolverWrapper) addChannelzTraceEvent(s resolver.State) {
180172
} else if len(ccr.curState.Addresses) == 0 && len(s.Addresses) > 0 {
181173
updates = append(updates, "resolver returned new addresses")
182174
}
183-
channelz.AddTraceEvent(logger, ccr.cc.channelzID, 0, &channelz.TraceEventDesc{
184-
Desc: fmt.Sprintf("Resolver state updated: %+v (%v)", s, strings.Join(updates, "; ")),
185-
Severity: channelz.CtInfo,
186-
})
175+
channelz.Infof(logger, ccr.cc.channelzID, "Resolver state updated: %s (%v)", pretty.ToJSON(s), strings.Join(updates, "; "))
187176
}

0 commit comments

Comments
 (0)