core: change ClientStreamTracer.StreamInfo to a final class with a builder (#5648)

As we are now endorsing the wrapping of ClientStreamTracers by
providing ForwardingClientStreamTracer, there is a need for altering
StreamInfo, especially CallOptions before it's passed onto the
delegate.  A Builder class and a toBuilder() provides a robust way
to copy the rest of the fields.

This is a breaking change for anybody who creates StreamInfo, which is
unlikely in non-test code, because StreamInfo was added as late as
1.20.0.
This commit is contained in:
Kun Zhang 2019-04-30 09:10:56 -07:00 committed by GitHub
parent 44840fe813
commit 973885457f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 184 additions and 101 deletions

View File

@ -16,6 +16,9 @@
package io.grpc; package io.grpc;
import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.base.MoreObjects;
import io.grpc.Grpc; import io.grpc.Grpc;
import javax.annotation.concurrent.ThreadSafe; import javax.annotation.concurrent.ThreadSafe;
@ -85,17 +88,102 @@ public abstract class ClientStreamTracer extends StreamTracer {
/** /**
* Information about a stream. * Information about a stream.
*
* <p>Note this class doesn't override {@code equals()} and {@code hashCode}, as is the case for
* {@link CallOptions}.
*
* @since 1.20.0
*/ */
public abstract static class StreamInfo { @ExperimentalApi("https://github.com/grpc/grpc-java/issues/2861")
public static final class StreamInfo {
private final Attributes transportAttrs;
private final CallOptions callOptions;
StreamInfo(Attributes transportAttrs, CallOptions callOptions) {
this.transportAttrs = checkNotNull(transportAttrs, "transportAttrs");
this.callOptions = checkNotNull(callOptions, "callOptions");
}
/** /**
* Returns the attributes of the transport that this stream was created on. * Returns the attributes of the transport that this stream was created on.
*/ */
@Grpc.TransportAttr @Grpc.TransportAttr
public abstract Attributes getTransportAttrs(); public Attributes getTransportAttrs() {
return transportAttrs;
}
/** /**
* Returns the effective CallOptions of the call. * Returns the effective CallOptions of the call.
*/ */
public abstract CallOptions getCallOptions(); public CallOptions getCallOptions() {
return callOptions;
}
/**
* Converts this StreamInfo into a new Builder.
*
* @since 1.21.0
*/
public Builder toBuilder() {
Builder builder = new Builder();
builder.setTransportAttrs(transportAttrs);
builder.setCallOptions(callOptions);
return builder;
}
/**
* Creates an empty Builder.
*
* @since 1.21.0
*/
public static Builder newBuilder() {
return new Builder();
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("transportAttrs", transportAttrs)
.add("callOptions", callOptions)
.toString();
}
/**
* Builds {@link StreamInfo} objects.
*
* @since 1.21.0
*/
public static final class Builder {
private Attributes transportAttrs = Attributes.EMPTY;
private CallOptions callOptions = CallOptions.DEFAULT;
Builder() {
}
/**
* Sets the attributes of the transport that this stream was created on. This field is
* optional.
*/
@Grpc.TransportAttr
public Builder setTransportAttrs(Attributes transportAttrs) {
this.transportAttrs = checkNotNull(transportAttrs, "transportAttrs cannot be null");
return this;
}
/**
* Sets the effective CallOptions of the call. This field is optional.
*/
public Builder setCallOptions(CallOptions callOptions) {
this.callOptions = checkNotNull(callOptions, "callOptions cannot be null");
return this;
}
/**
* Builds a new StreamInfo.
*/
public StreamInfo build() {
return new StreamInfo(transportAttrs, callOptions);
}
}
} }
} }

View File

@ -53,17 +53,9 @@ public final class StatsTraceContext {
if (factories.isEmpty()) { if (factories.isEmpty()) {
return NOOP; return NOOP;
} }
ClientStreamTracer.StreamInfo info = new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo info =
@Override ClientStreamTracer.StreamInfo.newBuilder()
public Attributes getTransportAttrs() { .setTransportAttrs(transportAttrs).setCallOptions(callOptions).build();
return transportAttrs;
}
@Override
public CallOptions getCallOptions() {
return callOptions;
}
};
// This array will be iterated multiple times per RPC. Use primitive array instead of Collection // This array will be iterated multiple times per RPC. Use primitive array instead of Collection
// so that for-each doesn't create an Iterator every time. // so that for-each doesn't create an Iterator every time.
StreamTracer[] tracers = new StreamTracer[factories.size()]; StreamTracer[] tracers = new StreamTracer[factories.size()];

View File

