Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: remove deprecated StreamInfo.transportAttrs #8768

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 2 additions & 37 deletions api/src/main/java/io/grpc/ClientStreamTracer.java
Expand Up @@ -81,10 +81,6 @@ public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata header
}
}

/** An abstract class for internal use only. */
@Internal
public abstract static class InternalLimitedInfoFactory extends Factory {}

/**
* Information about a stream.
*
Expand All @@ -95,32 +91,17 @@ public abstract static class InternalLimitedInfoFactory extends Factory {}
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/2861")
public static final class StreamInfo {
private final Attributes transportAttrs;
private final CallOptions callOptions;
private final int previousAttempts;
private final boolean isTransparentRetry;

StreamInfo(
Attributes transportAttrs, CallOptions callOptions, int previousAttempts,
boolean isTransparentRetry) {
this.transportAttrs = checkNotNull(transportAttrs, "transportAttrs");
CallOptions callOptions, int previousAttempts, boolean isTransparentRetry) {
this.callOptions = checkNotNull(callOptions, "callOptions");
this.previousAttempts = previousAttempts;
this.isTransparentRetry = isTransparentRetry;
}

/**
* Returns the attributes of the transport that this stream was created on.
*
* @deprecated Use {@link ClientStreamTracer#streamCreated(Attributes, Metadata)} to handle
* the transport Attributes instead.
*/
@Deprecated
@Grpc.TransportAttr
public Attributes getTransportAttrs() {
return transportAttrs;
}

