From c7d65021ba8a8ddc4993f9ccee6053e6ed9ae065 Mon Sep 17 00:00:00 2001 From: Saurav Date: Wed, 22 Apr 2026 19:02:42 +0000 Subject: [PATCH] xds: pre-parse custom metric names in WRR load balancer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce ParsedMetricName in MetricReportUtils to pre-parse configured custom metric names into Enums and key Strings on config initialization in WeightedRoundRobinLoadBalancerConfig, avoiding String parsing operations in the data path. This has been done by a combination of a few things - Streams -> loop - OptionalDouble -> double - Pre parsing instead of hot path substring OrcaReportListener now utilizes pre-parsed ParsedMetricName objects during getCustomMetricUtilization to prevent OptionalDouble heap allocations on the hot path. Updated test coverage in MetricReportUtilsTest and WeightedRoundRobinLoadBalancerTest. # JMH Benchmark Report: MetricReportUtils Optimization We performed a benchmark comparison of four different custom metric resolution implementations in the Weighted Round Robin (WRR) load balancer. ## Benchmark Results | Benchmark Variant | Average Latency | Normalized Heap Allocations | Speedup | | :------------------------------------ | :-------------- | :-------------------------- | :-------- | | **Baseline (`String` + Streams)** | 174.46 ns/op | 704.00 B/op | 1x | | **`ParsedMetricName` + Streams** | 148.95 ns/op | 608.00 B/op | ~1.1x | | **`String` + Loop** | 81.61 ns/op | 240.00 B/op | ~2.1x | | **`ParsedMetricName` + Loop** | 52.92 ns/op | 144.00 B/op | ~3.2x | | **`ParsedMetricName` + Unboxed Loop** | **43.76 ns/op** | **≈ 0.00 B/op** | **~4.0x** | --- --- .../xds/WeightedRoundRobinLoadBalancer.java | 80 +++++++----- .../grpc/xds/internal/MetricReportUtils.java | 118 +++++++++++++----- ...tedRoundRobinLoadBalancerProviderTest.java | 10 +- .../WeightedRoundRobinLoadBalancerTest.java | 68 +++++----- .../xds/internal/MetricReportUtilsTest.java | 62 ++++----- 5 files changed, 212 insertions(+), 126 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java b/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java index a8b7e120cca..979e11916ca 100644 --- a/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/WeightedRoundRobinLoadBalancer.java @@ -41,6 +41,7 @@ import io.grpc.util.ForwardingSubchannel; import io.grpc.util.MultiChildLoadBalancer; import io.grpc.xds.internal.MetricReportUtils; +import io.grpc.xds.internal.MetricReportUtils.ParsedMetricName; import io.grpc.xds.orca.OrcaOobUtil; import io.grpc.xds.orca.OrcaOobUtil.OrcaOobReportListener; import io.grpc.xds.orca.OrcaPerRequestUtil; @@ -50,7 +51,6 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; -import java.util.OptionalDouble; import java.util.Random; import java.util.Set; import java.util.concurrent.ScheduledExecutorService; @@ -239,7 +239,7 @@ protected void updateOverallBalancingState() { private SubchannelPicker createReadyPicker(Collection activeList) { WeightedRoundRobinPicker picker = new WeightedRoundRobinPicker(ImmutableList.copyOf(activeList), config.enableOobLoadReport, config.errorUtilizationPenalty, sequence, - config.metricNamesForComputingUtilization); + config.parsedMetricNamesForComputingUtilization); updateWeight(picker); return picker; } @@ -329,15 +329,15 @@ public void addSubchannel(WrrSubchannel wrrSubchannel) { } public OrcaReportListener getOrCreateOrcaListener(float errorUtilizationPenalty, - ImmutableList metricNamesForComputingUtilization) { + ImmutableList parsedMetricNamesForComputingUtilization) { if (orcaReportListener != null && orcaReportListener.errorUtilizationPenalty == errorUtilizationPenalty - && orcaReportListener.metricNamesForComputingUtilization - .equals(metricNamesForComputingUtilization)) { + && orcaReportListener.parsedMetricNamesForComputingUtilization + .equals(parsedMetricNamesForComputingUtilization)) { return orcaReportListener; } orcaReportListener = - new OrcaReportListener(errorUtilizationPenalty, metricNamesForComputingUtilization); + new OrcaReportListener(errorUtilizationPenalty, parsedMetricNamesForComputingUtilization); return orcaReportListener; } @@ -362,17 +362,17 @@ public void updateBalancingState(ConnectivityState newState, SubchannelPicker ne final class OrcaReportListener implements OrcaPerRequestReportListener, OrcaOobReportListener { private final float errorUtilizationPenalty; - private final ImmutableList metricNamesForComputingUtilization; + private final ImmutableList parsedMetricNamesForComputingUtilization; OrcaReportListener(float errorUtilizationPenalty, - ImmutableList metricNamesForComputingUtilization) { + ImmutableList parsedMetricNamesForComputingUtilization) { this.errorUtilizationPenalty = errorUtilizationPenalty; - this.metricNamesForComputingUtilization = metricNamesForComputingUtilization; + this.parsedMetricNamesForComputingUtilization = parsedMetricNamesForComputingUtilization; } @Override public void onLoadReport(MetricReport report) { - double utilization = getUtilization(report, metricNamesForComputingUtilization); + double utilization = getUtilization(report); double newWeight = 0; if (utilization > 0 && report.getQps() > 0) { @@ -398,10 +398,10 @@ public void onLoadReport(MetricReport report) { * if application utilization is > 0, it is returned. If neither are present, the CPU * utilization is returned. */ - private double getUtilization(MetricReport report, ImmutableList metricNames) { - OptionalDouble customUtil = getCustomMetricUtilization(report, metricNames); - if (customUtil.isPresent()) { - return customUtil.getAsDouble(); + private double getUtilization(MetricReport report) { + double customUtil = getCustomMetricUtilization(report); + if (customUtil >= 0) { + return customUtil; } double appUtil = report.getApplicationUtilization(); if (appUtil > 0) { @@ -412,20 +412,23 @@ private double getUtilization(MetricReport report, ImmutableList metricN /** * Returns the maximum utilization value among the specified metric names. - * Returns OptionalDouble.empty() if NONE of the specified metrics are present in the report, + * Returns -1 if NONE of the specified metrics are present in the report, * or if all present metrics are NaN. - * Returns OptionalDouble.of(maxUtil) if at least one non-NaN metric is present. */ - private OptionalDouble getCustomMetricUtilization(MetricReport report, - ImmutableList metricNames) { - return metricNames.stream() - .map(name -> MetricReportUtils.getMetric(report, name)) - .filter(OptionalDouble::isPresent) - .mapToDouble(OptionalDouble::getAsDouble) - .filter(d -> !Double.isNaN(d) && d > 0) - .max(); + private double getCustomMetricUtilization(MetricReport report) { + double max = -1.0; + for (int i = 0; i < parsedMetricNamesForComputingUtilization.size(); i++) { + double d = MetricReportUtils.getMetricValue(report, + parsedMetricNamesForComputingUtilization.get(i)); + if (!Double.isNaN(d) && d > 0 && d > max) { + max = d; + } + } + return max; } + } + } private final class UpdateWeightTask implements Runnable { @@ -446,7 +449,7 @@ private void createAndApplyOrcaListeners() { if (config.enableOobLoadReport) { OrcaOobUtil.setListener(weightedSubchannel, wChild.getOrCreateOrcaListener(config.errorUtilizationPenalty, - config.metricNamesForComputingUtilization), + config.parsedMetricNamesForComputingUtilization), OrcaOobUtil.OrcaReportingConfig.newBuilder() .setReportInterval(config.oobReportingPeriodNanos, TimeUnit.NANOSECONDS).build()); } else { @@ -516,7 +519,7 @@ static final class WeightedRoundRobinPicker extends SubchannelPicker { WeightedRoundRobinPicker(List children, boolean enableOobLoadReport, float errorUtilizationPenalty, AtomicInteger sequence, - ImmutableList metricNamesForComputingUtilization) { + ImmutableList parsedMetricNamesForComputingUtilization) { checkNotNull(children, "children"); Preconditions.checkArgument(!children.isEmpty(), "empty child list"); this.children = children; @@ -526,7 +529,7 @@ static final class WeightedRoundRobinPicker extends SubchannelPicker { WeightedChildLbState wChild = (WeightedChildLbState) child; pickers.add(wChild.getCurrentPicker()); reportListeners.add(wChild.getOrCreateOrcaListener(errorUtilizationPenalty, - metricNamesForComputingUtilization)); + parsedMetricNamesForComputingUtilization)); } this.pickers = pickers; this.reportListeners = reportListeners; @@ -767,7 +770,7 @@ static final class WeightedRoundRobinLoadBalancerConfig { final long oobReportingPeriodNanos; final long weightUpdatePeriodNanos; final float errorUtilizationPenalty; - final ImmutableList metricNamesForComputingUtilization; + final ImmutableList parsedMetricNamesForComputingUtilization; public static Builder newBuilder() { return new Builder(); @@ -783,7 +786,20 @@ private WeightedRoundRobinLoadBalancerConfig(long blackoutPeriodNanos, this.oobReportingPeriodNanos = oobReportingPeriodNanos; this.weightUpdatePeriodNanos = weightUpdatePeriodNanos; this.errorUtilizationPenalty = errorUtilizationPenalty; - this.metricNamesForComputingUtilization = metricNamesForComputingUtilization; + + ImmutableList.Builder builder = ImmutableList.builder(); + if (metricNamesForComputingUtilization != null) { + for (int i = 0; i < metricNamesForComputingUtilization.size(); i++) { + String metricName = metricNamesForComputingUtilization.get(i); + ParsedMetricName parsed = MetricReportUtils.ParsedMetricName.parse(metricName); + if (parsed.getMetricType() != MetricReportUtils.MetricType.INVALID) { + builder.add(parsed); + } else { + log.log(Level.FINE, "Invalid custom metric name configured and ignored: " + metricName); + } + } + } + this.parsedMetricNamesForComputingUtilization = builder.build(); } @Override @@ -799,15 +815,15 @@ public boolean equals(Object o) { && this.weightUpdatePeriodNanos == that.weightUpdatePeriodNanos // Float.compare considers NaNs equal && Float.compare(this.errorUtilizationPenalty, that.errorUtilizationPenalty) == 0 - && Objects.equals(this.metricNamesForComputingUtilization, - that.metricNamesForComputingUtilization); + && Objects.equals(this.parsedMetricNamesForComputingUtilization, + that.parsedMetricNamesForComputingUtilization); } @Override public int hashCode() { return Objects.hash(blackoutPeriodNanos, weightExpirationPeriodNanos, enableOobLoadReport, oobReportingPeriodNanos, weightUpdatePeriodNanos, errorUtilizationPenalty, - metricNamesForComputingUtilization); + parsedMetricNamesForComputingUtilization); } static final class Builder { diff --git a/xds/src/main/java/io/grpc/xds/internal/MetricReportUtils.java b/xds/src/main/java/io/grpc/xds/internal/MetricReportUtils.java index 7da9a3ab6d9..dda40d3a4f7 100644 --- a/xds/src/main/java/io/grpc/xds/internal/MetricReportUtils.java +++ b/xds/src/main/java/io/grpc/xds/internal/MetricReportUtils.java @@ -16,9 +16,10 @@ package io.grpc.xds.internal; +import com.google.auto.value.AutoValue; import io.grpc.services.MetricReport; -import java.util.Map; -import java.util.OptionalDouble; +import java.util.Optional; + /** * Utilities for parsing and resolving metrics from {@link MetricReport}. @@ -27,41 +28,92 @@ public final class MetricReportUtils { private MetricReportUtils() {} + public enum MetricType { + CPU_UTILIZATION, + APPLICATION_UTILIZATION, + MEMORY_UTILIZATION, + UTILIZATION, + NAMED_METRICS, + INVALID + } + + @AutoValue + public abstract static class ParsedMetricName { + public abstract MetricType getMetricType(); + + public abstract Optional getKey(); + + public static ParsedMetricName create(MetricType metricType, Optional key) { + return new AutoValue_MetricReportUtils_ParsedMetricName(metricType, key); + } + + /** + * Pre-parses a custom metric name into a {@link ParsedMetricName}. + * + * @param name The custom metric name to parse. + * @return The parsed metric name. + */ + public static ParsedMetricName parse(String name) { + if (name.equals("cpu_utilization")) { + return create(MetricType.CPU_UTILIZATION, Optional.empty()); + } + if (name.equals("application_utilization")) { + return create(MetricType.APPLICATION_UTILIZATION, Optional.empty()); + } + if (name.equals("mem_utilization")) { + return create(MetricType.MEMORY_UTILIZATION, Optional.empty()); + } + if (name.startsWith("utilization.")) { + return create(MetricType.UTILIZATION, Optional.of(name.substring("utilization.".length()))); + } + if (name.startsWith("named_metrics.")) { + return create(MetricType.NAMED_METRICS, + Optional.of(name.substring("named_metrics.".length()))); + } + return create(MetricType.INVALID, Optional.empty()); + } + + } + /** - * Resolves a metric value from the report based on the given metric name. - * The logic checks for specific prefixes to determine where to look up the metric: - *
    - *
  • "cpu_utilization" -> getCpuUtilization()
    • - *
  • "application_utilization" -> getApplicationUtilization()
    • - *
  • "mem_utilization" -> getMemoryUtilization()
    • - *
  • "utilization." -> lookup in utilizationMetrics
    • - *
  • "named_metrics." -> lookup in namedMetrics
    • - *
