services: socket options were erroneously ignored if socket has no stats (#4336)

This fixes listen sockets. It is ok to have no data but report socket
options.
This commit is contained in:
zpencer 2018-04-13 14:59:35 -07:00 committed by GitHub
parent e4502aca5d
commit 791a29f63b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 89 additions and 60 deletions

View File

@ -468,9 +468,9 @@ public final class Channelz {
SocketOptions socketOptions,
Security security) {
this.data = data;
this.local = local;
this.local = Preconditions.checkNotNull(local, "local socket");
this.remote = remote;
this.socketOptions = socketOptions;
this.socketOptions = Preconditions.checkNotNull(socketOptions);
this.security = security;
}
}

View File

@ -89,6 +89,7 @@ import java.io.InputStreamReader;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketAddress;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@ -1982,6 +1983,11 @@ public class OkHttpClientTransportTest {
public synchronized void close() {
frameReader.nextFrameAtEndOfStream();
}
@Override
public SocketAddress getLocalSocketAddress() {
return InetSocketAddress.createUnresolved("localhost", 4000);
}
}
static class PingCallbackImpl implements ClientTransport.PingCallback {

View File

@ -137,9 +137,7 @@ final class ChannelzProtoUtil {
if (socketStats.remote != null) {
builder.setRemote(toAddress(socketStats.remote));
}
if (socketStats.data != null) {
builder.setData(extractSocketData(socketStats));
}
builder.setData(extractSocketData(socketStats));
return builder.build();
}
@ -168,29 +166,31 @@ final class ChannelzProtoUtil {
}
static SocketData extractSocketData(SocketStats socketStats) {
TransportStats s = socketStats.data;
return SocketData
.newBuilder()
.setStreamsStarted(s.streamsStarted)
.setStreamsSucceeded(s.streamsSucceeded)
.setStreamsFailed(s.streamsFailed)
.setMessagesSent(s.messagesSent)
.setMessagesReceived(s.messagesReceived)
.setKeepAlivesSent(s.keepAlivesSent)
.setLastLocalStreamCreatedTimestamp(
Timestamps.fromNanos(s.lastLocalStreamCreatedTimeNanos))
.setLastRemoteStreamCreatedTimestamp(
Timestamps.fromNanos(s.lastRemoteStreamCreatedTimeNanos))
.setLastMessageSentTimestamp(
Timestamps.fromNanos(s.lastMessageSentTimeNanos))
.setLastMessageReceivedTimestamp(
Timestamps.fromNanos(s.lastMessageReceivedTimeNanos))
.setLocalFlowControlWindow(
Int64Value.newBuilder().setValue(s.localFlowControlWindow).build())
.setRemoteFlowControlWindow(
Int64Value.newBuilder().setValue(s.remoteFlowControlWindow).build())
.addAllOption(toSocketOptionsList(socketStats.socketOptions))
.build();
SocketData.Builder builder = SocketData.newBuilder();
if (socketStats.data != null) {
TransportStats s = socketStats.data;
builder
.setStreamsStarted(s.streamsStarted)
.setStreamsSucceeded(s.streamsSucceeded)
.setStreamsFailed(s.streamsFailed)
.setMessagesSent(s.messagesSent)
.setMessagesReceived(s.messagesReceived)
.setKeepAlivesSent(s.keepAlivesSent)
.setLastLocalStreamCreatedTimestamp(
Timestamps.fromNanos(s.lastLocalStreamCreatedTimeNanos))
.setLastRemoteStreamCreatedTimestamp(
Timestamps.fromNanos(s.lastRemoteStreamCreatedTimeNanos))
.setLastMessageSentTimestamp(
Timestamps.fromNanos(s.lastMessageSentTimeNanos))
.setLastMessageReceivedTimestamp(
Timestamps.fromNanos(s.lastMessageReceivedTimeNanos))
.setLocalFlowControlWindow(
Int64Value.of(s.localFlowControlWindow))
.setRemoteFlowControlWindow(
Int64Value.of(s.remoteFlowControlWindow));
}
builder.addAllOption(toSocketOptionsList(socketStats.socketOptions));
return builder.build();
}
public static final String SO_LINGER = "SO_LINGER";

View File

@ -21,6 +21,7 @@ import static io.grpc.internal.Channelz.id;
import static org.junit.Assert.assertEquals;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.protobuf.Any;
import com.google.protobuf.ByteString;
import com.google.protobuf.Int64Value;
@ -256,8 +257,7 @@ public final class ChannelzProtoUtilTest {
.newBuilder()
.setIpAddress(ByteString.copyFrom(
((InetSocketAddress) listenSocket.listenAddress).getAddress().getAddress()))
.setPort(1234)
.build())
.setPort(1234))
.build();
private final TestSocket socket = new TestSocket();
@ -266,7 +266,7 @@ public final class ChannelzProtoUtilTest {
.setName(socket.toString())
.setSocketId(socket.getLogId().getId())
.build();
private final SocketData socketDataNoSockOpts = SocketData
private final SocketData socketDataWithDataNoSockOpts = SocketData
.newBuilder()
.setStreamsStarted(1)
.setLastLocalStreamCreatedTimestamp(Timestamps.fromNanos(2))
@ -278,8 +278,8 @@ public final class ChannelzProtoUtilTest {
.setKeepAlivesSent(8)
.setLastMessageSentTimestamp(Timestamps.fromNanos(9))
.setLastMessageReceivedTimestamp(Timestamps.fromNanos(10))
.setLocalFlowControlWindow(Int64Value.newBuilder().setValue(11).build())
.setRemoteFlowControlWindow(Int64Value.newBuilder().setValue(12).build())
.setLocalFlowControlWindow(Int64Value.newBuilder().setValue(11))
.setRemoteFlowControlWindow(Int64Value.newBuilder().setValue(12))
.build();
private final Address localAddress = Address
.newBuilder()
@ -288,8 +288,7 @@ public final class ChannelzProtoUtilTest {
.newBuilder()
.setIpAddress(ByteString.copyFrom(
((InetSocketAddress) socket.local).getAddress().getAddress()))
.setPort(1000)
.build())
.setPort(1000))
.build();
private final Address remoteAddress = Address
.newBuilder()
@ -298,8 +297,7 @@ public final class ChannelzProtoUtilTest {
.newBuilder()
.setIpAddress(ByteString.copyFrom(
((InetSocketAddress) socket.remote).getAddress().getAddress()))
.setPort(1000)
.build())
.setPort(1000))
.build();
@Test
@ -333,14 +331,53 @@ public final class ChannelzProtoUtilTest {
}
@Test
public void toSocket() throws Exception {
public void toSocket_withDataNoOptions() throws Exception {
assertEquals(
Socket
.newBuilder()
.setRef(socketRef)
.setLocal(localAddress)
.setRemote(remoteAddress)
.setData(socketDataNoSockOpts)
.setData(socketDataWithDataNoSockOpts)
.build(),
ChannelzProtoUtil.toSocket(socket));
}
@Test
public void toSocket_noDataWithOptions() throws Exception {
assertEquals(
Socket
.newBuilder()
.setRef(listenSocketRef)
.setLocal(listenAddress)
.setData(
SocketData
.newBuilder()
.addOption(
SocketOption
.newBuilder()
.setName("listen_option")
.setValue("listen_option_value")))
.build(),
ChannelzProtoUtil.toSocket(listenSocket));
}
@Test
public void toSocket_withDataWithOptions() throws Exception {
socket.socketOptions
= new SocketOptions(null, null, null, ImmutableMap.of("test_name", "test_value"));
assertEquals(
Socket
.newBuilder()
.setRef(socketRef)
.setLocal(localAddress)
.setRemote(remoteAddress)
.setData(
SocketData
.newBuilder(socketDataWithDataNoSockOpts)
.addOption(
SocketOption.newBuilder()
.setName("test_name").setValue("test_value")))
.build(),
ChannelzProtoUtil.toSocket(socket));
}
@ -349,7 +386,7 @@ public final class ChannelzProtoUtilTest {
public void extractSocketData() throws Exception {
// no options
assertEquals(
socketDataNoSockOpts,
socketDataWithDataNoSockOpts,
ChannelzProtoUtil.extractSocketData(socket.getStats().get()));
// with options
@ -358,7 +395,7 @@ public final class ChannelzProtoUtilTest {
.setTcpInfo(channelzTcpInfo)
.build();
assertEquals(
socketDataNoSockOpts
socketDataWithDataNoSockOpts
.toBuilder()
.addOption(sockOptlinger10s)
.addOption(socketOptionTcpInfo)
@ -366,21 +403,10 @@ public final class ChannelzProtoUtilTest {
ChannelzProtoUtil.extractSocketData(socket.getStats().get()));
}
@Test
public void toSocket_listenSocket() {
assertEquals(
Socket
.newBuilder()
.setRef(listenSocketRef)
.setLocal(listenAddress)
.build(),
ChannelzProtoUtil.toSocket(listenSocket));
}
@Test
public void toSocketData() throws Exception {
assertEquals(
socketDataNoSockOpts
socketDataWithDataNoSockOpts
.toBuilder()
.build(),
ChannelzProtoUtil.extractSocketData(socket.getStats().get()));
@ -394,8 +420,7 @@ public final class ChannelzProtoUtilTest {
TcpIpAddress
.newBuilder()
.setIpAddress(ByteString.copyFrom(inet4.getAddress().getAddress()))
.setPort(1000)
.build())
.setPort(1000))
.build(),
ChannelzProtoUtil.toAddress(inet4));
}
@ -408,8 +433,7 @@ public final class ChannelzProtoUtilTest {
Address.newBuilder().setUdsAddress(
UdsAddress
.newBuilder()
.setFilename(path)
.build())
.setFilename(path))
.build(),
ChannelzProtoUtil.toAddress(uds));
}
@ -427,8 +451,7 @@ public final class ChannelzProtoUtilTest {
Address.newBuilder().setOtherAddress(
OtherAddress
.newBuilder()
.setName(name)
.build())
.setName(name))
.build(),
ChannelzProtoUtil.toAddress(other));
}

View File

@ -89,7 +89,7 @@ final class ChannelzTestHelper {
/*data=*/ null,
listenAddress,
/*remote=*/ null,
new SocketOptions.Builder().build(),
new SocketOptions.Builder().addOption("listen_option", "listen_option_value").build(),
/*security=*/ null));
return ret;
}