Ensure connection rows have unique IDs. (#1245)

* Ensure connection rows have unique IDs. This adds new types for connections tables. * UI support for new connection table rows * Parameterized node Id key for connections table * also s/node_id/nodeId, and s/topology_id/topologyId in connections * table * Added comment about nodeIdKey * Review feedback:
2026-07-28 01:31:17 +00:00 · 2016-04-11 14:40:50 +01:00
parent 689e06b216
commit d237f52a90
6 changed files with 109 additions and 77 deletions
--- a/client/app/scripts/components/node-details.js
+++ b/client/app/scripts/components/node-details.js
@@ -180,7 +180,8 @@ export default class NodeDetails extends React.Component {

          {details.connections && details.connections.map(connections => <div
            className="node-details-content-section" key={connections.id}>
-              <NodeDetailsTable {...connections} />
+              <NodeDetailsTable {...connections} nodes={connections.connections}
+                nodeIdKey="nodeId" />
            </div>
          )}

--- a/client/app/scripts/components/node-details/node-details-table-node-link.js
+++ b/client/app/scripts/components/node-details/node-details-table-node-link.js
@@ -14,7 +14,7 @@ export default class NodeDetailsTableNodeLink extends React.Component {

  handleClick(ev) {
    ev.preventDefault();
-    clickRelative(this.props.id, this.props.topologyId, this.props.label,
+    clickRelative(this.props.nodeId, this.props.topologyId, this.props.label,
      ReactDOM.findDOMNode(this).getBoundingClientRect());
  }

--- a/client/app/scripts/components/node-details/node-details-table.js
+++ b/client/app/scripts/components/node-details/node-details-table.js
@@ -148,6 +148,7 @@ export default class NodeDetailsTable extends React.Component {

  render() {
    const headers = this.renderHeaders();
+    const { nodeIdKey } = this.props;
    let nodes = _.sortBy(this.props.nodes, this.getValueForSortBy, 'label',
      this.getMetaDataSorters());
    const limited = nodes && this.state.limit > 0 && nodes.length > this.state.limit;
@@ -169,10 +170,12 @@ export default class NodeDetailsTable extends React.Component {
          <tbody>
          {nodes && nodes.map(node => {
            const values = this.renderValues(node);
+            const nodeId = node[nodeIdKey];
            return (
              <tr className="node-details-table-node" key={node.id}>
                <td className="node-details-table-node-label truncate">
-                  <NodeDetailsTableNodeLink topologyId={this.props.topologyId} {...node} />
+                  <NodeDetailsTableNodeLink {...node} topologyId={this.props.topologyId}
+                    nodeId={nodeId} />
                </td>
                {values}
              </tr>
@@ -186,3 +189,7 @@ export default class NodeDetailsTable extends React.Component {
    );
  }
 }
+
+NodeDetailsTable.defaultProps = {
+  nodeIdKey: 'id' // key to identify a node in a row (used for topology links)
+};
--- a/render/detailed/connections.go
+++ b/render/detailed/connections.go
@@ -1,6 +1,7 @@
 package detailed

 import (
+	"fmt"
 	"sort"
 	"strconv"

@@ -29,17 +30,46 @@ var (
 	}
 )

-type connectionsRow struct {
+// ConnectionsSummary is the table of connection to/form a node
+type ConnectionsSummary struct {
+	ID          string       `json:"id"`
+	TopologyID  string       `json:"topologyId"`
+	Label       string       `json:"label"`
+	Columns     []Column     `json:"columns"`
+	Connections []Connection `json:"connections"`
+}
+
+// Connection is a row in the connections table.
+type Connection struct {
+	ID       string        `json:"id"`     // ID of this element in the UI.  Must be unique for a given ConnectionsSummary.
+	NodeID   string        `json:"nodeId"` // ID of a node in the topology. Optional, must be set if linkable is true.
+	Label    string        `json:"label"`
+	Linkable bool          `json:"linkable"`
+	Metadata []MetadataRow `json:"metadata,omitempty"`
+}
+
+type connectionsByID []Connection
+
+func (s connectionsByID) Len() int           { return len(s) }
+func (s connectionsByID) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s connectionsByID) Less(i, j int) bool { return s[i].ID < s[j].ID }
+
+// Intermediate type used as a key to dedupe rows
+type connection struct {
 	remoteNode, localNode *report.Node
 	remoteAddr, localAddr string
 	port                  string // always the server-side port
 }

-func incomingConnectionsTable(topologyID string, n report.Node, ns report.Nodes) NodeSummaryGroup {
+func (row connection) ID() string {
+	return fmt.Sprintf("%s:%s-%s:%s-%s", row.remoteNode.ID, row.remoteAddr, row.localNode.ID, row.localAddr, row.port)
+}
+
+func incomingConnectionsSummary(topologyID string, n report.Node, ns report.Nodes) ConnectionsSummary {
 	localEndpointIDs := endpointChildIDsOf(n)

 	// For each node which has an edge TO me
-	counts := map[connectionsRow]int{}
+	counts := map[connection]int{}
 	for _, node := range ns {
 		if !node.Adjacency.Contains(n.ID) {
 			continue
@@ -58,7 +88,7 @@ func incomingConnectionsTable(topologyID string, n report.Node, ns report.Nodes)
 				if !ok {
 					continue
 				}
-				key := connectionsRow{
+				key := connection{
 					localNode:  &n,
 					remoteNode: &remoteNode,
 					port:       port,
@@ -75,20 +105,20 @@ func incomingConnectionsTable(topologyID string, n report.Node, ns report.Nodes)
 	if isInternetNode(n) {
 		columnHeaders = InternetColumns
 	}
-	return NodeSummaryGroup{
-		ID:         "incoming-connections",
-		TopologyID: topologyID,
-		Label:      "Inbound",
-		Columns:    columnHeaders,
-		Nodes:      connectionRows(counts, isInternetNode(n)),
+	return ConnectionsSummary{
+		ID:          "incoming-connections",
+		TopologyID:  topologyID,
+		Label:       "Inbound",
+		Columns:     columnHeaders,
+		Connections: connectionRows(counts, isInternetNode(n)),
 	}
 }

-func outgoingConnectionsTable(topologyID string, n report.Node, ns report.Nodes) NodeSummaryGroup {
+func outgoingConnectionsSummary(topologyID string, n report.Node, ns report.Nodes) ConnectionsSummary {
 	localEndpoints := endpointChildrenOf(n)

 	// For each node which has an edge FROM me
-	counts := map[connectionsRow]int{}
+	counts := map[connection]int{}
 	for _, id := range n.Adjacency {
 		node, ok := ns[id]
 		if !ok {
@@ -108,7 +138,7 @@ func outgoingConnectionsTable(topologyID string, n report.Node, ns report.Nodes)
 				if !ok {
 					continue
 				}
-				key := connectionsRow{
+				key := connection{
 					localNode:  &n,
 					remoteNode: &remoteNode,
 					port:       port,
@@ -125,12 +155,12 @@ func outgoingConnectionsTable(topologyID string, n report.Node, ns report.Nodes)
 	if isInternetNode(n) {
 		columnHeaders = InternetColumns
 	}
-	return NodeSummaryGroup{
-		ID:         "outgoing-connections",
-		TopologyID: topologyID,
-		Label:      "Outbound",
-		Columns:    columnHeaders,
-		Nodes:      connectionRows(counts, isInternetNode(n)),
+	return ConnectionsSummary{
+		ID:          "outgoing-connections",
+		TopologyID:  topologyID,
+		Label:       "Outbound",
+		Columns:     columnHeaders,
+		Connections: connectionRows(counts, isInternetNode(n)),
 	}
 }

@@ -158,30 +188,32 @@ func isInternetNode(n report.Node) bool {
 	return n.ID == render.IncomingInternetID || n.ID == render.OutgoingInternetID
 }

-func connectionRows(in map[connectionsRow]int, includeLocal bool) []NodeSummary {
-	nodes := []NodeSummary{}
+func connectionRows(in map[connection]int, includeLocal bool) []Connection {
+	output := []Connection{}
 	for row, count := range in {
 		// Use MakeNodeSummary to render the id and label of this node
 		// TODO(paulbellamy): Would be cleaner if we hade just a
 		// MakeNodeID(*row.remoteode). As we don't need the whole summary.
 		summary, ok := MakeNodeSummary(*row.remoteNode)
-		summary.Metadata, summary.Metrics, summary.DockerLabels = nil, nil, nil
+		connection := Connection{
+			ID:       row.ID(),
+			NodeID:   summary.ID,
+			Label:    summary.Label,
+			Linkable: true,
+		}
 		if !ok && row.remoteAddr != "" {
-			summary = NodeSummary{
-				ID:       row.remoteAddr + ":" + row.port,
-				Label:    row.remoteAddr,
-				Linkable: false,
-			}
+			connection.Label = row.remoteAddr
+			connection.Linkable = false
 		}
 		if includeLocal {
-			summary.Metadata = append(summary.Metadata,
+			connection.Metadata = append(connection.Metadata,
 				MetadataRow{
 					ID:       "foo",
 					Value:    row.localAddr,
 					Datatype: number,
 				})
 		}
-		summary.Metadata = append(summary.Metadata,
+		connection.Metadata = append(connection.Metadata,
 			MetadataRow{
 				ID:       portKey,
 				Value:    row.port,
@@ -193,8 +225,8 @@ func connectionRows(in map[connectionsRow]int, includeLocal bool) []NodeSummary
 				Datatype: number,
 			},
 		)
-		nodes = append(nodes, summary)
+		output = append(output, connection)
 	}
-	sort.Sort(nodeSummariesByID(nodes))
-	return nodes
+	sort.Sort(connectionsByID(output))
+	return output
 }
--- a/render/detailed/node.go
+++ b/render/detailed/node.go
@@ -15,10 +15,10 @@ import (
 // we want deep information about an individual node.
 type Node struct {
 	NodeSummary
-	Controls    []ControlInstance  `json:"controls"`
-	Children    []NodeSummaryGroup `json:"children,omitempty"`
-	Parents     []Parent           `json:"parents,omitempty"`
-	Connections []NodeSummaryGroup `json:"connections,omitempty"`
+	Controls    []ControlInstance    `json:"controls"`
+	Children    []NodeSummaryGroup   `json:"children,omitempty"`
+	Parents     []Parent             `json:"parents,omitempty"`
+	Connections []ConnectionsSummary `json:"connections,omitempty"`
 }

 // ControlInstance contains a control description, and all the info
@@ -83,9 +83,9 @@ func MakeNode(topologyID string, r report.Report, ns report.Nodes, n report.Node
 		Controls:    controls(r, n),
 		Children:    children(n),
 		Parents:     Parents(r, n),
-		Connections: []NodeSummaryGroup{
-			incomingConnectionsTable(topologyID, n, ns),
-			outgoingConnectionsTable(topologyID, n, ns),
+		Connections: []ConnectionsSummary{
+			incomingConnectionsSummary(topologyID, n, ns),
+			outgoingConnectionsSummary(topologyID, n, ns),
 		},
 	}
 }
--- a/render/detailed/node_test.go
+++ b/render/detailed/node_test.go
@@ -1,6 +1,7 @@
 package detailed_test

 import (
+	"fmt"
 	"testing"

 	"github.com/weaveworks/scope/probe/docker"
@@ -113,28 +114,25 @@ func TestMakeDetailedHostNode(t *testing.T) {
 				Nodes: []detailed.NodeSummary{containerImageNodeSummary},
 			},
 		},
-		Connections: []detailed.NodeSummaryGroup{
+		Connections: []detailed.ConnectionsSummary{
 			{
-				ID:         "incoming-connections",
-				TopologyID: "hosts",
-				Label:      "Inbound",
-				Columns:    detailed.NormalColumns,
-				Nodes:      []detailed.NodeSummary{},
+				ID:          "incoming-connections",
+				TopologyID:  "hosts",
+				Label:       "Inbound",
+				Columns:     detailed.NormalColumns,
+				Connections: []detailed.Connection{},
 			},
 			{
 				ID:         "outgoing-connections",
 				TopologyID: "hosts",
 				Label:      "Outbound",
 				Columns:    detailed.NormalColumns,
-				Nodes: []detailed.NodeSummary{
+				Connections: []detailed.Connection{
 					{
-						ID:         fixture.ServerHostNodeID,
-						Label:      "server",
-						LabelMinor: "hostname.com",
-						Rank:       "hostname.com",
-						Shape:      "circle",
-						Linkable:   true,
-						Adjacency:  report.MakeIDList(render.OutgoingInternetID),
+						ID:       fmt.Sprintf("%s:%s-%s:%s-%d", fixture.ServerHostNodeID, "", fixture.ClientHostNodeID, "", 80),
+						NodeID:   fixture.ServerHostNodeID,
+						Label:    "server",
+						Linkable: true,
 						Metadata: []detailed.MetadataRow{
 							{
 								ID:       "port",
@@ -223,20 +221,18 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 				TopologyID: "hosts",
 			},
 		},
-		Connections: []detailed.NodeSummaryGroup{
+		Connections: []detailed.ConnectionsSummary{
 			{
 				ID:         "incoming-connections",
 				TopologyID: "containers",
 				Label:      "Inbound",
 				Columns:    detailed.NormalColumns,
-				Nodes: []detailed.NodeSummary{
+				Connections: []detailed.Connection{
 					{
-						ID:         fixture.ClientContainerNodeID,
-						Label:      "client",
-						LabelMinor: "client.hostname.com",
-						Shape:      "hexagon",
-						Linkable:   true,
-						Adjacency:  report.MakeIDList(fixture.ServerContainerNodeID),
+						ID:       fmt.Sprintf("%s:%s-%s:%s-%d", fixture.ClientContainerNodeID, "", fixture.ServerContainerNodeID, "", 80),
+						NodeID:   fixture.ClientContainerNodeID,
+						Label:    "client",
+						Linkable: true,
 						Metadata: []detailed.MetadataRow{
 							{
 								ID:       "port",
@@ -251,14 +247,10 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 						},
 					},
 					{
-						ID:         render.IncomingInternetID,
-						Label:      render.InboundMajor,
-						LabelMinor: render.InboundMinor,
-						Rank:       render.IncomingInternetID,
-						Shape:      "cloud",
-						Linkable:   true,
-						Pseudo:     true,
-						Adjacency:  report.MakeIDList(fixture.ServerContainerNodeID),
+						ID:       fmt.Sprintf("%s:%s-%s:%s-%d", render.IncomingInternetID, "", fixture.ServerContainerNodeID, "", 80),
+						NodeID:   render.IncomingInternetID,
+						Label:    render.InboundMajor,
+						Linkable: true,
 						Metadata: []detailed.MetadataRow{
 							{
 								ID:       "port",
@@ -275,11 +267,11 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 				},
 			},
 			{
-				ID:         "outgoing-connections",
-				TopologyID: "containers",
-				Label:      "Outbound",
-				Columns:    detailed.NormalColumns,
-				Nodes:      []detailed.NodeSummary{},
+				ID:          "outgoing-connections",
+				TopologyID:  "containers",
+				Label:       "Outbound",
+				Columns:     detailed.NormalColumns,
+				Connections: []detailed.Connection{},
 			},
 		},
 	}