introspection: Add inactive connections to output (#686)

Tried to debug a stuck connection that was stuck in "startClose", but
unfortunately the introspection output was missing the connection as it
expects connections to be added as part of root peers, but peers only
store references to active connections.

Output inactive connections at the top level of the channel (which is
the only thing that has references to inactive connections).

introspection.go

@@ -78,6 +78,10 @@ type RuntimeState struct {
 	// Connections is the list of connection IDs in the channel
 	Connections []uint32 ` json:"connections"`
 
+	// InactiveConnections is the connection state for connections that are not active,
+	// and hence are not reported as part of root peers.
+	InactiveConnections []ConnectionRuntimeState `json:"inactiveConnections"`
+
 	// OtherChannels is information about any other channels running in this process.
 	OtherChannels map[string][]ChannelInfo `json:"otherChannels,omitEmpty"`
 
@@ -206,24 +210,29 @@ func (ch *Channel) IntrospectState(opts *IntrospectionOptions) *RuntimeState {
 
 	ch.mutable.RLock()
 	numConns := len(ch.mutable.conns)
+	inactiveConns := make([]*Connection, 0, numConns)
 	connIDs := make([]uint32, 0, numConns)
-	for id := range ch.mutable.conns {
+	for id, conn := range ch.mutable.conns {
 		connIDs = append(connIDs, id)
+		if !conn.IsActive() {
+			inactiveConns = append(inactiveConns, conn)
+		}
 	}
 
 	ch.mutable.RUnlock()
 
 	return &RuntimeState{
-		ID:             ch.chID,
-		CreatedStack:   ch.createdStack,
-		LocalPeer:      ch.PeerInfo(),
-		SubChannels:    ch.subChannels.IntrospectState(opts),
-		RootPeers:      ch.RootPeers().IntrospectState(opts),
-		Peers:          ch.Peers().IntrospectList(opts),
-		NumConnections: numConns,
-		Connections:    connIDs,
-		OtherChannels:  ch.IntrospectOthers(opts),
-		RuntimeVersion: introspectRuntimeVersion(),
+		ID:                  ch.chID,
+		CreatedStack:        ch.createdStack,
+		LocalPeer:           ch.PeerInfo(),
+		SubChannels:         ch.subChannels.IntrospectState(opts),
+		RootPeers:           ch.RootPeers().IntrospectState(opts),
+		Peers:               ch.Peers().IntrospectList(opts),
+		NumConnections:      numConns,
+		Connections:         connIDs,
+		InactiveConnections: getConnectionRuntimeState(inactiveConns, opts),
+		OtherChannels:       ch.IntrospectOthers(opts),
+		RuntimeVersion:      introspectRuntimeVersion(),
 	}
 }
 

introspection_test.go

@@ -68,16 +68,44 @@ func TestIntrospection(t *testing.T) {
 	})
 }
 
-func TestIntrospectNumConnections(t *testing.T) {
+func TestIntrospectClosedConn(t *testing.T) {
 	// Disable the relay, since the relay does not maintain a 1:1 mapping betewen
 	// incoming connections vs outgoing connections.
 	opts := testutils.NewOpts().NoRelay()
 	testutils.WithTestServer(t, opts, func(ts *testutils.TestServer) {
+		blockEcho := make(chan struct{})
+		gotEcho := make(chan struct{})
+		testutils.RegisterEcho(ts.Server(), func() {
+			close(gotEcho)
+			<-blockEcho
+		})
+
 		ctx, cancel := NewContext(time.Second)
 		defer cancel()
 
 		assert.Equal(t, 0, ts.Server().IntrospectNumConnections(), "Expected no connection on new server")
 
+		// Make sure that a closed connection will reduce NumConnections.
+		client := ts.NewClient(nil)
+		require.NoError(t, client.Ping(ctx, ts.HostPort()), "Ping from new client failed")
+		assert.Equal(t, 1, ts.Server().IntrospectNumConnections(), "Number of connections expected to increase")
+
+		go testutils.AssertEcho(t, client, ts.HostPort(), ts.ServiceName())
+
+		// The state will change to "closeStarted", but be blocked due to the blocked
+		// echo call.
+		<-gotEcho
+		client.Close()
+
+		introspected := client.IntrospectState(nil)
+		assert.Len(t, introspected.Connections, 1, "Expected single connection due to blocked call")
+		assert.Len(t, introspected.InactiveConnections, 1, "Expected inactive connection due to blocked call")
+
+		close(blockEcho)
+		require.True(t, testutils.WaitFor(100*time.Millisecond, func() bool {
+			return ts.Server().IntrospectNumConnections() == 0
+		}), "Closed connection did not get removed, num connections is %v", ts.Server().IntrospectNumConnections())
+
 		for i := 0; i < 10; i++ {
 			client := ts.NewClient(nil)
 			defer client.Close()
@@ -86,15 +114,5 @@ func TestIntrospectNumConnections(t *testing.T) {
 			assert.Equal(t, 1, client.IntrospectNumConnections(), "Client should have single connection")
 			assert.Equal(t, i+1, ts.Server().IntrospectNumConnections(), "Incorrect number of server connections")
 		}
-
-		// Make sure that a closed connection will reduce NumConnections.
-		client := ts.NewClient(nil)
-		require.NoError(t, client.Ping(ctx, ts.HostPort()), "Ping from new client failed")
-		assert.Equal(t, 11, ts.Server().IntrospectNumConnections(), "Number of connections expected to increase")
-
-		client.Close()
-		require.True(t, testutils.WaitFor(100*time.Millisecond, func() bool {
-			return ts.Server().IntrospectNumConnections() == 10
-		}), "Closed connection did not get removed, num connections is %v", ts.Server().IntrospectNumConnections())
 	})
 }