Skip to content

Commit a1d1932

Browse files
authoredMay 1, 2024
rls: Add the target label to RLS counter metrics (#11142)
1 parent 35a171b commit a1d1932

File tree

3 files changed

+32
-17
lines changed

3 files changed

+32
-17
lines changed
 

‎rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java

+15-15
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import com.google.common.base.MoreObjects;
2525
import com.google.common.base.MoreObjects.ToStringHelper;
2626
import com.google.common.base.Ticker;
27-
import com.google.common.collect.Lists;
2827
import com.google.common.util.concurrent.Futures;
2928
import com.google.common.util.concurrent.ListenableFuture;
3029
import com.google.common.util.concurrent.MoreExecutors;
@@ -61,6 +60,8 @@
6160
import io.grpc.util.ForwardingLoadBalancerHelper;
6261
import java.net.URI;
6362
import java.net.URISyntaxException;
63+
import java.util.Arrays;
64+
import java.util.Collections;
6465
import java.util.HashMap;
6566
import java.util.List;
6667
import java.util.Map;
@@ -127,16 +128,16 @@ final class CachingRlsLbClient {
127128
= MetricInstrumentRegistry.getDefaultRegistry();
128129
DEFAULT_TARGET_PICKS_COUNTER = metricInstrumentRegistry.registerLongCounter(
129130
"grpc.lb.rls.default_target_picks", "Number of LB picks sent to the default target", "pick",
130-
Lists.newArrayList("grpc.target", "grpc.lb.rls.server_target",
131-
"grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Lists.newArrayList(), true);
131+
Arrays.asList("grpc.target", "grpc.lb.rls.server_target",
132+
"grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Collections.emptyList(), true);
132133
TARGET_PICKS_COUNTER = metricInstrumentRegistry.registerLongCounter("grpc.lb.rls.target_picks",
133134
"Number of LB picks sent to each RLS target", "pick",
134-
Lists.newArrayList("grpc.target", "grpc.lb.rls.server_target",
135-
"grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Lists.newArrayList(), true);
135+
Arrays.asList("grpc.target", "grpc.lb.rls.server_target",
136+
"grpc.lb.rls.data_plane_target", "grpc.lb.pick_result"), Collections.emptyList(), true);
136137
FAILED_PICKS_COUNTER = metricInstrumentRegistry.registerLongCounter("grpc.lb.rls.failed_picks",
137138
"Number of LB picks failed due to either a failed RLS request or the RLS channel being "
138-
+ "throttled", "pick", Lists.newArrayList("grpc.target", "grpc.lb.rls.server_target"),
139-
Lists.newArrayList(), true);
139+
+ "throttled", "pick", Arrays.asList("grpc.target", "grpc.lb.rls.server_target"),
140+
Collections.emptyList(), true);
140141
}
141142

142143
private CachingRlsLbClient(Builder builder) {
@@ -968,11 +969,11 @@ public PickResult pickSubchannel(PickSubchannelArgs args) {
968969
// Happy path
969970
logger.log(ChannelLogLevel.DEBUG, "Returning PickResult");
970971
PickResult pickResult = picker.pickSubchannel(args);
971-
// TODO: include the "grpc.target" label once target is available here.
972972
if (pickResult.hasResult()) {
973973
helper.getMetricRecorder().addLongCounter(TARGET_PICKS_COUNTER, 1,
974-
Lists.newArrayList("", lookupService, childPolicyWrapper.getTarget(),
975-
determineMetricsPickResult(pickResult)), Lists.newArrayList());
974+
Arrays.asList(helper.getChannelTarget(), lookupService,
975+
childPolicyWrapper.getTarget(), determineMetricsPickResult(pickResult)),
976+
Collections.emptyList());
976977
}
977978
return pickResult;
978979
} else if (response.hasError()) {
@@ -982,9 +983,8 @@ public PickResult pickSubchannel(PickSubchannelArgs args) {
982983
return useFallback(args);
983984
}
984985
logger.log(ChannelLogLevel.DEBUG, "No RLS fallback, returning PickResult with an error");
985-
// TODO: include the "grpc.target" label once target is available here.
986986
helper.getMetricRecorder().addLongCounter(FAILED_PICKS_COUNTER, 1,
987-
Lists.newArrayList("", lookupService), Lists.newArrayList());
987+
Arrays.asList(helper.getChannelTarget(), lookupService), Collections.emptyList());
988988
return PickResult.withError(
989989
convertRlsServerStatus(response.getStatus(),
990990
lbPolicyConfig.getRouteLookupConfig().lookupService()));
@@ -1007,10 +1007,10 @@ private PickResult useFallback(PickSubchannelArgs args) {
10071007
}
10081008
PickResult pickResult = picker.pickSubchannel(args);
10091009
if (pickResult.hasResult()) {
1010-
// TODO: include the grpc.target label once target is available here.
10111010
helper.getMetricRecorder().addLongCounter(DEFAULT_TARGET_PICKS_COUNTER, 1,
1012-
Lists.newArrayList("", lookupService, fallbackChildPolicyWrapper.getTarget(),
1013-
determineMetricsPickResult(pickResult)), Lists.newArrayList());
1011+
Arrays.asList(helper.getChannelTarget(), lookupService,
1012+
fallbackChildPolicyWrapper.getTarget(), determineMetricsPickResult(pickResult)),
1013+
Collections.emptyList());
10141014
}
10151015
return pickResult;
10161016
}

‎rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java

+5
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,11 @@ public ChannelLogger getChannelLogger() {
900900
public MetricRecorder getMetricRecorder() {
901901
return mockMetricRecorder;
902902
}
903+
904+
@Override
905+
public String getChannelTarget() {
906+
return "channelTarget";
907+
}
903908
}
904909

905910
private static final class FakeThrottler implements Throttler {

‎rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java

+12-2
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ public void uncaughtException(Thread t, Throwable e) {
124124
private final FakeRlsServerImpl fakeRlsServerImpl = new FakeRlsServerImpl();
125125
private final Deque<FakeSubchannel> subchannels = new LinkedList<>();
126126
private final FakeThrottler fakeThrottler = new FakeThrottler();
127+
private final String channelTarget = "channelTarget";
127128
@Mock
128129
private Marshaller<Object> mockMarshaller;
129130
@Captor
@@ -296,6 +297,7 @@ public void lb_working_withoutDefaultTarget_noRlsResponse() throws Exception {
296297
PickResult res = picker.pickSubchannel(searchSubchannelArgs); // create subchannel
297298
assertThat(res.getStatus().getCode()).isEqualTo(Code.UNAVAILABLE);
298299
inOrder.verify(helper).getMetricRecorder();
300+
inOrder.verify(helper).getChannelTarget();
299301
inOrder.verifyNoMoreInteractions();
300302
verifyFailedPicksCounterAdd(1, 1);
301303
verifyNoMoreInteractions(mockMetricRecorder);
@@ -319,6 +321,7 @@ public void lb_working_withDefaultTarget_noRlsResponse() throws Exception {
319321
assertThat(fallbackSubchannel).isNotNull();
320322
assertThat(subchannelIsReady(fallbackSubchannel)).isTrue();
321323
inOrder.verify(helper).getMetricRecorder();
324+
inOrder.verify(helper).getChannelTarget();
322325
inOrder.verifyNoMoreInteractions();
323326
verifyLongCounterAdd("grpc.lb.rls.default_target_picks", 1, 1, "defaultTarget", "complete");
324327
verifyNoMoreInteractions(mockMetricRecorder);
@@ -393,6 +396,7 @@ public void lb_working_withoutDefaultTarget() throws Exception {
393396
FakeSubchannel searchSubchannel =
394397
(FakeSubchannel) markReadyAndGetPickResult(inOrder, searchSubchannelArgs).getSubchannel();
395398
inOrder.verify(helper).getMetricRecorder();
399+
inOrder.verify(helper).getChannelTarget();
396400
inOrder.verifyNoMoreInteractions();
397401
assertThat(subchannelIsReady(searchSubchannel)).isTrue();
398402
assertThat(subchannels.getLast()).isSameInstanceAs(searchSubchannel);
@@ -468,6 +472,7 @@ public void lb_nameResolutionFailed() throws Exception {
468472
verifyLongCounterAdd("grpc.lb.rls.target_picks", 1, 1, "wilderness", "complete");
469473

470474
inOrder.verify(helper).getMetricRecorder();
475+
inOrder.verify(helper).getChannelTarget();
471476
inOrder.verifyNoMoreInteractions();
472477

473478
rlsLb.handleNameResolutionError(Status.UNAVAILABLE);
@@ -559,7 +564,7 @@ public boolean matches(LongCounterMetricInstrument longCounterInstrument) {
559564
return longCounterInstrument.getName().equals(name);
560565
}
561566
}), eq(value),
562-
eq(Lists.newArrayList("", "localhost:8972", dataPlaneTargetLabel, pickResult)),
567+
eq(Lists.newArrayList(channelTarget, "localhost:8972", dataPlaneTargetLabel, pickResult)),
563568
eq(Lists.newArrayList()));
564569
}
565570

@@ -573,7 +578,7 @@ public boolean matches(LongCounterMetricInstrument longCounterInstrument) {
573578
return longCounterInstrument.getName().equals("grpc.lb.rls.failed_picks");
574579
}
575580
}), eq(value),
576-
eq(Lists.newArrayList("", "localhost:8972")),
581+
eq(Lists.newArrayList(channelTarget, "localhost:8972")),
577582
eq(Lists.newArrayList()));
578583
}
579584

@@ -675,6 +680,11 @@ public ChannelLogger getChannelLogger() {
675680
public MetricRecorder getMetricRecorder() {
676681
return mockMetricRecorder;
677682
}
683+
684+
@Override
685+
public String getChannelTarget() {
686+
return channelTarget;
687+
}
678688
}
679689

680690
private static final class FakeRlsServerImpl

0 commit comments

Comments
 (0)
Please sign in to comment.