From f737cbc143d7e6619471f845d1b5001ef33bc3b7 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 7 May 2024 13:50:45 -0700 Subject: [PATCH] api: Hide internal metric APIs Some APIs were marked experimental but had internal APIs in their surface. These were all changed to internal. And then the internal APIs were mostly hidden from generated documentation. All these APIs will eventually become public and maybe even stable. But they need some iteration before we're ready for others to start using them. --- api/build.gradle | 4 ++++ .../java/io/grpc/ForwardingChannelBuilder2.java | 2 +- .../java/io/grpc/InternalManagedChannelBuilder.java | 6 ++++++ api/src/main/java/io/grpc/LoadBalancer.java | 2 +- .../main/java/io/grpc/ManagedChannelBuilder.java | 4 ++-- api/src/main/java/io/grpc/MetricSink.java | 2 +- .../io/grpc/internal/ManagedChannelImplBuilder.java | 2 +- .../io/grpc/opentelemetry/GrpcOpenTelemetry.java | 2 +- .../test/java/io/grpc/rls/RlsLoadBalancerTest.java | 10 ++++++---- .../xds/WeightedRoundRobinLoadBalancerTest.java | 13 ++++++++----- 10 files changed, 31 insertions(+), 16 deletions(-) diff --git a/api/build.gradle b/api/build.gradle index 0a80a1e48b9..1d21c7bdcb6 100644 --- a/api/build.gradle +++ b/api/build.gradle @@ -55,7 +55,11 @@ dependencies { tasks.named("javadoc").configure { source sourceSets.context.allSource // We want io.grpc.Internal, but not io.grpc.Internal* + exclude 'io/grpc/*MetricInstrument.java' + exclude 'io/grpc/*MetricInstrumentRegistry.java' exclude 'io/grpc/Internal?*.java' + exclude 'io/grpc/MetricRecorder.java' + exclude 'io/grpc/MetricSink.java' } tasks.named("sourcesJar").configure { diff --git a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java index a34dea36453..7f21a57ec80 100644 --- a/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java +++ b/api/src/main/java/io/grpc/ForwardingChannelBuilder2.java @@ -258,7 +258,7 @@ public T disableServiceConfigLookUp() { } @Override - public T addMetricSink(MetricSink metricSink) { + protected T addMetricSink(MetricSink metricSink) { delegate().addMetricSink(metricSink); return thisT(); } diff --git a/api/src/main/java/io/grpc/InternalManagedChannelBuilder.java b/api/src/main/java/io/grpc/InternalManagedChannelBuilder.java index 15c079fdb6b..083cad40098 100644 --- a/api/src/main/java/io/grpc/InternalManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/InternalManagedChannelBuilder.java @@ -19,6 +19,7 @@ /** * Internal accessors for {@link ManagedChannelBuilder}. */ +@Internal public final class InternalManagedChannelBuilder { private InternalManagedChannelBuilder() {} @@ -27,5 +28,10 @@ public static > T interceptWithTarget( return builder.interceptWithTarget(factory); } + public static > T addMetricSink( + ManagedChannelBuilder builder, MetricSink metricSink) { + return builder.addMetricSink(metricSink); + } + public interface InternalInterceptorFactory extends ManagedChannelBuilder.InterceptorFactory {} } diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index ae9336eb98d..80e3f8b89c7 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -1255,7 +1255,7 @@ public NameResolverRegistry getNameResolverRegistry() { * * @since 1.64.0 */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/11110") + @Internal public MetricRecorder getMetricRecorder() { return new MetricRecorder() {}; } diff --git a/api/src/main/java/io/grpc/ManagedChannelBuilder.java b/api/src/main/java/io/grpc/ManagedChannelBuilder.java index 9f8e5479ad5..6e30d8eae04 100644 --- a/api/src/main/java/io/grpc/ManagedChannelBuilder.java +++ b/api/src/main/java/io/grpc/ManagedChannelBuilder.java @@ -628,8 +628,8 @@ public T disableServiceConfigLookUp() { * @return this * @since 1.64.0 */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/11110") - public T addMetricSink(MetricSink metricSink) { + @Internal + protected T addMetricSink(MetricSink metricSink) { throw new UnsupportedOperationException(); } diff --git a/api/src/main/java/io/grpc/MetricSink.java b/api/src/main/java/io/grpc/MetricSink.java index a7ca8d8f9aa..0f56b1acb73 100644 --- a/api/src/main/java/io/grpc/MetricSink.java +++ b/api/src/main/java/io/grpc/MetricSink.java @@ -23,7 +23,7 @@ /** * An internal interface representing a receiver or aggregator of gRPC metrics data. */ -@ExperimentalApi("https://github.com/grpc/grpc-java/issues/11110") +@Internal public interface MetricSink { /** diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index b12fd43d36f..7da9125087e 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -687,7 +687,7 @@ public ManagedChannelImplBuilder enableCheckAuthority() { } @Override - public ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { + protected ManagedChannelImplBuilder addMetricSink(MetricSink metricSink) { metricSinks.add(checkNotNull(metricSink, "metric sink")); return this; } diff --git a/opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java b/opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java index f35b7cd1818..e9f3ee73f16 100644 --- a/opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java +++ b/opentelemetry/src/main/java/io/grpc/opentelemetry/GrpcOpenTelemetry.java @@ -145,7 +145,7 @@ public void configureServerBuilder(ServerBuilder serverBuilder) { * Configures the given {@link ManagedChannelBuilder} with OpenTelemetry metrics instrumentation. */ public void configureChannelBuilder(ManagedChannelBuilder builder) { - builder.addMetricSink(sink); + InternalManagedChannelBuilder.addMetricSink(builder, sink); InternalManagedChannelBuilder.interceptWithTarget( builder, openTelemetryMetricsModule::getClientInterceptor); } diff --git a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java index 56cd3cd9449..1f46b86db60 100644 --- a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java @@ -42,6 +42,7 @@ import io.grpc.ConnectivityStateInfo; import io.grpc.EquivalentAddressGroup; import io.grpc.ForwardingChannelBuilder2; +import io.grpc.InternalManagedChannelBuilder; import io.grpc.LoadBalancer.CreateSubchannelArgs; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.PickDetailsConsumer; @@ -310,10 +311,11 @@ public void metricsWithRealChannel() throws Exception { .start()); MetricSink metrics = mock(MetricSink.class, delegatesTo(new NoopMetricSink())); ManagedChannel channel = grpcCleanupRule.register( - InProcessChannelBuilder.forName("fake-bigtable.googleapis.com") - .defaultServiceConfig(parseJson(getServiceConfigJsonStr())) - .addMetricSink(metrics) - .directExecutor() + InternalManagedChannelBuilder.addMetricSink( + InProcessChannelBuilder.forName("fake-bigtable.googleapis.com") + .defaultServiceConfig(parseJson(getServiceConfigJsonStr())) + .directExecutor(), + metrics) .build()); StreamRecorder recorder = StreamRecorder.create(); diff --git a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java index 618742c41ee..2913a1e1d7d 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java @@ -44,6 +44,7 @@ import io.grpc.ConnectivityStateInfo; import io.grpc.DoubleHistogramMetricInstrument; import io.grpc.EquivalentAddressGroup; +import io.grpc.InternalManagedChannelBuilder; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.CreateSubchannelArgs; import io.grpc.LoadBalancer.Helper; @@ -1268,11 +1269,13 @@ public void metricWithRealChannel() throws Exception { .start()); MetricSink metrics = mock(MetricSink.class, delegatesTo(new NoopMetricSink())); Channel channel = grpcCleanupRule.register( - InProcessChannelBuilder.forName(serverName) - .defaultServiceConfig(Collections.singletonMap( - "loadBalancingConfig", Arrays.asList(Collections.singletonMap( - "weighted_round_robin", Collections.emptyMap())))) - .addMetricSink(metrics) + InternalManagedChannelBuilder.addMetricSink( + InProcessChannelBuilder.forName(serverName) + .defaultServiceConfig(Collections.singletonMap( + "loadBalancingConfig", Arrays.asList(Collections.singletonMap( + "weighted_round_robin", Collections.emptyMap())))) + .directExecutor(), + metrics) .directExecutor() .build());