#19575

Committed 27 Nov 2024 07:37PM UTC coverage: 88.572% (-0.01%) from 88.582%

Build # #19575

Build Type

push

github

Committed by

web-flow

Commit Message

rls: Reduce RLS channel logging

The channel log is shared by many components and is poorly suited to
the noise of per-RPC events. This commit restricts RLS usage of the
logger to no more frequent than cache entry events. This may still be
too frequent, but should substantially improve the signal-to-noise and
we can do further rework as needed.

Many of the log entries were poor because they lacked enough context.
They weren't even clear they were from RLS. The cache entry events now
regularly include the request key in the logs, allowing you to follow
events for specific keys. I would have preferred using the hash code,
but NumberFormat is annoying and toString() may be acceptable given its
convenience.

This commit reverts much of eba699ad. Those logs have not proven to be
helpful as they produce more output than can be reasonably stored.

Run Details

33344 of 37646 relevant lines covered (88.57%)

0.89 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

84.0

/../rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java

/*
 * Copyright 2020 The gRPC Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package io.grpc.rls;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import io.grpc.ChannelLogger;
import io.grpc.ChannelLogger.ChannelLogLevel;
import io.grpc.ConnectivityState;
import io.grpc.LoadBalancer;
import io.grpc.Status;
import javax.annotation.Nullable;

/**
 * Implementation of {@link LoadBalancer} backed by route lookup service.
 */
final class RlsLoadBalancer extends LoadBalancer {

  private final ChannelLogger logger;
  private final Helper helper;
  @VisibleForTesting
  CachingRlsLbClientBuilderProvider cachingRlsLbClientBuilderProvider =
      new DefaultCachingRlsLbClientBuilderProvider();
  @Nullable
  private LbPolicyConfiguration lbPolicyConfiguration;
  @Nullable
  private CachingRlsLbClient routeLookupClient;

  RlsLoadBalancer(Helper helper) {
    this.helper = checkNotNull(helper, "helper");
    logger = helper.getChannelLogger();
    logger.log(ChannelLogLevel.DEBUG, "Rls lb created. Authority: {0}", helper.getAuthority());
  }

  @Override
  public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
    LbPolicyConfiguration lbPolicyConfiguration =
        (LbPolicyConfiguration) resolvedAddresses.getLoadBalancingPolicyConfig();
    checkNotNull(lbPolicyConfiguration, "Missing RLS LB config");
    if (!lbPolicyConfiguration.equals(this.lbPolicyConfiguration)) {
      logger.log(ChannelLogLevel.DEBUG, "A new RLS LB config received: {0}", lbPolicyConfiguration);
      boolean needToConnect = this.lbPolicyConfiguration == null
          || !this.lbPolicyConfiguration.getRouteLookupConfig().lookupService().equals(
          lbPolicyConfiguration.getRouteLookupConfig().lookupService());
      if (needToConnect) {
        logger.log(ChannelLogLevel.DEBUG, "RLS lookup service changed, need to connect");
        if (routeLookupClient != null) {
          routeLookupClient.close();
        }
        routeLookupClient =
            cachingRlsLbClientBuilderProvider
                .get()
                .setHelper(helper)
                .setLbPolicyConfig(lbPolicyConfiguration)
                .setResolvedAddressesFactory(
                    new ChildLbResolvedAddressFactory(
                        resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()))
                .build();
        logger.log(
            ChannelLogLevel.DEBUG, "LbPolicyConfiguration updated to {0}", lbPolicyConfiguration);
      }
      // TODO(creamsoup) allow incremental service config update. for initial use case, it is 
      //  not required.
      this.lbPolicyConfiguration = lbPolicyConfiguration;
    }
    return Status.OK;
  }

  @Override
  public void requestConnection() {
    if (routeLookupClient != null) {
      routeLookupClient.requestConnection();
    }
  }

  @Override
  public void handleNameResolutionError(final Status error) {
    class ErrorPicker extends SubchannelPicker {
      @Override
      public PickResult pickSubchannel(PickSubchannelArgs args) {
        return PickResult.withError(error);
      }

      @Override
      public String toString() {
        return MoreObjects.toStringHelper(this)
            .add("error", error)
            .toString();
      }
    }

    if (routeLookupClient != null) {
      logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient on a name resolution error");
      routeLookupClient.close();
      routeLookupClient = null;
      lbPolicyConfiguration = null;
    }
    helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new ErrorPicker());
  }

  @Override
  public void shutdown() {
    if (routeLookupClient != null) {
      logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient because of RLS LB shutdown");
      routeLookupClient.close();
      routeLookupClient = null;
    }
  }

  /**
   * Provides {@link CachingRlsLbClient.Builder} with default settings. This is useful for
   * testing.
   */
  interface CachingRlsLbClientBuilderProvider {
    CachingRlsLbClient.Builder get();
  }

  static final class DefaultCachingRlsLbClientBuilderProvider
      implements CachingRlsLbClientBuilderProvider {

    @Override
    public CachingRlsLbClient.Builder get() {
      return CachingRlsLbClient.newBuilder().setThrottler(AdaptiveThrottler.builder().build());
    }
  }
}

