services,orca: update backend metrics support to allow for server-wide metrics recording (per-call and OOB)

examples/example-orca/src/main/java/io/grpc/examples/orca/CustomBackendMetricsServer.java

@@ -63,7 +63,7 @@ private void start() throws IOException {
         // Enable OOB custom backend metrics reporting.
         .addService(orcaOobService)
         // Enable per-query custom backend metrics reporting.
-        .intercept(OrcaMetricReportingServerInterceptor.getInstance())
+        .intercept(OrcaMetricReportingServerInterceptor.create(metricRecorder))
         .build()
         .start();
     logger.info("Server started, listening on " + port);

interop-testing/src/main/java/io/grpc/testing/integration/AbstractInteropTest.java

@@ -1777,7 +1777,7 @@ public void testOrcaOob() throws Exception {
     final TestOrcaReport answer2 = TestOrcaReport.newBuilder()
         .setCpuUtilization(0.29309)
         .setMemoryUtilization(0.2)
-        .putUtilization("util", 100.2039)
+        .putUtilization("util", 0.2039)
         .build();
 
     final int retryLimit = 5;

interop-testing/src/main/java/io/grpc/testing/integration/TestServiceServer.java

@@ -164,7 +164,7 @@ void start() throws Exception {
             ServerInterceptors.intercept(
                 new TestServiceImpl(executor, metricRecorder), TestServiceImpl.interceptors()))
         .addService(orcaOobService)
-        .intercept(OrcaMetricReportingServerInterceptor.getInstance())
+        .intercept(OrcaMetricReportingServerInterceptor.create(metricRecorder))
         .build()
         .start();
   }

services/BUILD.bazel

@@ -37,6 +37,7 @@ java_library(
         "src/main/java/io/grpc/services/CallMetricRecorder.java",
         "src/main/java/io/grpc/services/MetricRecorder.java",
         "src/main/java/io/grpc/services/MetricReport.java",
+        "src/main/java/io/grpc/services/MetricRecorderHelper.java",
     ],
     deps = [
         "//api",

services/src/main/java/io/grpc/services/CallMetricRecorder.java

@@ -65,16 +65,16 @@ public static CallMetricRecorder getCurrent() {
   }
 
   /**
-   * Records a call metric measurement for utilization.
-   * If RPC has already finished, this method is no-op.
+   * Records a call metric measurement for utilization in the range [0, 1]. Values outside the valid
+   * range are ignored. If RPC has already finished, this method is no-op.
    *
    * <p>A latter record will overwrite its former name-sakes.
    *
    * @return this recorder object
    * @since 1.23.0
    */
   public CallMetricRecorder recordUtilizationMetric(String name, double value) {
-    if (disabled) {
+    if (disabled || !MetricRecorderHelper.isUtilizationValid(value)) {
       return this;
     }
     if (utilizationMetrics.get() == null) {
@@ -126,50 +126,50 @@ public CallMetricRecorder recordRequestCostMetric(String name, double value) {
   }
 
   /**
-   * Records a call metric measurement for CPU utilization.
-   * If RPC has already finished, this method is no-op.
+   * Records a call metric measurement for CPU utilization in the range [0, 1]. Values outside the
+   * valid range are ignored. If RPC has already finished, this method is no-op.
    *
    * <p>A latter record will overwrite its former name-sakes.
    *
    * @return this recorder object
    * @since 1.47.0
    */
   public CallMetricRecorder recordCpuUtilizationMetric(double value) {
-    if (disabled) {
+    if (disabled || !MetricRecorderHelper.isUtilizationValid(value)) {
       return this;
     }
     cpuUtilizationMetric = value;
     return this;
   }
 
   /**
-   * Records a call metric measurement for memory utilization.
-   * If RPC has already finished, this method is no-op.
+   * Records a call metric measurement for memory utilization in the range [0, 1]. Values outside
+   * the valid range are ignored. If RPC has already finished, this method is no-op.
    *
    * <p>A latter record will overwrite its former name-sakes.
    *
    * @return this recorder object
    * @since 1.47.0
    */
   public CallMetricRecorder recordMemoryUtilizationMetric(double value) {
-    if (disabled) {
+    if (disabled || !MetricRecorderHelper.isUtilizationValid(value)) {
       return this;
     }
     memoryUtilizationMetric = value;
     return this;
   }
 
   /**
-   * Records a call metric measurement for qps.
-   * If RPC has already finished, this method is no-op.
+   * Records a call metric measurement for qps in the range [0, inf). Values outside the valid range
+   * are ignored. If RPC has already finished, this method is no-op.
    *
    * <p>A latter record will overwrite its former name-sakes.
    *
    * @return this recorder object
    * @since 1.54.0
    */
   public CallMetricRecorder recordQpsMetric(double value) {
-    if (disabled) {
+    if (disabled || !MetricRecorderHelper.isQpsValid(value)) {
       return this;
     }
     qps = value;

services/src/main/java/io/grpc/services/MetricRecorder.java

@@ -39,14 +39,18 @@ public static MetricRecorder newInstance() {
   private MetricRecorder() {}
 
   /**
-   * Update the metrics value corresponding to the specified key.
+   * Update the metrics value in the range [0, 1] corresponding to the specified key. Values outside
+   * the valid range are ignored.
    */
   public void putUtilizationMetric(String key, double value) {
+    if (!MetricRecorderHelper.isUtilizationValid(value)) {
+      return;
+    }
     metricsData.put(key, value);
   }
 
   /**
-   * Replace the whole metrics data using the specified map.
+   * Replace the whole metrics data using the specified map. No range validation.
    */
   public void setAllUtilizationMetrics(Map<String, Double> metrics) {
     metricsData = new ConcurrentHashMap<>(metrics);
@@ -60,9 +64,13 @@ public void removeUtilizationMetric(String key) {
   }
 
   /**
-   * Update the CPU utilization metrics data.
+   * Update the CPU utilization metrics data in the range [0, 1]. Values outside the valid range are
+   * ignored.
    */
   public void setCpuUtilizationMetric(double value) {
+    if (!MetricRecorderHelper.isUtilizationValid(value)) {
+      return;
+    }
     cpuUtilization = value;
   }
 
@@ -74,9 +82,13 @@ public void clearCpuUtilizationMetric() {
   }
 
   /**
-   * Update the memory utilization metrics data.
+   * Update the memory utilization metrics data in the range [0, 1]. Values outside the valid range
+   * are ignored.
    */
   public void setMemoryUtilizationMetric(double value) {
+    if (!MetricRecorderHelper.isUtilizationValid(value)) {
+      return;
+    }
     memoryUtilization = value;
   }
 
@@ -88,9 +100,12 @@ public void clearMemoryUtilizationMetric() {
   }
 
   /**
-   * Update the QPS metrics data.
+   * Update the QPS metrics data in the range [0, inf). Values outside the valid range are ignored.
    */
   public void setQps(double value) {
+    if (!MetricRecorderHelper.isQpsValid(value)) {
+      return;
+    }
     qps = value;
   }
 

services/src/main/java/io/grpc/services/MetricRecorderHelper.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2023 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.services;
+
+/**
+ * Utility helper class to check whether values for {@link CallMetricRecorder} and
+ * {@link MetricRecorder} are inside the valid range.
+ */
+final class MetricRecorderHelper {
+
+  /**
+   * Return true if the utilization value is in the range [0, 1] and false otherwise.
+   */
+  static boolean isUtilizationValid(double utilization) {
+    return utilization >= 0.0 && utilization <= 1.0;
+  }
+
+  /**
+   * Return true if the qps value is in the range [0, inf) and false otherwise.
+   */
+  static boolean isQpsValid(double qps) {
+    return qps >= 0.0;
+  }
+
+  // Prevent instantiation.
+  private MetricRecorderHelper() {
+  }
+}

services/src/test/java/io/grpc/services/CallMetricRecorderTest.java

@@ -38,54 +38,83 @@ public void dumpGivesEmptyResultWhenNoSavedMetricValues() {
 
   @Test
   public void dumpDumpsAllSavedMetricValues() {
-    recorder.recordUtilizationMetric("util1", 154353.423);
+    recorder.recordUtilizationMetric("util1", 0.154353423);
     recorder.recordUtilizationMetric("util2", 0.1367);
-    recorder.recordUtilizationMetric("util3", 1437.34);
+    recorder.recordUtilizationMetric("util3", 0.143734);
     recorder.recordRequestCostMetric("cost1", 37465.12);
     recorder.recordRequestCostMetric("cost2", 10293.0);
     recorder.recordRequestCostMetric("cost3", 1.0);
     recorder.recordCpuUtilizationMetric(0.1928);
-    recorder.recordMemoryUtilizationMetric(47.4);
+    recorder.recordMemoryUtilizationMetric(0.474);
     recorder.recordQpsMetric(2522.54);
 
     MetricReport dump = recorder.finalizeAndDump2();
     Truth.assertThat(dump.getUtilizationMetrics())
-        .containsExactly("util1", 154353.423, "util2", 0.1367, "util3", 1437.34);
+        .containsExactly("util1", 0.154353423, "util2", 0.1367, "util3", 0.143734);
     Truth.assertThat(dump.getRequestCostMetrics())
         .containsExactly("cost1", 37465.12, "cost2", 10293.0, "cost3", 1.0);
     Truth.assertThat(dump.getCpuUtilization()).isEqualTo(0.1928);
-    Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(47.4);
+    Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(0.474);
     Truth.assertThat(dump.getQps()).isEqualTo(2522.54);
   }
 
   @Test
   public void noMetricsRecordedAfterSnapshot() {
     Map<String, Double> initDump = recorder.finalizeAndDump();
-    recorder.recordUtilizationMetric("cost", 154353.423);
+    recorder.recordUtilizationMetric("cost", 0.154353423);
     assertThat(recorder.finalizeAndDump()).isEqualTo(initDump);
   }
 
+  @Test
+  public void noMetricsRecordedIfUtilizationIsGreaterThanUpperBound() {
+    recorder.recordCpuUtilizationMetric(1.001);
+    recorder.recordMemoryUtilizationMetric(1.001);
+    recorder.recordUtilizationMetric("util1", 1.001);
+
+    MetricReport dump = recorder.finalizeAndDump2();
+    Truth.assertThat(dump.getCpuUtilization()).isEqualTo(0);
+    Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(0);
+    Truth.assertThat(dump.getQps()).isEqualTo(0);
+    Truth.assertThat(dump.getUtilizationMetrics()).isEmpty();
+    Truth.assertThat(dump.getRequestCostMetrics()).isEmpty();
+  }
+
+  @Test
+  public void noMetricsRecordedIfUtilizationAndQpsAreLessThanLowerBound() {
+    recorder.recordCpuUtilizationMetric(-0.001);
+    recorder.recordMemoryUtilizationMetric(-0.001);
+    recorder.recordQpsMetric(-0.001);
+    recorder.recordUtilizationMetric("util1", -0.001);
+
+    MetricReport dump = recorder.finalizeAndDump2();
+    Truth.assertThat(dump.getCpuUtilization()).isEqualTo(0);
+    Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(0);
+    Truth.assertThat(dump.getQps()).isEqualTo(0);
+    Truth.assertThat(dump.getUtilizationMetrics()).isEmpty();
+    Truth.assertThat(dump.getRequestCostMetrics()).isEmpty();
+  }
+
   @Test
   public void lastValueWinForMetricsWithSameName() {
     recorder.recordRequestCostMetric("cost1", 3412.5435);
     recorder.recordRequestCostMetric("cost2", 6441.341);
     recorder.recordRequestCostMetric("cost1", 6441.341);
     recorder.recordRequestCostMetric("cost1", 4654.67);
     recorder.recordRequestCostMetric("cost2", 75.83);
-    recorder.recordMemoryUtilizationMetric(1.3);
-    recorder.recordMemoryUtilizationMetric(3.1);
-    recorder.recordUtilizationMetric("util1", 28374.21);
-    recorder.recordMemoryUtilizationMetric(9384.0);
-    recorder.recordUtilizationMetric("util1", 84323.3);
+    recorder.recordMemoryUtilizationMetric(0.13);
+    recorder.recordMemoryUtilizationMetric(0.31);
+    recorder.recordUtilizationMetric("util1", 0.2837421);
+    recorder.recordMemoryUtilizationMetric(0.93840);
+    recorder.recordUtilizationMetric("util1", 0.843233);
     recorder.recordQpsMetric(1928.3);
     recorder.recordQpsMetric(100.8);
 
     MetricReport dump = recorder.finalizeAndDump2();
     Truth.assertThat(dump.getRequestCostMetrics())
         .containsExactly("cost1", 4654.67, "cost2", 75.83);
-    Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(9384.0);
+    Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(0.93840);
     Truth.assertThat(dump.getUtilizationMetrics())
-        .containsExactly("util1", 84323.3);
+        .containsExactly("util1", 0.843233);
     Truth.assertThat(dump.getCpuUtilization()).isEqualTo(0);
     Truth.assertThat(dump.getQps()).isEqualTo(100.8);
   }