/**
* Returns the effective CallOptions of the call.
*/
Expand Down Expand Up @@ -154,7 +135,6 @@ public boolean isTransparentRetry() {
public Builder toBuilder() {
return new Builder()
.setCallOptions(callOptions)
.setTransportAttrs(transportAttrs)
.setPreviousAttempts(previousAttempts)
.setIsTransparentRetry(isTransparentRetry);
}
Expand All @@ -171,7 +151,6 @@ public static Builder newBuilder() {
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("transportAttrs", transportAttrs)
.add("callOptions", callOptions)
.add("previousAttempts", previousAttempts)
.add("isTransparentRetry", isTransparentRetry)
Expand All @@ -184,27 +163,13 @@ public String toString() {
* @since 1.21.0
*/
public static final class Builder {
private Attributes transportAttrs = Attributes.EMPTY;
private CallOptions callOptions = CallOptions.DEFAULT;
private int previousAttempts;
private boolean isTransparentRetry;

Builder() {
}

/**
* Sets the attributes of the transport that this stream was created on. This field is
* optional.
*
* @deprecated Use {@link ClientStreamTracer#streamCreated(Attributes, Metadata)} to handle
* the transport Attributes instead.
*/
@Deprecated
public Builder setTransportAttrs(@Grpc.TransportAttr Attributes transportAttrs) {
this.transportAttrs = checkNotNull(transportAttrs, "transportAttrs cannot be null");
return this;
}

/**
* Sets the effective CallOptions of the call. This field is optional.
*/
Expand Down Expand Up @@ -237,7 +202,7 @@ public Builder setIsTransparentRetry(boolean isTransparentRetry) {
* Builds a new StreamInfo.
*/
public StreamInfo build() {
return new StreamInfo(transportAttrs, callOptions, previousAttempts, isTransparentRetry);
return new StreamInfo(callOptions, previousAttempts, isTransparentRetry);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/test/java/io/grpc/CallOptionsTest.java
Expand Up @@ -272,7 +272,7 @@ public void increment(long period, TimeUnit unit) {
}
}

private static class FakeTracerFactory extends ClientStreamTracer.InternalLimitedInfoFactory {
private static class FakeTracerFactory extends ClientStreamTracer.Factory {
final String name;

FakeTracerFactory(String name) {
Expand Down
2 changes: 1 addition & 1 deletion census/src/main/java/io/grpc/census/CensusStatsModule.java
Expand Up @@ -397,7 +397,7 @@ void recordFinishedAttempt() {

@VisibleForTesting
static final class CallAttemptsTracerFactory extends
ClientStreamTracer.InternalLimitedInfoFactory {
ClientStreamTracer.Factory {
static final MeasureLong RETRIES_PER_CALL =
Measure.MeasureLong.create(
"grpc.io/client/retries_per_call", "Number of retries per call", "1");
Expand Down
Expand Up @@ -226,7 +226,7 @@ private static void recordMessageEvent(
}

@VisibleForTesting
final class CallAttemptsTracerFactory extends ClientStreamTracer.InternalLimitedInfoFactory {
final class CallAttemptsTracerFactory extends ClientStreamTracer.Factory {
volatile int callEnded;

private final boolean isSampledToLocalTracing;
Expand Down
53 changes: 2 additions & 51 deletions core/src/main/java/io/grpc/internal/GrpcUtil.java
Expand Up @@ -27,10 +27,8 @@
import com.google.common.base.Supplier;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ClientStreamTracer;
import io.grpc.ClientStreamTracer.InternalLimitedInfoFactory;
import io.grpc.ClientStreamTracer.StreamInfo;
import io.grpc.InternalChannelz.SocketStats;
import io.grpc.InternalLogId;
Expand Down Expand Up @@ -725,7 +723,7 @@ public ClientStream newStream(
ClientStreamTracer[] tracers) {
StreamInfo info = StreamInfo.newBuilder().setCallOptions(callOptions).build();
ClientStreamTracer streamTracer =
newClientStreamTracer(streamTracerFactory, info, headers);
streamTracerFactory.newClientStreamTracer(info, headers);
checkState(tracers[tracers.length - 1] == NOOP_TRACER, "lb tracer already assigned");
tracers[tracers.length - 1] = streamTracer;
return transport.newStream(method, headers, callOptions, tracers);
Expand Down Expand Up @@ -769,61 +767,14 @@ public static ClientStreamTracer[] getClientStreamTracers(
.setIsTransparentRetry(isTransparentRetry)
.build();
for (int i = 0; i < factories.size(); i++) {
tracers[i] = newClientStreamTracer(factories.get(i), streamInfo, headers);
tracers[i] = factories.get(i).newClientStreamTracer(streamInfo, headers);
}
// Reserved to be set later by the lb as per the API contract of ClientTransport.newStream().
// See also GrpcUtil.getTransportFromPickResult()
tracers[tracers.length - 1] = NOOP_TRACER;
return tracers;
}

// A util function for backward compatibility to support deprecated StreamInfo.getAttributes().
@VisibleForTesting
static ClientStreamTracer newClientStreamTracer(
final ClientStreamTracer.Factory streamTracerFactory, final StreamInfo info,
final Metadata headers) {
ClientStreamTracer streamTracer;
if (streamTracerFactory instanceof InternalLimitedInfoFactory) {
streamTracer = streamTracerFactory.newClientStreamTracer(info, headers);
} else {
streamTracer = new ForwardingClientStreamTracer() {
final ClientStreamTracer noop = new ClientStreamTracer() {};
volatile ClientStreamTracer delegate = noop;

void maybeInit(StreamInfo info, Metadata headers) {
if (delegate != noop) {
return;
}
synchronized (this) {
if (delegate == noop) {
delegate = streamTracerFactory.newClientStreamTracer(info, headers);
}
}
}

@Override
protected ClientStreamTracer delegate() {
return delegate;
}

@SuppressWarnings("deprecation")
@Override
public void streamCreated(Attributes transportAttrs, Metadata headers) {
StreamInfo streamInfo = info.toBuilder().setTransportAttrs(transportAttrs).build();
maybeInit(streamInfo, headers);
delegate().streamCreated(transportAttrs, headers);
}

@Override
public void streamClosed(Status status) {
maybeInit(info, headers);
delegate().streamClosed(status);
}
};
}
return streamTracer;
}

/** Quietly closes all messages in MessageProducer. */
static void closeQuietly(MessageProducer producer) {
InputStream message;
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/internal/RetriableStream.java
Expand Up @@ -221,7 +221,7 @@ private Substream createSubstream(int previousAttemptCount, boolean isTransparen
Substream sub = new Substream(previousAttemptCount);
// one tracer per substream
final ClientStreamTracer bufferSizeTracer = new BufferSizeTracer(sub);
ClientStreamTracer.Factory tracerFactory = new ClientStreamTracer.InternalLimitedInfoFactory() {
ClientStreamTracer.Factory tracerFactory = new ClientStreamTracer.Factory() {
@Override
public ClientStreamTracer newClientStreamTracer(
ClientStreamTracer.StreamInfo info, Metadata headers) {
Expand Down
22 changes: 4 additions & 18 deletions core/src/test/java/io/grpc/ClientStreamTracerTest.java
Expand Up @@ -27,48 +27,34 @@
/** 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
@SuppressWarnings("deprecation") // info.getTransportAttrs()
public void streamInfo_empty() {
StreamInfo info = StreamInfo.newBuilder().build();
assertThat(info.getCallOptions()).isSameInstanceAs(CallOptions.DEFAULT);
assertThat(info.getTransportAttrs()).isSameInstanceAs(Attributes.EMPTY);
}

@Test
@SuppressWarnings("deprecation") // info.getTransportAttrs()
public void streamInfo_withInfo() {
StreamInfo info = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
StreamInfo info = StreamInfo.newBuilder().setCallOptions(callOptions).build();
assertThat(info.getCallOptions()).isSameInstanceAs(callOptions);
assertThat(info.getTransportAttrs()).isSameInstanceAs(transportAttrs);
}

@Test
@SuppressWarnings("deprecation") // info.setTransportAttrs()
public void streamInfo_noEquality() {
StreamInfo info1 = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
StreamInfo info2 = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
StreamInfo info1 = StreamInfo.newBuilder().setCallOptions(callOptions).build();
StreamInfo info2 = StreamInfo.newBuilder().setCallOptions(callOptions).build();

assertThat(info1).isNotSameInstanceAs(info2);
assertThat(info1).isNotEqualTo(info2);
}

@Test
@SuppressWarnings("deprecation") // info.getTransportAttrs()
public void streamInfo_toBuilder() {
StreamInfo info1 = StreamInfo.newBuilder()
.setCallOptions(callOptions).setTransportAttrs(transportAttrs).build();
.setCallOptions(callOptions).build();
StreamInfo info2 = info1.toBuilder().build();
assertThat(info2.getCallOptions()).isSameInstanceAs(callOptions);
assertThat(info2.getTransportAttrs()).isSameInstanceAs(transportAttrs);
}
}
38 changes: 0 additions & 38 deletions core/src/test/java/io/grpc/internal/GrpcUtilTest.java
Expand Up @@ -16,7 +16,6 @@

package io.grpc.internal;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand All @@ -28,18 +27,14 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

import io.grpc.Attributes;
import io.grpc.CallOptions;
import io.grpc.ClientStreamTracer;
import io.grpc.ClientStreamTracer.StreamInfo;
import io.grpc.LoadBalancer.PickResult;
import io.grpc.Metadata;
import io.grpc.Status;
import io.grpc.internal.ClientStreamListener.RpcProgress;
import io.grpc.internal.GrpcUtil.Http2Error;
import io.grpc.testing.TestMethodDescriptors;
import java.util.ArrayDeque;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
Expand Down Expand Up @@ -296,37 +291,4 @@ public void getTransportFromPickResult_dropPickResult_failFast() {

verify(listener).closed(eq(status), eq(RpcProgress.DROPPED), any(Metadata.class));
}

@Test
public void clientStreamTracerFactoryBackwardCompatibility() {
final AtomicReference<Attributes> transportAttrsRef = new AtomicReference<>();
final ClientStreamTracer mockTracer = mock(ClientStreamTracer.class);
final Metadata.Key<String> key = Metadata.Key.of("fake-key", Metadata.ASCII_STRING_MARSHALLER);
final ArrayDeque<ClientStreamTracer> tracers = new ArrayDeque<>();
ClientStreamTracer.Factory oldFactoryImpl = new ClientStreamTracer.Factory() {
@SuppressWarnings("deprecation")
@Override
public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) {
transportAttrsRef.set(info.getTransportAttrs());
headers.put(key, "fake-value");
tracers.offer(mockTracer);
return mockTracer;
}
};

StreamInfo info =
StreamInfo.newBuilder().setCallOptions(CallOptions.DEFAULT.withWaitForReady()).build();
Metadata metadata = new Metadata();
Attributes transAttrs =
Attributes.newBuilder().set(Attributes.Key.<String>create("foo"), "bar").build();
ClientStreamTracer tracer = GrpcUtil.newClientStreamTracer(oldFactoryImpl, info, metadata);
tracer.streamCreated(transAttrs, metadata);
assertThat(tracers.poll()).isSameInstanceAs(mockTracer);
assertThat(transportAttrsRef.get()).isEqualTo(transAttrs);
assertThat(metadata.get(key)).isEqualTo("fake-value");

tracer.streamClosed(Status.UNAVAILABLE);
// verify that newClientStreamTracer() is called no more than once
assertThat(tracers).isEmpty();
}
}
Expand Up @@ -2447,13 +2447,13 @@ public void pickerReturnsStreamTracer_noDelay() {
ClientStream mockStream = mock(ClientStream.class);
final ClientStreamTracer tracer1 = new ClientStreamTracer() {};
final ClientStreamTracer tracer2 = new ClientStreamTracer() {};
ClientStreamTracer.Factory factory1 = new ClientStreamTracer.InternalLimitedInfoFactory() {
ClientStreamTracer.Factory factory1 = new ClientStreamTracer.Factory() {
@Override
public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) {
return tracer1;
}
};
ClientStreamTracer.Factory factory2 = new ClientStreamTracer.InternalLimitedInfoFactory() {
ClientStreamTracer.Factory factory2 = new ClientStreamTracer.Factory() {
@Override
public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) {
return tracer2;
Expand Down Expand Up @@ -2491,13 +2491,13 @@ public void pickerReturnsStreamTracer_delayed() {
ClientStream mockStream = mock(ClientStream.class);
final ClientStreamTracer tracer1 = new ClientStreamTracer() {};
final ClientStreamTracer tracer2 = new ClientStreamTracer() {};
ClientStreamTracer.Factory factory1 = new ClientStreamTracer.InternalLimitedInfoFactory() {
ClientStreamTracer.Factory factory1 = new ClientStreamTracer.Factory() {
@Override
public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) {
return tracer1;
}
};
ClientStreamTracer.Factory factory2 = new ClientStreamTracer.InternalLimitedInfoFactory() {
ClientStreamTracer.Factory factory2 = new ClientStreamTracer.Factory() {
@Override
public ClientStreamTracer newClientStreamTracer(StreamInfo info, Metadata headers) {
return tracer2;
Expand Down
Expand Up @@ -37,7 +37,7 @@
* span of an LB stream with the remote load-balancer.
*/
@ThreadSafe
final class GrpclbClientLoadRecorder extends ClientStreamTracer.InternalLimitedInfoFactory {
final class GrpclbClientLoadRecorder extends ClientStreamTracer.Factory {

private static final AtomicLongFieldUpdater<GrpclbClientLoadRecorder> callsStartedUpdater =
AtomicLongFieldUpdater.newUpdater(GrpclbClientLoadRecorder.class, "callsStarted");
Expand Down
Expand Up @@ -30,7 +30,7 @@
* Wraps a {@link ClientStreamTracer.Factory}, retrieves tokens from transport attributes and
* attaches them to headers. This is only used in the PICK_FIRST mode.
*/
final class TokenAttachingTracerFactory extends ClientStreamTracer.InternalLimitedInfoFactory {
final class TokenAttachingTracerFactory extends ClientStreamTracer.Factory {
private static final ClientStreamTracer NOOP_TRACER = new ClientStreamTracer() {};

@Nullable
Expand Down
Expand Up @@ -49,7 +49,7 @@ public void streamCreated(Attributes transportAttrs, Metadata headers) {
private final ClientStreamTracer.Factory delegate = mock(
ClientStreamTracer.Factory.class,
delegatesTo(
new ClientStreamTracer.InternalLimitedInfoFactory() {
new ClientStreamTracer.Factory() {
@Override
public ClientStreamTracer newClientStreamTracer(
ClientStreamTracer.StreamInfo info, Metadata headers) {
Expand Down