@ -0,0 +1,70 @@
/*
* Copyright 2019, gRPC Authors All rights reserved.
*
* 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;
import static com.google.common.truth.Truth.assertThat;
import static java.util.concurrent.TimeUnit.MINUTES;
import io.grpc.ClientStreamTracer.StreamInfo;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
/** Unit tests for the embedded classes in {@link ClientStreamTracer}. */
@RunWith(JUnit4.class)
public class ClientStreamTracerTest {
private static final Attributes.Key<String> TRANSPORT_ATTR_KEY =
Attributes.Key.create("transport-attr-key");
private final CallOptions callOptions = CallOptions.DEFAULT.withDeadlineAfter(1, MINUTES);
private final Attributes transportAttrs =
Attributes.newBuilder().set(TRANSPORT_ATTR_KEY, "value").build();
@Test
public void streamInfo_empty() {
StreamInfo info = StreamInfo.newBuilder().build();
assertThat(info.getCallOptions()).isSameInstanceAs(CallOptions.DEFAULT);
assertThat(info.getTransportAttrs()).isSameInstanceAs(Attributes.EMPTY);
}
@Test
public void streamInfo_withInfo() {
StreamInfo info = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
assertThat(info.getCallOptions()).isSameInstanceAs(callOptions);
assertThat(info.getTransportAttrs()).isSameInstanceAs(transportAttrs);
}
@Test
public void streamInfo_noEquality() {
StreamInfo info1 = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
StreamInfo info2 = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
assertThat(info1).isNotSameInstanceAs(info2);
assertThat(info1).isNotEqualTo(info2);
}
@Test
public void streamInfo_toBuilder() {
StreamInfo info1 = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
StreamInfo info2 = info1.toBuilder().build();
assertThat(info2.getCallOptions()).isSameInstanceAs(callOptions);
assertThat(info2.getTransportAttrs()).isSameInstanceAs(transportAttrs);
}
}

View File

@ -107,17 +107,7 @@ public class CensusModulesTest {
private static final CallOptions CALL_OPTIONS = private static final CallOptions CALL_OPTIONS =
CallOptions.DEFAULT.withOption(CUSTOM_OPTION, "customvalue"); CallOptions.DEFAULT.withOption(CUSTOM_OPTION, "customvalue");
private static final ClientStreamTracer.StreamInfo STREAM_INFO = private static final ClientStreamTracer.StreamInfo STREAM_INFO =
new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo.newBuilder().build();
@Override
public Attributes getTransportAttrs() {
return Attributes.EMPTY;
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
private static class StringInputStream extends InputStream { private static class StringInputStream extends InputStream {
final String string; final String string;

View File

@ -41,8 +41,6 @@ import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.MoreExecutors;
import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ClientStreamTracer; import io.grpc.ClientStreamTracer;
import io.grpc.Codec; import io.grpc.Codec;
import io.grpc.Compressor; import io.grpc.Compressor;
@ -92,17 +90,7 @@ public class RetriableStreamTest {
private static final double BACKOFF_MULTIPLIER = 2D; private static final double BACKOFF_MULTIPLIER = 2D;
private static final double FAKE_RANDOM = .5D; private static final double FAKE_RANDOM = .5D;
private static final ClientStreamTracer.StreamInfo STREAM_INFO = private static final ClientStreamTracer.StreamInfo STREAM_INFO =
new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo.newBuilder().build();
@Override
public Attributes getTransportAttrs() {
return Attributes.EMPTY;
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
static { static {
RetriableStream.setRandom( RetriableStream.setRandom(

View File

@ -49,7 +49,6 @@ import com.google.protobuf.ByteString;
import com.google.protobuf.util.Durations; import com.google.protobuf.util.Durations;
import com.google.protobuf.util.Timestamps; import com.google.protobuf.util.Timestamps;
import io.grpc.Attributes; import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ChannelLogger; import io.grpc.ChannelLogger;
import io.grpc.ClientStreamTracer; import io.grpc.ClientStreamTracer;
import io.grpc.ConnectivityState; import io.grpc.ConnectivityState;
@ -187,17 +186,7 @@ public class GrpclbLoadBalancerTest {
} }
}); });
private static final ClientStreamTracer.StreamInfo STREAM_INFO = private static final ClientStreamTracer.StreamInfo STREAM_INFO =
new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo.newBuilder().build();
@Override
public Attributes getTransportAttrs() {
return Attributes.EMPTY;
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
private io.grpc.Server fakeLbServer; private io.grpc.Server fakeLbServer;
@Captor @Captor

View File

@ -23,7 +23,6 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import io.grpc.Attributes; import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ClientStreamTracer; import io.grpc.ClientStreamTracer;
import io.grpc.Metadata; import io.grpc.Metadata;
import io.grpc.internal.GrpcAttributes; import io.grpc.internal.GrpcAttributes;
@ -50,20 +49,12 @@ public class TokenAttachingTracerFactoryTest {
@Test @Test
public void hasToken() { public void hasToken() {
TokenAttachingTracerFactory factory = new TokenAttachingTracerFactory(delegate); TokenAttachingTracerFactory factory = new TokenAttachingTracerFactory(delegate);
ClientStreamTracer.StreamInfo info = new ClientStreamTracer.StreamInfo() { Attributes eagAttrs = Attributes.newBuilder()
@Override .set(GrpclbConstants.TOKEN_ATTRIBUTE_KEY, "token0001").build();
public Attributes getTransportAttrs() { ClientStreamTracer.StreamInfo info = ClientStreamTracer.StreamInfo.newBuilder()
Attributes eagAttrs = Attributes.newBuilder() .setTransportAttrs(
.set(GrpclbConstants.TOKEN_ATTRIBUTE_KEY, "token0001").build(); Attributes.newBuilder().set(GrpcAttributes.ATTR_CLIENT_EAG_ATTRS, eagAttrs).build())
return Attributes.newBuilder() .build();
.set(GrpcAttributes.ATTR_CLIENT_EAG_ATTRS, eagAttrs).build();
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
Metadata headers = new Metadata(); Metadata headers = new Metadata();
// Preexisting token should be replaced // Preexisting token should be replaced
headers.put(GrpclbConstants.TOKEN_METADATA_KEY, "preexisting-token"); headers.put(GrpclbConstants.TOKEN_METADATA_KEY, "preexisting-token");
@ -77,18 +68,11 @@ public class TokenAttachingTracerFactoryTest {
@Test @Test
public void noToken() { public void noToken() {
TokenAttachingTracerFactory factory = new TokenAttachingTracerFactory(delegate); TokenAttachingTracerFactory factory = new TokenAttachingTracerFactory(delegate);
ClientStreamTracer.StreamInfo info = new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo info = ClientStreamTracer.StreamInfo.newBuilder()
@Override .setTransportAttrs(
public Attributes getTransportAttrs() { Attributes.newBuilder()
return Attributes.newBuilder() .set(GrpcAttributes.ATTR_CLIENT_EAG_ATTRS, Attributes.EMPTY).build())
.set(GrpcAttributes.ATTR_CLIENT_EAG_ATTRS, Attributes.EMPTY).build(); .build();
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
Metadata headers = new Metadata(); Metadata headers = new Metadata();
// Preexisting token should be removed // Preexisting token should be removed
@ -103,18 +87,12 @@ public class TokenAttachingTracerFactoryTest {
@Test @Test
public void nullDelegate() { public void nullDelegate() {
TokenAttachingTracerFactory factory = new TokenAttachingTracerFactory(null); TokenAttachingTracerFactory factory = new TokenAttachingTracerFactory(null);
ClientStreamTracer.StreamInfo info = new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo info = ClientStreamTracer.StreamInfo.newBuilder()
@Override .setTransportAttrs(
public Attributes getTransportAttrs() { Attributes.newBuilder()
return Attributes.newBuilder() .set(GrpcAttributes.ATTR_CLIENT_EAG_ATTRS, Attributes.EMPTY).build())
.set(GrpcAttributes.ATTR_CLIENT_EAG_ATTRS, Attributes.EMPTY).build(); .build();
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
Metadata headers = new Metadata(); Metadata headers = new Metadata();
ClientStreamTracer tracer = factory.newClientStreamTracer(info, headers); ClientStreamTracer tracer = factory.newClientStreamTracer(info, headers);

View File

@ -28,8 +28,6 @@ import io.envoyproxy.envoy.api.v2.core.Locality;
import io.envoyproxy.envoy.api.v2.endpoint.ClusterStats; import io.envoyproxy.envoy.api.v2.endpoint.ClusterStats;
import io.envoyproxy.envoy.api.v2.endpoint.ClusterStats.DroppedRequests; import io.envoyproxy.envoy.api.v2.endpoint.ClusterStats.DroppedRequests;
import io.envoyproxy.envoy.api.v2.endpoint.UpstreamLocalityStats; import io.envoyproxy.envoy.api.v2.endpoint.UpstreamLocalityStats;
import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ClientStreamTracer; import io.grpc.ClientStreamTracer;
import io.grpc.LoadBalancer.PickResult; import io.grpc.LoadBalancer.PickResult;
import io.grpc.LoadBalancer.Subchannel; import io.grpc.LoadBalancer.Subchannel;
@ -58,17 +56,7 @@ import org.mockito.MockitoAnnotations;
public class XdsLoadReportStoreTest { public class XdsLoadReportStoreTest {
private static final String SERVICE_NAME = "api.google.com"; private static final String SERVICE_NAME = "api.google.com";
private static final ClientStreamTracer.StreamInfo STREAM_INFO = private static final ClientStreamTracer.StreamInfo STREAM_INFO =
new ClientStreamTracer.StreamInfo() { ClientStreamTracer.StreamInfo.newBuilder().build();
@Override
public Attributes getTransportAttrs() {
return Attributes.EMPTY;
}
@Override
public CallOptions getCallOptions() {
return CallOptions.DEFAULT;
}
};
private static final Locality TEST_LOCALITY = Locality.newBuilder() private static final Locality TEST_LOCALITY = Locality.newBuilder()
.setRegion("test_region") .setRegion("test_region")
.setZone("test_zone") .setZone("test_zone")