1	/*
2	* Copyright 2020 The gRPC Authors
3	*
4	* Licensed under the Apache License, Version 2.0 (the "License");
5	* you may not use this file except in compliance with the License.
6	* You may obtain a copy of the License at
7	*
8	* http://www.apache.org/licenses/LICENSE-2.0
9	*
10	* Unless required by applicable law or agreed to in writing, software
11	* distributed under the License is distributed on an "AS IS" BASIS,
12	* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13	* See the License for the specific language governing permissions and
14	* limitations under the License.
15	*/
16
17	package io.grpc.rls;
18
19	import static com.google.common.base.Preconditions.checkNotNull;
20
21	import com.google.common.annotations.VisibleForTesting;
22	import com.google.common.base.MoreObjects;
23	import io.grpc.ChannelLogger;
24	import io.grpc.ChannelLogger.ChannelLogLevel;
25	import io.grpc.ConnectivityState;
26	import io.grpc.LoadBalancer;
27	import io.grpc.Status;
28	import javax.annotation.Nullable;
29
30	/**
31	* Implementation of {@link LoadBalancer} backed by route lookup service.
32	*/
33	final class RlsLoadBalancer extends LoadBalancer {
34
35	private final ChannelLogger logger;
36	private final Helper helper;
37	@VisibleForTesting	1✔
38	CachingRlsLbClientBuilderProvider cachingRlsLbClientBuilderProvider =
39	new DefaultCachingRlsLbClientBuilderProvider();
40	@Nullable
41	private LbPolicyConfiguration lbPolicyConfiguration;
42	@Nullable
43	private CachingRlsLbClient routeLookupClient;
44
45	RlsLoadBalancer(Helper helper) {	1✔
46	this.helper = checkNotNull(helper, "helper");	1✔
47	logger = helper.getChannelLogger();	1✔
48	logger.log(ChannelLogLevel.DEBUG, "Rls lb created. Authority: {0}", helper.getAuthority());	1✔
49	}	1✔
50
51	@Override
52	public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
53	LbPolicyConfiguration lbPolicyConfiguration =	1✔
54	(LbPolicyConfiguration) resolvedAddresses.getLoadBalancingPolicyConfig();	1✔
55	checkNotNull(lbPolicyConfiguration, "Missing RLS LB config");	1✔
56	if (!lbPolicyConfiguration.equals(this.lbPolicyConfiguration)) {	1✔
57	logger.log(ChannelLogLevel.DEBUG, "A new RLS LB config received: {0}", lbPolicyConfiguration);	1✔
58	boolean needToConnect = this.lbPolicyConfiguration == null	1✔
59	\|\| !this.lbPolicyConfiguration.getRouteLookupConfig().lookupService().equals(	1✔
60	lbPolicyConfiguration.getRouteLookupConfig().lookupService());	×
61	if (needToConnect) {	1✔
62	logger.log(ChannelLogLevel.DEBUG, "RLS lookup service changed, need to connect");	1✔
63	if (routeLookupClient != null) {	1✔
64	routeLookupClient.close();	×
65	}
66	routeLookupClient =	1✔
67	cachingRlsLbClientBuilderProvider
68	.get()	1✔
69	.setHelper(helper)	1✔
70	.setLbPolicyConfig(lbPolicyConfiguration)	1✔
71	.setResolvedAddressesFactory(	1✔
72	new ChildLbResolvedAddressFactory(
73	resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()))	1✔
74	.build();	1✔
75	logger.log(	1✔
76	ChannelLogLevel.DEBUG, "LbPolicyConfiguration updated to {0}", lbPolicyConfiguration);
77	}
78	// TODO(creamsoup) allow incremental service config update. for initial use case, it is
79	// not required.
80	this.lbPolicyConfiguration = lbPolicyConfiguration;	1✔
81	}
82	return Status.OK;	1✔
83	}
84
85	@Override
86	public void requestConnection() {
87	if (routeLookupClient != null) {	×
88	routeLookupClient.requestConnection();	×
89	}
90	}	×
91
92	@Override
93	public void handleNameResolutionError(final Status error) {
94	class ErrorPicker extends SubchannelPicker {	1✔
95	@Override
96	public PickResult pickSubchannel(PickSubchannelArgs args) {
97	return PickResult.withError(error);	1✔
98	}
99
100	@Override
101	public String toString() {
102	return MoreObjects.toStringHelper(this)	×
103	.add("error", error)	×
104	.toString();	×
105	}
106	}
107
108	if (routeLookupClient != null) {	1✔
109	logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient on a name resolution error");	1✔
110	routeLookupClient.close();	1✔
111	routeLookupClient = null;	1✔
112	lbPolicyConfiguration = null;	1✔
113	}
114	helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new ErrorPicker());	1✔
115	}	1✔
116
117	@Override
118	public void shutdown() {
119	if (routeLookupClient != null) {	1✔
120	logger.log(ChannelLogLevel.DEBUG, "closing the routeLookupClient because of RLS LB shutdown");	1✔
121	routeLookupClient.close();	1✔
122	routeLookupClient = null;	1✔
123	}
124	}	1✔
125
126	/**
127	* Provides {@link CachingRlsLbClient.Builder} with default settings. This is useful for
128	* testing.
129	*/
130	interface CachingRlsLbClientBuilderProvider {
131	CachingRlsLbClient.Builder get();
132	}
133
134	static final class DefaultCachingRlsLbClientBuilderProvider	1✔
135	implements CachingRlsLbClientBuilderProvider {
136
137	@Override
138	public CachingRlsLbClient.Builder get() {
139	return CachingRlsLbClient.newBuilder().setThrottler(AdaptiveThrottler.builder().build());	1✔
140	}
141	}
142	}

grpc / grpc-java / #19575

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous