From bcf21b1966a79a4cace8ccfcfc4116be437edf7a Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 19 Sep 2016 14:10:35 +0100 Subject: [PATCH 1/2] show more details of a node's internet connections in the details panel, instead of showing connections to/from the internet as "TheInternet ", we now show "() " (or just " " if we don't have a dns name). Fixes #1713. --- render/detailed/connections.go | 39 +++++++++++++++++++++++----------- render/detailed/node_test.go | 18 ++++++++-------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/render/detailed/connections.go b/render/detailed/connections.go index fd75284616..cc5750ea8e 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -57,9 +57,9 @@ 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 { - remoteNodeID string - addr string // for internet nodes only: the address - port string // destination port + remoteNodeID string + remoteAddr, localAddr string // for internet nodes only + port string // destination port } type connectionCounters struct { @@ -92,19 +92,31 @@ func (c *connectionCounters) add(outgoing bool, localNode, remoteNode, localEndp return } // For internet nodes we break out individual addresses - if isInternetNode(localNode) { - if _, conn.addr, _, ok = report.ParseEndpointNodeID(localEndpoint.ID); !ok { - return - } - if set, ok := localEndpoint.Sets.Lookup(endpoint.ReverseDNSNames); ok && len(set) > 0 { - conn.addr = fmt.Sprintf("%s (%s)", set[0], conn.addr) - } + if conn.remoteAddr, ok = internetAddr(remoteNode, remoteEndpoint); !ok { + return + } + if conn.localAddr, ok = internetAddr(localNode, localEndpoint); !ok { + return } c.counted[connectionID] = struct{}{} c.counts[conn]++ } +func internetAddr(node report.Node, ep report.Node) (string, bool) { + if !isInternetNode(node) { + return "", true + } + _, addr, _, ok := report.ParseEndpointNodeID(ep.ID) + if !ok { + return "", false + } + if set, ok := ep.Sets.Lookup(endpoint.ReverseDNSNames); ok && len(set) > 0 { + addr = fmt.Sprintf("%s (%s)", set[0], addr) + } + return addr, true +} + func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection { output := []Connection{} for row, count := range c.counts { @@ -113,16 +125,19 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal // MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary. summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID]) connection := Connection{ - ID: fmt.Sprintf("%s-%s-%s", row.remoteNodeID, row.addr, row.port), + ID: fmt.Sprintf("%s-%s-%s-%s", row.remoteNodeID, row.remoteAddr, row.localAddr, row.port), NodeID: summary.ID, Label: summary.Label, Linkable: true, } + if row.remoteAddr != "" { + connection.Label = row.remoteAddr + } if includeLocal { connection.Metadata = append(connection.Metadata, report.MetadataRow{ ID: "foo", - Value: row.addr, + Value: row.localAddr, Datatype: number, }) } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index aff45bc47b..cd7f14a2c3 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -25,8 +25,8 @@ func child(t *testing.T, r render.Renderer, id string) detailed.NodeSummary { return s.SummarizeMetrics() } -func connectionID(nodeID string) string { - return fmt.Sprintf("%s-%s-%d", nodeID, "", 80) +func connectionID(nodeID string, addr string) string { + return fmt.Sprintf("%s-%s-%s-%d", nodeID, addr, "", 80) } func TestMakeDetailedHostNode(t *testing.T) { @@ -153,7 +153,7 @@ func TestMakeDetailedHostNode(t *testing.T) { Columns: detailed.NormalColumns, Connections: []detailed.Connection{ { - ID: connectionID(fixture.ServerHostNodeID), + ID: connectionID(fixture.ServerHostNodeID, ""), NodeID: fixture.ServerHostNodeID, Label: "server", Linkable: true, @@ -261,7 +261,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { Columns: detailed.NormalColumns, Connections: []detailed.Connection{ { - ID: connectionID(fixture.ClientContainerNodeID), + ID: connectionID(fixture.ClientContainerNodeID, ""), NodeID: fixture.ClientContainerNodeID, Label: "client", Linkable: true, @@ -279,9 +279,9 @@ func TestMakeDetailedContainerNode(t *testing.T) { }, }, { - ID: connectionID(render.IncomingInternetID), + ID: connectionID(render.IncomingInternetID, fixture.RandomClientIP), NodeID: render.IncomingInternetID, - Label: render.InboundMajor, + Label: fixture.RandomClientIP, Linkable: true, Metadata: []report.MetadataRow{ { @@ -381,7 +381,7 @@ func TestMakeDetailedPodNode(t *testing.T) { Columns: detailed.NormalColumns, Connections: []detailed.Connection{ { - ID: connectionID(fixture.ClientPodNodeID), + ID: connectionID(fixture.ClientPodNodeID, ""), NodeID: fixture.ClientPodNodeID, Label: "pong-a", Linkable: true, @@ -399,9 +399,9 @@ func TestMakeDetailedPodNode(t *testing.T) { }, }, { - ID: connectionID(render.IncomingInternetID), + ID: connectionID(render.IncomingInternetID, fixture.RandomClientIP), NodeID: render.IncomingInternetID, - Label: render.InboundMajor, + Label: fixture.RandomClientIP, Linkable: true, Metadata: []report.MetadataRow{ { From c455f048e252cb18e98e02575800ae0e495cccad Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 19 Sep 2016 15:31:26 +0100 Subject: [PATCH 2/2] comment on arbitrariness of displayed dns name --- render/detailed/connections.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/render/detailed/connections.go b/render/detailed/connections.go index cc5750ea8e..2d20615e84 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -112,6 +112,10 @@ func internetAddr(node report.Node, ep report.Node) (string, bool) { return "", false } if set, ok := ep.Sets.Lookup(endpoint.ReverseDNSNames); ok && len(set) > 0 { + // TODO We show just one of the names, selected rather + // abitrarily. We don't have space to show all (except in the + // tooltip perhaps), but should think of better strategies for + // choosing the name to display. addr = fmt.Sprintf("%s (%s)", set[0], addr) } return addr, true