+ * Resolves a custom metric value for `parsedMetric` + * Returns -1.0 if the metric is absent or invalid. * * @param report The metric report to query. - * @param metricName The name of the custom metric to look up. - * @return The value of the metric if found, or empty if not found. + * @param parsedMetric The parsed metric to lookup. + * @return The metric value or -1.0 if absent. */ - public static OptionalDouble getMetric(MetricReport report, String metricName) { - if (metricName.equals("cpu_utilization")) { - return OptionalDouble.of(report.getCpuUtilization()); - } else if (metricName.equals("application_utilization")) { - return OptionalDouble.of(report.getApplicationUtilization()); - } else if (metricName.equals("mem_utilization")) { - return OptionalDouble.of(report.getMemoryUtilization()); - } else if (metricName.startsWith("utilization.")) { - Map map = report.getUtilizationMetrics(); - Double val = map.get(metricName.substring("utilization.".length())); - if (val != null) { - return OptionalDouble.of(val); - } - } else if (metricName.startsWith("named_metrics.")) { - Map map = report.getNamedMetrics(); - Double val = map.get(metricName.substring("named_metrics.".length())); - if (val != null) { - return OptionalDouble.of(val); - } + + public static double getMetricValue(MetricReport report, ParsedMetricName parsedMetric) { + switch (parsedMetric.getMetricType()) { + case CPU_UTILIZATION: + return report.getCpuUtilization(); + case APPLICATION_UTILIZATION: + return report.getApplicationUtilization(); + case MEMORY_UTILIZATION: + return report.getMemoryUtilization(); + case UTILIZATION: + if (parsedMetric.getKey().isPresent()) { + String key = parsedMetric.getKey().get(); + Double val = report.getUtilizationMetrics().get(key); + if (val != null) { + return val; + } + } + return -1.0; + case NAMED_METRICS: + if (parsedMetric.getKey().isPresent()) { + String key = parsedMetric.getKey().get(); + Double val = report.getNamedMetrics().get(key); + if (val != null) { + return val; + } + } + return -1.0; + case INVALID: + return -1.0; + default: + return -1.0; } - return OptionalDouble.empty(); } } diff --git a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerProviderTest.java b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerProviderTest.java index 7bd1590885e..0bd3283cb79 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerProviderTest.java @@ -29,6 +29,7 @@ import io.grpc.internal.FakeClock; import io.grpc.internal.JsonParser; import io.grpc.xds.WeightedRoundRobinLoadBalancer.WeightedRoundRobinLoadBalancerConfig; +import io.grpc.xds.internal.MetricReportUtils.ParsedMetricName; import java.io.IOException; import java.util.Map; import org.junit.Test; @@ -112,16 +113,19 @@ public void parseLoadBalancingConfigDefaultValues() throws IOException { } @Test - public void parseLoadBalancingConfigCustomMetrics() throws IOException { + public void parseLoadBalancingConfigCustomMetricsIgnoresInvalid() throws IOException { System.setProperty("GRPC_EXPERIMENTAL_WRR_CUSTOM_METRICS", "true"); try { - String lbConfig = "{\"metricNamesForComputingUtilization\" : [\"foo\", \"bar\"]}"; + String lbConfig = + "{\"metricNamesForComputingUtilization\" : " + + "[\"utilization.foo\", \"invalid_name\", \"named_metrics.bar\"]}"; ConfigOrError configOrError = provider.parseLoadBalancingPolicyConfig( parseJsonObject(lbConfig)); assertThat(configOrError.getConfig()).isNotNull(); WeightedRoundRobinLoadBalancerConfig config = (WeightedRoundRobinLoadBalancerConfig) configOrError.getConfig(); - assertThat(config.metricNamesForComputingUtilization).containsExactly("foo", "bar"); + assertThat(config.parsedMetricNamesForComputingUtilization).containsExactly( + ParsedMetricName.parse("utilization.foo"), ParsedMetricName.parse("named_metrics.bar")); } finally { System.clearProperty("GRPC_EXPERIMENTAL_WRR_CUSTOM_METRICS"); } diff --git a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java index d495521123a..bac62d1a103 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java @@ -291,11 +291,11 @@ public void wrrLifeCycle() { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); int expectedTasks = isEnabledHappyEyeballs() ? 2 : 1; @@ -348,11 +348,11 @@ public void enableOobLoadReportConfig() { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.9, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); int expectedTasks = isEnabledHappyEyeballs() ? 2 : 1; @@ -409,11 +409,11 @@ private void pickByWeight(MetricReport r1, MetricReport r2, MetricReport r3, WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); WeightedChildLbState weightedChild3 = (WeightedChildLbState) getChild(weightedPicker, 2); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport(r1); + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport(r1); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport(r2); + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport(r2); weightedChild3.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport(r3); + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport(r3); assertThat(fakeClock.forwardTime(11, TimeUnit.SECONDS)).isEqualTo(1); Map pickCount = new HashMap<>(); @@ -611,11 +611,11 @@ public void blackoutPeriod() { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); int expectedCount = isEnabledHappyEyeballs() ? 2 : 1; @@ -676,11 +676,11 @@ public void updateWeightTimer() { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); int expectedTasks = isEnabledHappyEyeballs() ? 2 : 1; @@ -695,11 +695,11 @@ weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, .setAttributes(affinity).build())); assertThat(getNumFilteredPendingTasks()).isEqualTo(1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); //timer fires, new weight updated @@ -732,11 +732,11 @@ public void weightExpired() { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); int expectedTasks = isEnabledHappyEyeballs() ? 2 : 1; @@ -840,11 +840,11 @@ public void unknownWeightIsAvgWeight() { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); assertThat(fakeClock.forwardTime(10, TimeUnit.SECONDS)).isEqualTo(1); @@ -883,11 +883,11 @@ public void pickFromOtherThread() throws Exception { WeightedChildLbState weightedChild1 = (WeightedChildLbState) getChild(weightedPicker, 0); WeightedChildLbState weightedChild2 = (WeightedChildLbState) getChild(weightedPicker, 1); weightedChild1.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty, - weightedConfig.metricNamesForComputingUtilization).onLoadReport( + weightedConfig.parsedMetricNamesForComputingUtilization).onLoadReport( InternalCallMetricRecorder.createMetricReport( 0.2, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); CyclicBarrier barrier = new CyclicBarrier(2); @@ -1224,7 +1224,8 @@ public void metrics() { // can be calculated, but it's still essentially round_robin Iterator childLbStates = wrr.getChildLbStates().iterator(); ((WeightedChildLbState) childLbStates.next()).new OrcaReportListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization) + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization) .onLoadReport(InternalCallMetricRecorder.createMetricReport(0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); @@ -1232,11 +1233,13 @@ public void metrics() { // Now send a second child LB state an ORCA update, so there's real weights ((WeightedChildLbState) childLbStates.next()).new OrcaReportListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization) + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization) .onLoadReport(InternalCallMetricRecorder.createMetricReport(0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); ((WeightedChildLbState) childLbStates.next()).new OrcaReportListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization) + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization) .onLoadReport(InternalCallMetricRecorder.createMetricReport(0.1, 0, 0.1, 1, 0, new HashMap<>(), new HashMap<>(), new HashMap<>())); @@ -1355,7 +1358,8 @@ public void customMetric_priority_overAppUtil() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); Map namedMetrics = new HashMap<>(); namedMetrics.put("cost", 0.5); @@ -1389,7 +1393,8 @@ public void customMetric_invalid_fallbackToAppUtil() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); // custom metric is NaN, App util = 0.8 Map namedMetrics = new HashMap<>(); @@ -1424,7 +1429,8 @@ public void customMetric_mapLookup_used() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); Map namedMetrics = new HashMap<>(); namedMetrics.put("cost", 0.5); @@ -1456,7 +1462,8 @@ public void customMetric_shouldFilterOutAndFallbackToCpu() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); // custom metric is NaN, but CPU is 0.1 Map namedMetrics = new HashMap<>(); @@ -1493,7 +1500,8 @@ public void customMetric_multipleMetrics_maxUsed() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); Map namedMetrics = new HashMap<>(); namedMetrics.put("cost", 0.5); @@ -1528,7 +1536,8 @@ public void customMetric_allInvalid_fallbackToCpu() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); Map namedMetrics = new HashMap<>(); namedMetrics.put("cost", Double.NaN); @@ -1564,7 +1573,8 @@ public void customMetric_mixInvalidAndValid_validUsed() { WeightedChildLbState weightedChild = (WeightedChildLbState) wrr.getChildLbStates().iterator().next(); WeightedChildLbState.OrcaReportListener listener = weightedChild.getOrCreateOrcaListener( - weightedConfig.errorUtilizationPenalty, weightedConfig.metricNamesForComputingUtilization); + weightedConfig.errorUtilizationPenalty, + weightedConfig.parsedMetricNamesForComputingUtilization); Map namedMetrics = new HashMap<>(); namedMetrics.put("cost", Double.NaN); diff --git a/xds/src/test/java/io/grpc/xds/internal/MetricReportUtilsTest.java b/xds/src/test/java/io/grpc/xds/internal/MetricReportUtilsTest.java index bf5e0ae9ede..5939340c31c 100644 --- a/xds/src/test/java/io/grpc/xds/internal/MetricReportUtilsTest.java +++ b/xds/src/test/java/io/grpc/xds/internal/MetricReportUtilsTest.java @@ -17,15 +17,12 @@ package io.grpc.xds.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import io.grpc.services.InternalCallMetricRecorder; import io.grpc.services.MetricReport; import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.OptionalDouble; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -35,59 +32,66 @@ public class MetricReportUtilsTest { @Test - public void getMetric_cpuUtilization() { + public void getMetricValue_cpuUtilization() { MetricReport report = createMetricReport(0.5, 0.1, 0.2, 10.0, 5.0, Collections.emptyMap()); - OptionalDouble result = MetricReportUtils.getMetric(report, "cpu_utilization"); - assertTrue(result.isPresent()); - assertEquals(0.5, result.getAsDouble(), 0.0001); + MetricReportUtils.ParsedMetricName parsed = + MetricReportUtils.ParsedMetricName.parse("cpu_utilization"); + assertEquals(0.5, MetricReportUtils.getMetricValue(report, parsed), 0.0001); } @Test - public void getMetric_applicationUtilization() { + public void getMetricValue_applicationUtilization() { MetricReport report = createMetricReport(0.5, 0.1, 0.2, 10.0, 5.0, Collections.emptyMap()); - OptionalDouble result = MetricReportUtils.getMetric(report, "application_utilization"); - assertTrue(result.isPresent()); - assertEquals(0.1, result.getAsDouble(), 0.0001); + MetricReportUtils.ParsedMetricName parsed = + MetricReportUtils.ParsedMetricName.parse("application_utilization"); + assertEquals(0.1, MetricReportUtils.getMetricValue(report, parsed), 0.0001); } @Test - public void getMetric_memUtilization() { + public void getMetricValue_memUtilization() { MetricReport report = createMetricReport(0.5, 0.1, 0.2, 10.0, 5.0, Collections.emptyMap()); - OptionalDouble result = MetricReportUtils.getMetric(report, "mem_utilization"); - assertTrue(result.isPresent()); - assertEquals(0.2, result.getAsDouble(), 0.0001); + MetricReportUtils.ParsedMetricName parsed = + MetricReportUtils.ParsedMetricName.parse("mem_utilization"); + assertEquals(0.2, MetricReportUtils.getMetricValue(report, parsed), 0.0001); } @Test - public void getMetric_utilizationMetric() { + public void getMetricValue_utilizationMetric() { Map utilizationMetrics = new HashMap<>(); utilizationMetrics.put("foo", 1.23); MetricReport report = InternalCallMetricRecorder.createMetricReport( 0, 0, 0, 0, 0, Collections.emptyMap(), utilizationMetrics, Collections.emptyMap()); - OptionalDouble result = MetricReportUtils.getMetric(report, "utilization.foo"); - assertTrue(result.isPresent()); - assertEquals(1.23, result.getAsDouble(), 0.0001); - assertFalse(MetricReportUtils.getMetric(report, "utilization.bar").isPresent()); + MetricReportUtils.ParsedMetricName parsed = + MetricReportUtils.ParsedMetricName.parse("utilization.foo"); + assertEquals(1.23, MetricReportUtils.getMetricValue(report, parsed), 0.0001); + + MetricReportUtils.ParsedMetricName bad = + MetricReportUtils.ParsedMetricName.parse("utilization.bar"); + assertEquals(-1.0, MetricReportUtils.getMetricValue(report, bad), 0.0001); } @Test - public void getMetric_namedMetric() { + public void getMetricValue_namedMetric() { Map namedMetrics = new HashMap<>(); namedMetrics.put("foo", 7.89); MetricReport report = createMetricReport(0, 0, 0, 0, 0, namedMetrics); - OptionalDouble result = MetricReportUtils.getMetric(report, "named_metrics.foo"); - assertTrue(result.isPresent()); - assertEquals(7.89, result.getAsDouble(), 0.0001); - assertFalse(MetricReportUtils.getMetric(report, "named_metrics.bar").isPresent()); + MetricReportUtils.ParsedMetricName parsed = + MetricReportUtils.ParsedMetricName.parse("named_metrics.foo"); + assertEquals(7.89, MetricReportUtils.getMetricValue(report, parsed), 0.0001); + + MetricReportUtils.ParsedMetricName bad = + MetricReportUtils.ParsedMetricName.parse("named_metrics.bar"); + assertEquals(-1.0, MetricReportUtils.getMetricValue(report, bad), 0.0001); } @Test - public void getMetric_unknownPrefix() { - MetricReport report = createMetricReport(0, 0, 0, 0, 0, Collections.emptyMap()); - assertFalse(MetricReportUtils.getMetric(report, "unknown.foo").isPresent()); - assertFalse(MetricReportUtils.getMetric(report, "foo").isPresent()); + public void getMetricValue_invalidMetric() { + MetricReport report = createMetricReport(0.5, 0.1, 0.2, 10.0, 5.0, Collections.emptyMap()); + MetricReportUtils.ParsedMetricName invalid = + MetricReportUtils.ParsedMetricName.parse("invalid_metric"); + assertEquals(-1.0, MetricReportUtils.getMetricValue(report, invalid), 0.0001); } private MetricReport createMetricReport(double cpu, double app, double mem, double qps,