Skip to content

Commit 5fa434b

Browse files
committed
grpclog: remove Debugf method to avoid unnecessary evaluation
1 parent 3267089 commit 5fa434b

File tree

6 files changed

+42
-35
lines changed

6 files changed

+42
-35
lines changed

internal/grpclog/prefixLogger.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,24 +61,6 @@ func (pl *PrefixLogger) Errorf(format string, args ...any) {
6161
ErrorDepth(1, fmt.Sprintf(format, args...))
6262
}
6363

64-
// Debugf does info logging at verbose level 2.
65-
func (pl *PrefixLogger) Debugf(format string, args ...any) {
66-
// TODO(6044): Refactor interfaces LoggerV2 and DepthLogger, and maybe
67-
// rewrite PrefixLogger a little to ensure that we don't use the global
68-
// `Logger` here, and instead use the `logger` field.
69-
if !Logger.V(2) {
70-
return
71-
}
72-
if pl != nil {
73-
// Handle nil, so the tests can pass in a nil logger.
74-
format = pl.prefix + format
75-
pl.logger.InfoDepth(1, fmt.Sprintf(format, args...))
76-
return
77-
}
78-
InfoDepth(1, fmt.Sprintf(format, args...))
79-
80-
}
81-
8264
// V reports whether verbosity level l is at least the requested verbose level.
8365
func (pl *PrefixLogger) V(l int) bool {
8466
// TODO(6044): Refactor interfaces LoggerV2 and DepthLogger, and maybe

internal/xds/bootstrap/bootstrap.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,9 @@ func bootstrapConfigFromEnvVariable() ([]byte, error) {
509509
fContent := envconfig.XDSBootstrapFileContent
510510

511511
if fName != "" {
512-
logger.Debugf("Using bootstrap file with name %q", fName)
512+
if logger.V(2) {
513+
logger.Infof("Using bootstrap file with name %q", fName)
514+
}
513515
return bootstrapFileReadFunc(fName)
514516
}
515517

xds/internal/balancer/priority/balancer_priority.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ var (
8383
// Caller must hold b.mu.
8484
func (b *priorityBalancer) syncPriority(childUpdating string) {
8585
if b.inhibitPickerUpdates {
86-
b.logger.Debugf("Skipping update from child policy %q", childUpdating)
86+
if b.logger.V(2) {
87+
b.logger.Infof("Skipping update from child policy %q", childUpdating)
88+
}
8789
return
8890
}
8991
for p, name := range b.priorities {
@@ -99,12 +101,16 @@ func (b *priorityBalancer) syncPriority(childUpdating string) {
99101
(child.state.ConnectivityState == connectivity.Connecting && child.initTimer != nil) ||
100102
p == len(b.priorities)-1 {
101103
if b.childInUse != child.name || child.name == childUpdating {
102-
b.logger.Debugf("childInUse, childUpdating: %q, %q", b.childInUse, child.name)
104+
if b.logger.V(2) {
105+
b.logger.Infof("childInUse, childUpdating: %q, %q", b.childInUse, child.name)
106+
}
103107
// If we switch children or the child in use just updated its
104108
// picker, push the child's picker to the parent.
105109
b.cc.UpdateState(child.state)
106110
}
107-
b.logger.Debugf("Switching to (%q, %v) in syncPriority", child.name, p)
111+
if b.logger.V(2) {
112+
b.logger.Infof("Switching to (%q, %v) in syncPriority", child.name, p)
113+
}
108114
b.switchToChild(child, p)
109115
break
110116
}

xds/internal/balancer/ringhash/ring.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,15 @@ type ringEntry struct {
6767
//
6868
// Must be called with a non-empty subConns map.
6969
func newRing(subConns *resolver.AddressMap, minRingSize, maxRingSize uint64, logger *grpclog.PrefixLogger) *ring {
70-
logger.Debugf("newRing: number of subConns is %d, minRingSize is %d, maxRingSize is %d", subConns.Len(), minRingSize, maxRingSize)
70+
if logger.V(2) {
71+
logger.Infof("newRing: number of subConns is %d, minRingSize is %d, maxRingSize is %d", subConns.Len(), minRingSize, maxRingSize)
72+
}
7173

7274
// https://github.com/envoyproxy/envoy/blob/765c970f06a4c962961a0e03a467e165b276d50f/source/common/upstream/ring_hash_lb.cc#L114
7375
normalizedWeights, minWeight := normalizeWeights(subConns)
74-
logger.Debugf("newRing: normalized subConn weights is %v", normalizedWeights)
76+
if logger.V(2) {
77+
logger.Infof("newRing: normalized subConn weights is %v", normalizedWeights)
78+
}
7579

7680
// Normalized weights for {3,3,4} is {0.3,0.3,0.4}.
7781

@@ -82,7 +86,9 @@ func newRing(subConns *resolver.AddressMap, minRingSize, maxRingSize uint64, log
8286
scale := math.Min(math.Ceil(minWeight*float64(minRingSize))/minWeight, float64(maxRingSize))
8387
ringSize := math.Ceil(scale)
8488
items := make([]*ringEntry, 0, int(ringSize))
85-
logger.Debugf("newRing: creating new ring of size %v", ringSize)
89+
if logger.V(2) {
90+
logger.Infof("newRing: creating new ring of size %v", ringSize)
91+
}
8692

8793
// For each entry, scale*weight nodes are generated in the ring.
8894
//

xds/internal/xdsclient/authority.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,9 @@ func (a *authority) updateResourceStateAndScheduleCallbacks(rType xdsresource.Ty
229229
}
230230
}
231231
// Sync cache.
232-
a.logger.Debugf("Resource type %q with name %q added to cache", rType.TypeName(), name)
232+
if a.logger.V(2) {
233+
a.logger.Infof("Resource type %q with name %q added to cache", rType.TypeName(), name)
234+
}
233235
state.cache = uErr.resource
234236
// Set status to ACK, and clear error state. The metadata might be a
235237
// NACK metadata because some other resources in the same response
@@ -454,7 +456,9 @@ func (a *authority) close() {
454456
}
455457

456458
func (a *authority) watchResource(rType xdsresource.Type, resourceName string, watcher xdsresource.ResourceWatcher) func() {
457-
a.logger.Debugf("New watch for type %q, resource name %q", rType.TypeName(), resourceName)
459+
if a.logger.V(2) {
460+
a.logger.Infof("New watch for type %q, resource name %q", rType.TypeName(), resourceName)
461+
}
458462
a.resourcesMu.Lock()
459463
defer a.resourcesMu.Unlock()
460464

@@ -471,7 +475,9 @@ func (a *authority) watchResource(rType xdsresource.Type, resourceName string, w
471475
// instruct the transport layer to send a DiscoveryRequest for the same.
472476
state := resources[resourceName]
473477
if state == nil {
474-
a.logger.Debugf("First watch for type %q, resource name %q", rType.TypeName(), resourceName)
478+
if a.logger.V(2) {
479+
a.logger.Infof("First watch for type %q, resource name %q", rType.TypeName(), resourceName)
480+
}
475481
state = &resourceState{
476482
watchers: make(map[xdsresource.ResourceWatcher]bool),
477483
md: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusRequested},
@@ -510,7 +516,9 @@ func (a *authority) watchResource(rType xdsresource.Type, resourceName string, w
510516
// There are no more watchers for this resource, delete the state
511517
// associated with it, and instruct the transport to send a request
512518
// which does not include this resource name.
513-
a.logger.Debugf("Removing last watch for type %q, resource name %q", rType.TypeName(), resourceName)
519+
if a.logger.V(2) {
520+
a.logger.Infof("Removing last watch for type %q, resource name %q", rType.TypeName(), resourceName)
521+
}
514522
delete(resources, resourceName)
515523
a.sendDiscoveryRequestLocked(rType, resources)
516524
}

xds/internal/xdsclient/transport/transport.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ import (
4848

4949
// Any per-RPC level logs which print complete request or response messages
5050
// should be gated at this verbosity level. Other per-RPC level logs which print
51-
// terse output should be at `INFO` and verbosity 2, which corresponds to using
52-
// the `Debugf` method on the logger.
51+
// terse output should be at `INFO` and verbosity 2.
5352
const perRPCVerbosityLevel = 9
5453

5554
type adsStream = v3adsgrpc.AggregatedDiscoveryService_StreamAggregatedResourcesClient
@@ -298,7 +297,9 @@ func (t *Transport) sendAggregatedDiscoveryServiceRequest(stream adsStream, send
298297
if t.logger.V(perRPCVerbosityLevel) {
299298
t.logger.Infof("ADS request sent: %v", pretty.ToJSON(req))
300299
} else {
301-
t.logger.Debugf("ADS request sent for type %q, resources: %v, version %q, nonce %q", resourceURL, resourceNames, version, nonce)
300+
if t.logger.V(2) {
301+
t.logger.Infof("ADS request sent for type %q, resources: %v, version %q, nonce %q", resourceURL, resourceNames, version, nonce)
302+
}
302303
}
303304
t.onSendHandler(&ResourceSendInfo{URL: resourceURL, ResourceNames: resourceNames})
304305
return nil
@@ -311,8 +312,8 @@ func (t *Transport) recvAggregatedDiscoveryServiceResponse(stream adsStream) (re
311312
}
312313
if t.logger.V(perRPCVerbosityLevel) {
313314
t.logger.Infof("ADS response received: %v", pretty.ToJSON(resp))
314-
} else {
315-
t.logger.Debugf("ADS response received for type %q, version %q, nonce %q", resp.GetTypeUrl(), resp.GetVersionInfo(), resp.GetNonce())
315+
} else if t.logger.V(2) {
316+
t.logger.Infof("ADS response received for type %q, version %q, nonce %q", resp.GetTypeUrl(), resp.GetVersionInfo(), resp.GetNonce())
316317
}
317318
return resp.GetResources(), resp.GetTypeUrl(), resp.GetVersionInfo(), resp.GetNonce(), nil
318319
}
@@ -512,7 +513,9 @@ func (t *Transport) recv(stream adsStream) bool {
512513
stream: stream,
513514
version: rVersion,
514515
})
515-
t.logger.Debugf("Sending ACK for resource type: %q, version: %q, nonce: %q", url, rVersion, nonce)
516+
if t.logger.V(2) {
517+
t.logger.Infof("Sending ACK for resource type: %q, version: %q, nonce: %q", url, rVersion, nonce)
518+
}
516519
}
517520
}
518521

0 commit comments

Comments
 (0)