rls: Add the target label to RLS counter metrics (#11142)

3 files changed, 32 insertions, 17 deletions

diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java
index 54c4c411f..5263975b4 100644
--- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java
+++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java
@@ -24,7 +24,6 @@ import com.google.common.base.Converter;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.MoreObjects.ToStringHelper;
 import com.google.common.base.Ticker;
-import com.google.common.collect.Lists;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
@@ -61,6 +60,8 @@ import io.grpc.stub.StreamObserver;
 import io.grpc.util.ForwardingLoadBalancerHelper;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -127,16 +128,16 @@ final class CachingRlsLbClient {
         = MetricInstrumentRegistry.getDefaultRegistry();
     DEFAULT_TARGET_PICKS_COUNTER = metricInstrumentRegistry.registerLongCounter(
         "grpc.lb.rls.default_target_picks", "Number of LB picks sent to the default target", "pick",
-        Lists.newArrayList("grpc.target", "grpc.lb.rls.server_target",
-            "grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Lists.newArrayList(), true);
+        Arrays.asList("grpc.target", "grpc.lb.rls.server_target",
+            "grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Collections.emptyList(), true);
     TARGET_PICKS_COUNTER = metricInstrumentRegistry.registerLongCounter("grpc.lb.rls.target_picks",
         "Number of LB picks sent to each RLS target", "pick",
-        Lists.newArrayList("grpc.target", "grpc.lb.rls.server_target",
-            "grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Lists.newArrayList(), true);
+        Arrays.asList("grpc.target", "grpc.lb.rls.server_target",
+            "grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Collections.emptyList(), true);
     FAILED_PICKS_COUNTER = metricInstrumentRegistry.registerLongCounter("grpc.lb.rls.failed_picks",
         "Number of LB picks failed due to either a failed RLS request or the RLS channel being "
-            + "throttled", "pick", Lists.newArrayList("grpc.target", "grpc.lb.rls.server_target"),
-        Lists.newArrayList(), true);
+            + "throttled", "pick", Arrays.asList("grpc.target", "grpc.lb.rls.server_target"),
+        Collections.emptyList(), true);
   }
 
   private CachingRlsLbClient(Builder builder) {
@@ -968,11 +969,11 @@ final class CachingRlsLbClient {
         // Happy path
         logger.log(ChannelLogLevel.DEBUG, "Returning PickResult");
         PickResult pickResult = picker.pickSubchannel(args);
-        // TODO: include the "grpc.target" label once target is available here.
         if (pickResult.hasResult()) {
           helper.getMetricRecorder().addLongCounter(TARGET_PICKS_COUNTER, 1,
-              Lists.newArrayList("", lookupService, childPolicyWrapper.getTarget(),
-                  determineMetricsPickResult(pickResult)), Lists.newArrayList());
+              Arrays.asList(helper.getChannelTarget(), lookupService,
+                  childPolicyWrapper.getTarget(), determineMetricsPickResult(pickResult)),
+              Collections.emptyList());
         }
         return pickResult;
       } else if (response.hasError()) {
@@ -982,9 +983,8 @@ final class CachingRlsLbClient {
           return useFallback(args);
         }
         logger.log(ChannelLogLevel.DEBUG, "No RLS fallback, returning PickResult with an error");
-        // TODO: include the "grpc.target" label once target is available here.
         helper.getMetricRecorder().addLongCounter(FAILED_PICKS_COUNTER, 1,
-            Lists.newArrayList("", lookupService), Lists.newArrayList());
+            Arrays.asList(helper.getChannelTarget(), lookupService), Collections.emptyList());
         return PickResult.withError(
             convertRlsServerStatus(response.getStatus(),
                 lbPolicyConfig.getRouteLookupConfig().lookupService()));
@@ -1007,10 +1007,10 @@ final class CachingRlsLbClient {
       }
       PickResult pickResult = picker.pickSubchannel(args);
       if (pickResult.hasResult()) {
-        // TODO: include the grpc.target label once target is available here.
         helper.getMetricRecorder().addLongCounter(DEFAULT_TARGET_PICKS_COUNTER, 1,
-            Lists.newArrayList("", lookupService, fallbackChildPolicyWrapper.getTarget(),
-                determineMetricsPickResult(pickResult)), Lists.newArrayList());
+            Arrays.asList(helper.getChannelTarget(), lookupService,
+                fallbackChildPolicyWrapper.getTarget(), determineMetricsPickResult(pickResult)),
+            Collections.emptyList());
       }
       return pickResult;
     }
diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java
index 362e961fb..9cef95c2a 100644
--- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java
+++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java
@@ -900,6 +900,11 @@ public class CachingRlsLbClientTest {
     public MetricRecorder getMetricRecorder() {
       return mockMetricRecorder;
     }
+
+    @Override
+    public String getChannelTarget() {
+      return "channelTarget";
+    }
   }
 
   private static final class FakeThrottler implements Throttler {
diff --git a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java
index 2137dfeb3..fd29c871a 100644
--- a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java
+++ b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java
@@ -124,6 +124,7 @@ public class RlsLoadBalancerTest {
   private final FakeRlsServerImpl fakeRlsServerImpl = new FakeRlsServerImpl();
   private final Deque<FakeSubchannel> subchannels = new LinkedList<>();
   private final FakeThrottler fakeThrottler = new FakeThrottler();
+  private final String channelTarget = "channelTarget";
   @Mock
   private Marshaller<Object> mockMarshaller;
   @Captor
@@ -296,6 +297,7 @@ public class RlsLoadBalancerTest {
     PickResult res = picker.pickSubchannel(searchSubchannelArgs); // create subchannel
     assertThat(res.getStatus().getCode()).isEqualTo(Code.UNAVAILABLE);
     inOrder.verify(helper).getMetricRecorder();
+    inOrder.verify(helper).getChannelTarget();
     inOrder.verifyNoMoreInteractions();
     verifyFailedPicksCounterAdd(1, 1);
     verifyNoMoreInteractions(mockMetricRecorder);
@@ -319,6 +321,7 @@ public class RlsLoadBalancerTest {
     assertThat(fallbackSubchannel).isNotNull();
     assertThat(subchannelIsReady(fallbackSubchannel)).isTrue();
     inOrder.verify(helper).getMetricRecorder();
+    inOrder.verify(helper).getChannelTarget();
     inOrder.verifyNoMoreInteractions();
     verifyLongCounterAdd("grpc.lb.rls.default_target_picks", 1, 1, "defaultTarget", "complete");
     verifyNoMoreInteractions(mockMetricRecorder);
@@ -393,6 +396,7 @@ public class RlsLoadBalancerTest {
     FakeSubchannel searchSubchannel =
         (FakeSubchannel) markReadyAndGetPickResult(inOrder, searchSubchannelArgs).getSubchannel();
     inOrder.verify(helper).getMetricRecorder();
+    inOrder.verify(helper).getChannelTarget();
     inOrder.verifyNoMoreInteractions();
     assertThat(subchannelIsReady(searchSubchannel)).isTrue();
     assertThat(subchannels.getLast()).isSameInstanceAs(searchSubchannel);
@@ -468,6 +472,7 @@ public class RlsLoadBalancerTest {
     verifyLongCounterAdd("grpc.lb.rls.target_picks", 1, 1, "wilderness", "complete");
 
     inOrder.verify(helper).getMetricRecorder();
+    inOrder.verify(helper).getChannelTarget();
     inOrder.verifyNoMoreInteractions();
 
     rlsLb.handleNameResolutionError(Status.UNAVAILABLE);
@@ -559,7 +564,7 @@ public class RlsLoadBalancerTest {
             return longCounterInstrument.getName().equals(name);
           }
         }), eq(value),
-        eq(Lists.newArrayList("", "localhost:8972", dataPlaneTargetLabel, pickResult)),
+        eq(Lists.newArrayList(channelTarget, "localhost:8972", dataPlaneTargetLabel, pickResult)),
         eq(Lists.newArrayList()));
   }
 
@@ -573,7 +578,7 @@ public class RlsLoadBalancerTest {
             return longCounterInstrument.getName().equals("grpc.lb.rls.failed_picks");
           }
         }), eq(value),
-        eq(Lists.newArrayList("", "localhost:8972")),
+        eq(Lists.newArrayList(channelTarget, "localhost:8972")),
         eq(Lists.newArrayList()));
   }
 
@@ -675,6 +680,11 @@ public class RlsLoadBalancerTest {
     public MetricRecorder getMetricRecorder() {
       return mockMetricRecorder;
     }
+
+    @Override
+    public String getChannelTarget() {
+      return channelTarget;
+    }
   }
 
   private static final class FakeRlsServerImpl