From 014c022b084e50a18370824e4d71dc464ca91d56 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Mon, 18 Jul 2022 13:23:55 -0700 Subject: [PATCH] service: make the orca MetricReport a top level experimental class (#9382) --- ...tomBackendMetricsLoadBalancerProvider.java | 9 ++- services/BUILD.bazel | 1 + .../io/grpc/services/CallMetricRecorder.java | 42 +------------ .../services/InternalCallMetricRecorder.java | 10 ++-- .../grpc/services/InternalMetricRecorder.java | 2 +- .../java/io/grpc/services/MetricRecorder.java | 4 +- .../java/io/grpc/services/MetricReport.java | 59 +++++++++++++++++++ .../grpc/services/CallMetricRecorderTest.java | 4 +- .../OrcaMetricReportingServerInterceptor.java | 4 +- .../java/io/grpc/xds/orca/OrcaOobUtil.java | 9 ++- .../io/grpc/xds/orca/OrcaPerRequestUtil.java | 10 ++-- .../io/grpc/xds/orca/OrcaServiceImpl.java | 4 +- .../io/grpc/xds/orca/OrcaOobUtilTest.java | 6 +- .../grpc/xds/orca/OrcaPerRequestUtilTest.java | 21 ++++--- 14 files changed, 101 insertions(+), 84 deletions(-) create mode 100644 services/src/main/java/io/grpc/services/MetricReport.java diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/CustomBackendMetricsLoadBalancerProvider.java b/interop-testing/src/main/java/io/grpc/testing/integration/CustomBackendMetricsLoadBalancerProvider.java index 7c88c7b1d73..1864afd3c42 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/CustomBackendMetricsLoadBalancerProvider.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/CustomBackendMetricsLoadBalancerProvider.java @@ -23,7 +23,7 @@ import io.grpc.LoadBalancer; import io.grpc.LoadBalancerProvider; import io.grpc.LoadBalancerRegistry; -import io.grpc.services.CallMetricRecorder; +import io.grpc.services.MetricReport; import io.grpc.testing.integration.Messages.TestOrcaReport; import io.grpc.util.ForwardingLoadBalancer; import io.grpc.util.ForwardingLoadBalancerHelper; @@ -87,7 +87,7 @@ public Subchannel createSubchannel(CreateSubchannelArgs args) { Subchannel subchannel = super.createSubchannel(args); OrcaOobUtil.setListener(subchannel, new OrcaOobUtil.OrcaOobReportListener() { @Override - public void onLoadReport(CallMetricRecorder.CallMetricReport orcaLoadReport) { + public void onLoadReport(MetricReport orcaLoadReport) { latestOobReport = fromCallMetricReport(orcaLoadReport); } }, @@ -133,7 +133,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) { OrcaPerRequestUtil.getInstance().newOrcaClientStreamTracerFactory( new OrcaPerRequestUtil.OrcaPerRequestReportListener() { @Override - public void onLoadReport(CallMetricRecorder.CallMetricReport callMetricReport) { + public void onLoadReport(MetricReport callMetricReport) { AtomicReference reportRef = args.getCallOptions().getOption(ORCA_RPC_REPORT_KEY); reportRef.set(fromCallMetricReport(callMetricReport)); @@ -143,8 +143,7 @@ public void onLoadReport(CallMetricRecorder.CallMetricReport callMetricReport) { } } - private static TestOrcaReport fromCallMetricReport( - CallMetricRecorder.CallMetricReport callMetricReport) { + private static TestOrcaReport fromCallMetricReport(MetricReport callMetricReport) { return TestOrcaReport.newBuilder() .setCpuUtilization(callMetricReport.getCpuUtilization()) .setMemoryUtilization(callMetricReport.getMemoryUtilization()) diff --git a/services/BUILD.bazel b/services/BUILD.bazel index 6e85d31d952..78a953d0882 100644 --- a/services/BUILD.bazel +++ b/services/BUILD.bazel @@ -37,6 +37,7 @@ java_library( srcs = [ "src/main/java/io/grpc/services/CallMetricRecorder.java", "src/main/java/io/grpc/services/MetricRecorder.java", + "src/main/java/io/grpc/services/MetricReport.java", ], deps = [ "//api", diff --git a/services/src/main/java/io/grpc/services/CallMetricRecorder.java b/services/src/main/java/io/grpc/services/CallMetricRecorder.java index e87512e3002..2feb5e28e6f 100644 --- a/services/src/main/java/io/grpc/services/CallMetricRecorder.java +++ b/services/src/main/java/io/grpc/services/CallMetricRecorder.java @@ -16,8 +16,6 @@ package io.grpc.services; -import static com.google.common.base.Preconditions.checkNotNull; - import com.google.common.annotations.VisibleForTesting; import io.grpc.Context; import io.grpc.ExperimentalApi; @@ -46,42 +44,6 @@ public final class CallMetricRecorder { private double memoryUtilizationMetric = 0; private volatile boolean disabled; - public static final class CallMetricReport { - private double cpuUtilization; - private double memoryUtilization; - private Map requestCostMetrics; - private Map utilizationMetrics; - - /** - * A gRPC object of orca load report. LB policies listening at per-rpc or oob orca load reports - * will be notified of the metrics data in this data format. - */ - CallMetricReport(double cpuUtilization, double memoryUtilization, - Map requestCostMetrics, - Map utilizationMetrics) { - this.cpuUtilization = cpuUtilization; - this.memoryUtilization = memoryUtilization; - this.requestCostMetrics = checkNotNull(requestCostMetrics, "requestCostMetrics"); - this.utilizationMetrics = checkNotNull(utilizationMetrics, "utilizationMetrics"); - } - - public double getCpuUtilization() { - return cpuUtilization; - } - - public double getMemoryUtilization() { - return memoryUtilization; - } - - public Map getRequestCostMetrics() { - return requestCostMetrics; - } - - public Map getUtilizationMetrics() { - return utilizationMetrics; - } - } - /** * Returns the call metric recorder attached to the current {@link Context}. If there is none, * returns a no-op recorder. @@ -201,13 +163,13 @@ Map finalizeAndDump() { * * @return a per-request ORCA reports containing all saved metrics. */ - CallMetricReport finalizeAndDump2() { + MetricReport finalizeAndDump2() { Map savedRequestCostMetrics = finalizeAndDump(); Map savedUtilizationMetrics = utilizationMetrics.get(); if (savedUtilizationMetrics == null) { savedUtilizationMetrics = Collections.emptyMap(); } - return new CallMetricReport(cpuUtilizationMetric, + return new MetricReport(cpuUtilizationMetric, memoryUtilizationMetric, Collections.unmodifiableMap(savedRequestCostMetrics), Collections.unmodifiableMap(savedUtilizationMetrics) ); diff --git a/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java b/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java index 01f6cc6417c..97e5e5a0aa6 100644 --- a/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java +++ b/services/src/main/java/io/grpc/services/InternalCallMetricRecorder.java @@ -41,15 +41,13 @@ public static Map finalizeAndDump(CallMetricRecorder recorder) { return recorder.finalizeAndDump(); } - public static CallMetricRecorder.CallMetricReport finalizeAndDump2(CallMetricRecorder recorder) { + public static MetricReport finalizeAndDump2(CallMetricRecorder recorder) { return recorder.finalizeAndDump2(); } - public static CallMetricRecorder.CallMetricReport createMetricReport( - double cpuUtilization, double memoryUtilization, - Map requestCostMetrics, - Map utilizationMetrics) { - return new CallMetricRecorder.CallMetricReport(cpuUtilization, memoryUtilization, + public static MetricReport createMetricReport(double cpuUtilization, double memoryUtilization, + Map requestCostMetrics, Map utilizationMetrics) { + return new MetricReport(cpuUtilization, memoryUtilization, requestCostMetrics, utilizationMetrics); } } diff --git a/services/src/main/java/io/grpc/services/InternalMetricRecorder.java b/services/src/main/java/io/grpc/services/InternalMetricRecorder.java index 4519bf661c3..cd36c425ac8 100644 --- a/services/src/main/java/io/grpc/services/InternalMetricRecorder.java +++ b/services/src/main/java/io/grpc/services/InternalMetricRecorder.java @@ -29,7 +29,7 @@ public final class InternalMetricRecorder { private InternalMetricRecorder() { } - public static CallMetricRecorder.CallMetricReport getMetricReport(MetricRecorder recorder) { + public static MetricReport getMetricReport(MetricRecorder recorder) { return recorder.getMetricReport(); } } diff --git a/services/src/main/java/io/grpc/services/MetricRecorder.java b/services/src/main/java/io/grpc/services/MetricRecorder.java index 31c10083732..a576386e98b 100644 --- a/services/src/main/java/io/grpc/services/MetricRecorder.java +++ b/services/src/main/java/io/grpc/services/MetricRecorder.java @@ -86,8 +86,8 @@ public void clearMemoryUtilizationMetric() { memoryUtilization = 0; } - CallMetricRecorder.CallMetricReport getMetricReport() { - return new CallMetricRecorder.CallMetricReport(cpuUtilization, memoryUtilization, + MetricReport getMetricReport() { + return new MetricReport(cpuUtilization, memoryUtilization, Collections.emptyMap(), Collections.unmodifiableMap(metricsData)); } } diff --git a/services/src/main/java/io/grpc/services/MetricReport.java b/services/src/main/java/io/grpc/services/MetricReport.java new file mode 100644 index 00000000000..3b4ccf3e5f8 --- /dev/null +++ b/services/src/main/java/io/grpc/services/MetricReport.java @@ -0,0 +1,59 @@ +/* + * Copyright 2022 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; + +import static com.google.common.base.Preconditions.checkNotNull; + +import io.grpc.ExperimentalApi; +import java.util.Map; + +/** + * A gRPC object of orca load report. LB policies listening at per-rpc or oob orca load reports + * will be notified of the metrics data in this data format. + */ +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/9381") +public final class MetricReport { + private double cpuUtilization; + private double memoryUtilization; + private Map requestCostMetrics; + private Map utilizationMetrics; + + MetricReport(double cpuUtilization, double memoryUtilization, + Map requestCostMetrics, + Map utilizationMetrics) { + this.cpuUtilization = cpuUtilization; + this.memoryUtilization = memoryUtilization; + this.requestCostMetrics = checkNotNull(requestCostMetrics, "requestCostMetrics"); + this.utilizationMetrics = checkNotNull(utilizationMetrics, "utilizationMetrics"); + } + + public double getCpuUtilization() { + return cpuUtilization; + } + + public double getMemoryUtilization() { + return memoryUtilization; + } + + public Map getRequestCostMetrics() { + return requestCostMetrics; + } + + public Map getUtilizationMetrics() { + return utilizationMetrics; + } +} diff --git a/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java b/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java index e6649d42887..93745d53ed7 100644 --- a/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java +++ b/services/src/test/java/io/grpc/services/CallMetricRecorderTest.java @@ -47,7 +47,7 @@ public void dumpDumpsAllSavedMetricValues() { recorder.recordCpuUtilizationMetric(0.1928); recorder.recordMemoryUtilizationMetric(47.4); - CallMetricRecorder.CallMetricReport dump = recorder.finalizeAndDump2(); + MetricReport dump = recorder.finalizeAndDump2(); Truth.assertThat(dump.getUtilizationMetrics()) .containsExactly("util1", 154353.423, "util2", 0.1367, "util3", 1437.34); Truth.assertThat(dump.getRequestCostMetrics()) @@ -76,7 +76,7 @@ public void lastValueWinForMetricsWithSameName() { recorder.recordMemoryUtilizationMetric(9384.0); recorder.recordUtilizationMetric("util1", 84323.3); - CallMetricRecorder.CallMetricReport dump = recorder.finalizeAndDump2(); + MetricReport dump = recorder.finalizeAndDump2(); Truth.assertThat(dump.getRequestCostMetrics()) .containsExactly("cost1", 4654.67, "cost2", 75.83); Truth.assertThat(dump.getMemoryUtilization()).isEqualTo(9384.0); diff --git a/xds/src/main/java/io/grpc/xds/orca/OrcaMetricReportingServerInterceptor.java b/xds/src/main/java/io/grpc/xds/orca/OrcaMetricReportingServerInterceptor.java index db624df7435..729277072c2 100644 --- a/xds/src/main/java/io/grpc/xds/orca/OrcaMetricReportingServerInterceptor.java +++ b/xds/src/main/java/io/grpc/xds/orca/OrcaMetricReportingServerInterceptor.java @@ -31,6 +31,7 @@ import io.grpc.protobuf.ProtoUtils; import io.grpc.services.CallMetricRecorder; import io.grpc.services.InternalCallMetricRecorder; +import io.grpc.services.MetricReport; /** * A {@link ServerInterceptor} that intercepts a {@link ServerCall} by running server-side RPC @@ -89,8 +90,7 @@ public void close(Status status, Metadata trailers) { next); } - private static OrcaLoadReport fromInternalReport( - CallMetricRecorder.CallMetricReport internalReport) { + private static OrcaLoadReport fromInternalReport(MetricReport internalReport) { return OrcaLoadReport.newBuilder() .setCpuUtilization(internalReport.getCpuUtilization()) .setMemUtilization(internalReport.getMemoryUtilization()) diff --git a/xds/src/main/java/io/grpc/xds/orca/OrcaOobUtil.java b/xds/src/main/java/io/grpc/xds/orca/OrcaOobUtil.java index 2e6a2891569..016c4ba0eb5 100644 --- a/xds/src/main/java/io/grpc/xds/orca/OrcaOobUtil.java +++ b/xds/src/main/java/io/grpc/xds/orca/OrcaOobUtil.java @@ -51,7 +51,7 @@ import io.grpc.internal.BackoffPolicy; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; -import io.grpc.services.CallMetricRecorder; +import io.grpc.services.MetricReport; import io.grpc.util.ForwardingLoadBalancerHelper; import io.grpc.util.ForwardingSubchannel; import java.util.HashMap; @@ -169,9 +169,9 @@ public interface OrcaOobReportListener { *

Note this callback will be invoked from the {@link SynchronizationContext} of the * delegated helper, implementations should not block. * - * @param report load report in the format of grpc {@link CallMetricRecorder.CallMetricReport}. + * @param report load report in the format of grpc {@link MetricReport}. */ - void onLoadReport(CallMetricRecorder.CallMetricReport report); + void onLoadReport(MetricReport report); } static final Attributes.Key ORCA_REPORTING_STATE_KEY = @@ -451,8 +451,7 @@ void handleResponse(OrcaLoadReport response) { callHasResponded = true; backoffPolicy = null; subchannelLogger.log(ChannelLogLevel.DEBUG, "Received an ORCA report: {0}", response); - CallMetricRecorder.CallMetricReport metricReport = - OrcaPerRequestUtil.fromOrcaLoadReport(response); + MetricReport metricReport = OrcaPerRequestUtil.fromOrcaLoadReport(response); for (OrcaOobReportListener listener : configs.keySet()) { listener.onLoadReport(metricReport); } diff --git a/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java b/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java index 4006e989545..0c2c7395b47 100644 --- a/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java +++ b/xds/src/main/java/io/grpc/xds/orca/OrcaPerRequestUtil.java @@ -28,8 +28,8 @@ import io.grpc.Metadata; import io.grpc.internal.ForwardingClientStreamTracer; import io.grpc.protobuf.ProtoUtils; -import io.grpc.services.CallMetricRecorder; import io.grpc.services.InternalCallMetricRecorder; +import io.grpc.services.MetricReport; import java.util.ArrayList; import java.util.List; @@ -182,9 +182,9 @@ public interface OrcaPerRequestReportListener { *

Note this callback will be invoked from the network thread as the RPC finishes, * implementations should not block. * - * @param report load report in the format of grpc {@link CallMetricRecorder.CallMetricReport}. + * @param report load report in the format of grpc {@link MetricReport}. */ - void onLoadReport(CallMetricRecorder.CallMetricReport report); + void onLoadReport(MetricReport report); } /** @@ -252,7 +252,7 @@ public void inboundTrailers(Metadata trailers) { } } - static CallMetricRecorder.CallMetricReport fromOrcaLoadReport(OrcaLoadReport loadReport) { + static MetricReport fromOrcaLoadReport(OrcaLoadReport loadReport) { return InternalCallMetricRecorder.createMetricReport(loadReport.getCpuUtilization(), loadReport.getMemUtilization(), loadReport.getRequestCostMap(), loadReport.getUtilizationMap()); @@ -271,7 +271,7 @@ void addListener(OrcaPerRequestReportListener listener) { } void onReport(OrcaLoadReport report) { - CallMetricRecorder.CallMetricReport metricReport = fromOrcaLoadReport(report); + MetricReport metricReport = fromOrcaLoadReport(report); for (OrcaPerRequestReportListener listener : listeners) { listener.onLoadReport(metricReport); } diff --git a/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java b/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java index 6046fbd1441..1ea64f70bf2 100644 --- a/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java +++ b/xds/src/main/java/io/grpc/xds/orca/OrcaServiceImpl.java @@ -26,9 +26,9 @@ import io.grpc.BindableService; import io.grpc.ServerServiceDefinition; import io.grpc.SynchronizationContext; -import io.grpc.services.CallMetricRecorder; import io.grpc.services.InternalMetricRecorder; import io.grpc.services.MetricRecorder; +import io.grpc.services.MetricReport; import io.grpc.stub.ServerCallStreamObserver; import io.grpc.stub.StreamObserver; import java.util.concurrent.ScheduledExecutorService; @@ -146,7 +146,7 @@ public void run() { } private OrcaLoadReport generateMetricsReport() { - CallMetricRecorder.CallMetricReport internalReport = + MetricReport internalReport = InternalMetricRecorder.getMetricReport(metricRecorder); return OrcaLoadReport.newBuilder().setCpuUtilization(internalReport.getCpuUtilization()) .setMemUtilization(internalReport.getMemoryUtilization()) diff --git a/xds/src/test/java/io/grpc/xds/orca/OrcaOobUtilTest.java b/xds/src/test/java/io/grpc/xds/orca/OrcaOobUtilTest.java index 5a981123ae0..4ae70bfba4a 100644 --- a/xds/src/test/java/io/grpc/xds/orca/OrcaOobUtilTest.java +++ b/xds/src/test/java/io/grpc/xds/orca/OrcaOobUtilTest.java @@ -62,7 +62,7 @@ import io.grpc.inprocess.InProcessServerBuilder; import io.grpc.internal.BackoffPolicy; import io.grpc.internal.FakeClock; -import io.grpc.services.CallMetricRecorder; +import io.grpc.services.MetricReport; import io.grpc.stub.StreamObserver; import io.grpc.testing.GrpcCleanupRule; import io.grpc.util.ForwardingLoadBalancerHelper; @@ -667,7 +667,7 @@ public void policiesReceiveSameReportIndependently() { orcaServiceImps[0].calls.peek().responseObserver.onNext(report); assertLog(subchannels[0].logs, "DEBUG: Received an ORCA report: " + report); // Only parent helper's listener receives the report. - ArgumentCaptor parentReportCaptor = + ArgumentCaptor parentReportCaptor = ArgumentCaptor.forClass(null); verify(mockOrcaListener1).onLoadReport(parentReportCaptor.capture()); assertThat(OrcaPerRequestUtilTest.reportEqual(parentReportCaptor.getValue(), @@ -679,7 +679,7 @@ public void policiesReceiveSameReportIndependently() { orcaServiceImps[0].calls.peek().responseObserver.onNext(report); assertLog(subchannels[0].logs, "DEBUG: Received an ORCA report: " + report); // Both helper receives the same report instance. - ArgumentCaptor childReportCaptor = + ArgumentCaptor childReportCaptor = ArgumentCaptor.forClass(null); verify(mockOrcaListener1, times(2)) .onLoadReport(parentReportCaptor.capture()); diff --git a/xds/src/test/java/io/grpc/xds/orca/OrcaPerRequestUtilTest.java b/xds/src/test/java/io/grpc/xds/orca/OrcaPerRequestUtilTest.java index 2b3de09b573..5f91b0fdcb1 100644 --- a/xds/src/test/java/io/grpc/xds/orca/OrcaPerRequestUtilTest.java +++ b/xds/src/test/java/io/grpc/xds/orca/OrcaPerRequestUtilTest.java @@ -31,7 +31,7 @@ import com.google.common.base.Objects; import io.grpc.ClientStreamTracer; import io.grpc.Metadata; -import io.grpc.services.CallMetricRecorder; +import io.grpc.services.MetricReport; import io.grpc.xds.orca.OrcaPerRequestUtil.OrcaPerRequestReportListener; import io.grpc.xds.orca.OrcaPerRequestUtil.OrcaReportingTracerFactory; import org.junit.Before; @@ -99,29 +99,28 @@ public void singlePolicyTypicalWorkflow() { OrcaReportingTracerFactory.ORCA_ENDPOINT_LOAD_METRICS_KEY, OrcaLoadReport.getDefaultInstance()); tracer.inboundTrailers(trailer); - ArgumentCaptor reportCaptor = + ArgumentCaptor reportCaptor = ArgumentCaptor.forClass(null); verify(orcaListener1).onLoadReport(reportCaptor.capture()); assertThat(reportEqual(reportCaptor.getValue(), OrcaPerRequestUtil.fromOrcaLoadReport(OrcaLoadReport.getDefaultInstance()))).isTrue(); } - static final class MetricsReportMatcher implements - ArgumentMatcher { - private CallMetricRecorder.CallMetricReport original; + static final class MetricsReportMatcher implements ArgumentMatcher { + private MetricReport original; - public MetricsReportMatcher(CallMetricRecorder.CallMetricReport report) { + public MetricsReportMatcher(MetricReport report) { this.original = report; } @Override - public boolean matches(CallMetricRecorder.CallMetricReport argument) { + public boolean matches(MetricReport argument) { return reportEqual(original, argument); } } - static boolean reportEqual(CallMetricRecorder.CallMetricReport a, - CallMetricRecorder.CallMetricReport b) { + static boolean reportEqual(MetricReport a, + MetricReport b) { return a.getCpuUtilization() == b.getCpuUtilization() && a.getMemoryUtilization() == b.getMemoryUtilization() && Objects.equal(a.getRequestCostMetrics(), b.getRequestCostMetrics()) @@ -163,9 +162,9 @@ public void twoLevelPoliciesTypicalWorkflow() { OrcaReportingTracerFactory.ORCA_ENDPOINT_LOAD_METRICS_KEY, OrcaLoadReport.getDefaultInstance()); childTracer.inboundTrailers(trailer); - ArgumentCaptor parentReportCap = + ArgumentCaptor parentReportCap = ArgumentCaptor.forClass(null); - ArgumentCaptor childReportCap = + ArgumentCaptor childReportCap = ArgumentCaptor.forClass(null); verify(orcaListener1).onLoadReport(parentReportCap.capture()); verify(orcaListener2).onLoadReport(childReportCap.capture());