From 0cb34b53cc2d538c1c8462b5dcdc56be84a60daa Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 15:58:35 +0000
Subject: [PATCH 01/24] introduce ParseProcessNodeID

unused for now
---
 report/id.go | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/report/id.go b/report/id.go
index 6b2f0b6c5..3d8b99274 100644
--- a/report/id.go
+++ b/report/id.go
@@ -229,6 +229,11 @@ func ParseAddressNodeID(addressNodeID string) (hostID, address string, ok bool)
 	return split2(addressNodeID, ScopeDelim)
 }
 
+// ParseProcessNodeID produces the host ID and PID from a process node ID.
+func ParseProcessNodeID(processNodeID string) (hostID, pid string, ok bool) {
+	return split2(processNodeID, ScopeDelim)
+}
+
 // ParseECSServiceNodeID produces the cluster, service name from an ECS Service node ID
 func ParseECSServiceNodeID(ecsServiceNodeID string) (cluster, serviceName string, ok bool) {
 	cluster, serviceName, ok = split2(ecsServiceNodeID, ScopeDelim)

From bd214227dc6189e44d4b32c62d51f36f2ea4e036 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 19:30:01 +0000
Subject: [PATCH 02/24] introduce ParsePseudoNodeID

unused for now

A convenient place to document and deal with the ugliness.
---
 render/id.go | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/render/id.go b/render/id.go
index 25ba459f9..97f5f5b9f 100644
--- a/render/id.go
+++ b/render/id.go
@@ -23,6 +23,20 @@ func MakePseudoNodeID(parts ...string) string {
 	return strings.Join(append([]string{"pseudo"}, parts...), ":")
 }
 
+// ParsePseudoNodeID returns the joined id parts of a pseudonode
+// ID. If the ID is not recognisable as a pseudonode ID, it is
+// returned as is, with the returned bool set to false. That is
+// convenient because not all pseudonode IDs actually follow the
+// format produced by MakePseudoNodeID.
+func ParsePseudoNodeID(nodeID string) (string, bool) {
+	// Not using strings.SplitN() to avoid a heap allocation
+	pos := strings.Index(nodeID, ":")
+	if pos == -1 || nodeID[:pos] != "pseudo" {
+		return nodeID, false
+	}
+	return nodeID[pos+1:], true
+}
+
 // MakeGroupNodeTopology joins the parts of a group topology into the topology of a group node
 func MakeGroupNodeTopology(originalTopology, key string) string {
 	return strings.Join([]string{"group", originalTopology, key}, ":")

From b8eeadda347dbf0448c6267675171b78ec91d458 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Fri, 22 Dec 2017 02:28:14 +0000
Subject: [PATCH 03/24] refactor: don't set shape based on unitialised topology

This doesn't make any difference to the outcome - we were simply
setting the shape in the NodeSummary to "", which is what it starts
out as - but looks less weird in the code.
---
 render/detailed/summary.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 5445349c9..124bbc775 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -348,12 +348,12 @@ func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) (NodeSum
 	}
 	base.Label, base.Rank = label, label
 
-	t, ok := r.Topology(parts[1])
-	if ok && t.Label != "" {
-		base.LabelMinor = pluralize(n.Counters, parts[1], t.Label, t.LabelPlural)
+	if t, ok := r.Topology(parts[1]); ok {
+		base.Shape = t.GetShape()
+		if t.Label != "" {
+			base.LabelMinor = pluralize(n.Counters, parts[1], t.Label, t.LabelPlural)
+		}
 	}
-
-	base.Shape = t.GetShape()
 	base.Stack = true
 	return base, true
 }

From ec589e08f6aa3cca6789e68420640675b218153c Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Fri, 22 Dec 2017 02:39:44 +0000
Subject: [PATCH 04/24] refactor: introduce ParseGroupNodeTopology

---
 render/detailed/summary.go | 10 +++++-----
 render/id.go               |  9 +++++++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 124bbc775..ee00f2d59 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -337,21 +337,21 @@ func weaveNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 // groupNodeSummary renders the summary for a group node. n.Topology is
 // expected to be of the form: group:container:hostname
 func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) (NodeSummary, bool) {
-	parts := strings.Split(n.Topology, ":")
-	if len(parts) != 3 {
+	topology, key, ok := render.ParseGroupNodeTopology(n.Topology)
+	if !ok {
 		return NodeSummary{}, false
 	}
 
-	label, ok := n.Latest.Lookup(parts[2])
+	label, ok := n.Latest.Lookup(key)
 	if !ok {
 		return NodeSummary{}, false
 	}
 	base.Label, base.Rank = label, label
 
-	if t, ok := r.Topology(parts[1]); ok {
+	if t, ok := r.Topology(topology); ok {
 		base.Shape = t.GetShape()
 		if t.Label != "" {
-			base.LabelMinor = pluralize(n.Counters, parts[1], t.Label, t.LabelPlural)
+			base.LabelMinor = pluralize(n.Counters, topology, t.Label, t.LabelPlural)
 		}
 	}
 	base.Stack = true
diff --git a/render/id.go b/render/id.go
index 97f5f5b9f..5fa34f8f2 100644
--- a/render/id.go
+++ b/render/id.go
@@ -42,6 +42,15 @@ func MakeGroupNodeTopology(originalTopology, key string) string {
 	return strings.Join([]string{"group", originalTopology, key}, ":")
 }
 
+// ParseGroupNodeTopology returns the parts of a group topology.
+func ParseGroupNodeTopology(topology string) (string, string, bool) {
+	parts := strings.Split(topology, ":")
+	if len(parts) != 3 || parts[0] != "group" {
+		return "", "", false
+	}
+	return parts[1], parts[2], true
+}
+
 // NewDerivedNode makes a node based on node, but with a new ID
 func NewDerivedNode(id string, node report.Node) report.Node {
 	return report.MakeNode(id).WithChildren(node.Children.Add(node))

From e5149aa7cd3e27d44426999893f5dd9415e8fdf8 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 00:04:03 +0000
Subject: [PATCH 05/24] render sensible labels for processes with little/no
 metadata

We cope with the absence of the process name and/or container name,
and extract the hostID and pid from the node id rather than metadata
since that way we are guaranteed to get values for them.
---
 render/detailed/summary.go | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index ee00f2d59..2f9e10fd0 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -203,19 +203,29 @@ func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 }
 
 func processNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
-	base.Label, _ = n.Latest.Lookup(process.Name)
-	base.Rank, _ = n.Latest.Lookup(process.Name)
-
-	pid, ok := n.Latest.Lookup(process.PID)
-	if !ok {
-		return NodeSummary{}, false
+	var (
+		hostID, pid, _   = report.ParseProcessNodeID(n.ID)
+		processName, _   = n.Latest.Lookup(process.Name)
+		containerName, _ = n.Latest.Lookup(docker.ContainerName)
+	)
+	switch {
+	case processName != "" && containerName != "":
+		base.Label = processName
+		base.LabelMinor = fmt.Sprintf("%s (%s:%s)", hostID, containerName, pid)
+		base.Rank = processName
+	case processName != "":
+		base.Label = processName
+		base.LabelMinor = fmt.Sprintf("%s (%s)", hostID, pid)
+		base.Rank = processName
+	case containerName != "":
+		base.Label = pid
+		base.LabelMinor = fmt.Sprintf("%s (%s)", hostID, containerName)
+		base.Rank = hostID
+	default:
+		base.Label = pid
+		base.LabelMinor = hostID
+		base.Rank = hostID
 	}
-	if containerName, ok := n.Latest.Lookup(docker.ContainerName); ok {
-		base.LabelMinor = fmt.Sprintf("%s (%s:%s)", report.ExtractHostID(n), containerName, pid)
-	} else {
-		base.LabelMinor = fmt.Sprintf("%s (%s)", report.ExtractHostID(n), pid)
-	}
-
 	base.Linkable = render.IsConnected(n)
 	return base, true
 }

From d92c4b12c3b92649d29bfe75f06bbbfebb5c94e5 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 00:57:47 +0000
Subject: [PATCH 06/24] render sensible labels for containers with little/no
 metadata

We fall back to the truncated container id when we cannot find a
name. NB: this also happens when rendering a container as a parent.

We cope with the absence of an image name and/or host name.
---
 render/detailed/summary.go | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 2f9e10fd0..3990af033 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -231,13 +231,20 @@ func processNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 }
 
 func containerNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
-	base.Label = getRenderableContainerName(n)
-	base.LabelMinor = report.ExtractHostID(n)
-
-	if imageName, ok := n.Latest.Lookup(docker.ImageName); ok {
+	var (
+		containerName = getRenderableContainerName(n)
+		hostName      = report.ExtractHostID(n)
+		imageName, _  = n.Latest.Lookup(docker.ImageName)
+	)
+	base.Label = containerName
+	base.LabelMinor = hostName
+	if imageName != "" {
 		base.Rank = docker.ImageNameWithoutVersion(imageName)
+	} else if hostName != "" {
+		base.Rank = hostName
+	} else {
+		base.Rank = base.Label
 	}
-
 	return base, true
 }
 
@@ -428,5 +435,9 @@ func getRenderableContainerName(nmd report.Node) string {
 			return label
 		}
 	}
-	return ""
+	containerID, _ := report.ParseContainerNodeID(nmd.ID)
+	if len(containerID) > 12 {
+		containerID = containerID[:12]
+	}
+	return containerID
 }

From 19dc67b6cf1ebbab739dc861f22ba242ead00f6a Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 09:35:41 +0000
Subject: [PATCH 07/24] render sensible labels for pseudo nodes with little/no
 metadata

The main change here is to to label the node with its pseudoID as the
last resort.

We also set the rank to the pseudoID instead of the full node id,
since including the "pseudo:" prefix is not conducive to good ranking.

The rest is just moving from 'if' to 'switch'.
---
 render/detailed/summary.go | 52 +++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 3990af033..27e8292a7 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -155,51 +155,45 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary {
 }
 
 func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+	pseudoID, _ := render.ParsePseudoNodeID(n.ID)
 	base.Pseudo = true
-	base.Rank = n.ID
+	base.Rank = pseudoID
 
-	// try rendering as an internet node
-	if template, ok := templates[n.ID]; ok {
+	switch {
+	case render.IsInternetNode(n):
+		// render as an internet node
+		template := templates[n.ID]
 		base.Label = template.Label
 		base.LabelMinor = template.LabelMinor
 		base.Shape = report.Cloud
-		return base, true
-	}
-
-	// try rendering as a known service node
-	if strings.HasPrefix(n.ID, render.ServiceNodeIDPrefix) {
+	case strings.HasPrefix(n.ID, render.ServiceNodeIDPrefix):
+		// render as a known service node
 		base.Label = n.ID[len(render.ServiceNodeIDPrefix):]
 		base.LabelMinor = ""
 		base.Shape = report.Cloud
-		return base, true
-	}
-
-	// try rendering it as an uncontained node
-	if strings.HasPrefix(n.ID, render.UncontainedIDPrefix) {
+	case strings.HasPrefix(n.ID, render.UncontainedIDPrefix):
+		// render as an uncontained node
 		base.Label = render.UncontainedMajor
 		base.LabelMinor = report.ExtractHostID(n)
 		base.Shape = report.Square
 		base.Stack = true
-		return base, true
-	}
-
-	// try rendering it as an unmanaged node
-	if strings.HasPrefix(n.ID, render.UnmanagedIDPrefix) {
+	case strings.HasPrefix(n.ID, render.UnmanagedIDPrefix):
+		// render as an unmanaged node
 		base.Label = render.UnmanagedMajor
+		base.LabelMinor = report.ExtractHostID(n)
 		base.Shape = report.Square
 		base.Stack = true
-		base.LabelMinor = report.ExtractHostID(n)
-		return base, true
+	default:
+		// try rendering it as an endpoint
+		if _, addr, _, ok := report.ParseEndpointNodeID(n.ID); ok {
+			base.Label = addr
+			base.Shape = report.Circle
+		} else {
+			// last resort
+			base.Label = pseudoID
+		}
 	}
-
-	// try rendering it as an endpoint
-	if _, addr, _, ok := report.ParseEndpointNodeID(n.ID); ok {
-		base.Label = addr
-		base.Shape = report.Circle
-		return base, true
-	}
-
-	return NodeSummary{}, false
+	return base, true
 }
 
 func processNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {

From b83f7e8ce62aead5592e7ad2529cef91a8924a43 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 15:23:13 +0000
Subject: [PATCH 08/24] render sensible labels for images with little/no
 metadata

We fall back to image id when we cannot find a name. We extract the
image id from the node id rather than the docker.ImageID latest map
entry since that way we are guaranteed to get it.
---
 render/detailed/summary.go | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 27e8292a7..e46e9b7df 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -243,25 +243,24 @@ func containerNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 }
 
 func containerImageNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
-	imageName, ok := n.Latest.Lookup(docker.ImageName)
-	if !ok {
-		return NodeSummary{}, false
+	var (
+		imageName, _            = n.Latest.Lookup(docker.ImageName)
+		imageNameWithoutVersion = docker.ImageNameWithoutVersion(imageName)
+	)
+	switch {
+	case imageNameWithoutVersion != "" && imageNameWithoutVersion != ImageNameNone:
+		base.Label = imageNameWithoutVersion
+	case imageName != "" && imageName != ImageNameNone:
+		base.Label = imageName
+	default:
+		// The id can be an image id or an image name. Ideally we'd
+		// truncate the former but not the latter, but short of
+		// heuristic regexp match we cannot tell the difference.
+		base.Label, _ = report.ParseContainerImageNodeID(n.ID)
 	}
-
-	imageNameWithoutVersion := docker.ImageNameWithoutVersion(imageName)
-	base.Label = imageNameWithoutVersion
-	base.Rank = imageNameWithoutVersion
-	base.Stack = true
-
-	if base.Label == ImageNameNone {
-		base.Label, _ = n.Latest.Lookup(docker.ImageID)
-		if len(base.Label) > 12 {
-			base.Label = base.Label[:12]
-		}
-	}
-
 	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
-
+	base.Rank = base.Label
+	base.Stack = true
 	return base, true
 }
 

From f192e793462d899af9868a3e129f029da858776b Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 18:01:28 +0000
Subject: [PATCH 09/24] render sensible labels for host nodes with little/no
 metadata

The node id, which we always have, actually contains the hostname.
---
 render/detailed/summary.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index e46e9b7df..4c1e7821f 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -6,7 +6,6 @@ import (
 
 	"github.com/weaveworks/scope/probe/awsecs"
 	"github.com/weaveworks/scope/probe/docker"
-	"github.com/weaveworks/scope/probe/host"
 	"github.com/weaveworks/scope/probe/kubernetes"
 	"github.com/weaveworks/scope/probe/overlay"
 	"github.com/weaveworks/scope/probe/process"
@@ -319,7 +318,7 @@ func swarmServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool
 
 func hostNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 	var (
-		hostname, _ = n.Latest.Lookup(host.HostName)
+		hostname, _ = report.ParseHostNodeID(n.ID)
 		parts       = strings.SplitN(hostname, ".", 2)
 	)
 

From 289226707d67a7b626039428b1fa82e1ab16caf5 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 16:24:46 +0000
Subject: [PATCH 10/24] render sensible labels for k8s nodes with little/no
 metadata

We fall back to using the object id as the label.
---
 render/detailed/summary.go | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 4c1e7821f..d6b0bbd39 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -264,8 +264,15 @@ func containerImageNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bo
 }
 
 func addKubernetesLabelAndRank(base NodeSummary, n report.Node) NodeSummary {
-	base.Label, _ = n.Latest.Lookup(kubernetes.Name)
-	namespace, _ := n.Latest.Lookup(kubernetes.Namespace)
+	var (
+		name, _      = n.Latest.Lookup(kubernetes.Name)
+		namespace, _ = n.Latest.Lookup(kubernetes.Namespace)
+	)
+	if name != "" {
+		base.Label = name
+	} else {
+		base.Label, _, _ = report.ParseNodeID(n.ID)
+	}
 	base.Rank = namespace + "/" + base.Label
 	return base
 }

From 9aed0792acb145d5284acd9ac79caa0bbd1d9932 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 16:39:31 +0000
Subject: [PATCH 11/24] render sensible labels for ecs task nodes with
 little/no metadata

We fall back to using the ARN as the label.
---
 render/detailed/summary.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index d6b0bbd39..74685a91f 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -309,6 +309,9 @@ func podGroupNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 
 func ecsTaskNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 	base.Label, _ = n.Latest.Lookup(awsecs.TaskFamily)
+	if base.Label == "" {
+		base.Label, _ = report.ParseECSTaskNodeID(n.ID)
+	}
 	return base, true
 }
 

From 5e099640ebd69c6eb5efa0cbd042cdcdcbc93fca Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 17:47:22 +0000
Subject: [PATCH 12/24] render sensible labels for swarm service nodes with
 little/no metadata

We fall back to using the service ID as the label.
---
 render/detailed/summary.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 74685a91f..0693dbf87 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -323,6 +323,9 @@ func ecsServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool)
 
 func swarmServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 	base.Label, _ = n.Latest.Lookup(docker.ServiceName)
+	if base.Label == "" {
+		base.Label, _ = report.ParseSwarmServiceNodeID(n.ID)
+	}
 	return base, true
 }
 

From 15881cd7fd938fcd5783d0645c74ca6dfc7938c3 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 18:09:31 +0000
Subject: [PATCH 13/24] render sensible labels for weave peer nodes with
 little/no metadata

We fall back to using the peerName as the label, which we always have.
---
 render/detailed/summary.go | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 0693dbf87..5dbdfa57c 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -347,12 +347,14 @@ func hostNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 func weaveNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 	var (
 		nickname, _ = n.Latest.Lookup(overlay.WeavePeerNickName)
+		_, peerName = report.ParseOverlayNodeID(n.ID)
 	)
-
-	_, peerName := report.ParseOverlayNodeID(n.ID)
-
-	base.Label, base.LabelMinor = nickname, peerName
-
+	if nickname != "" {
+		base.Label = nickname
+	} else {
+		base.Label = peerName
+	}
+	base.LabelMinor = peerName
 	return base, true
 }
 

From da11655659ddfebc4d3bbd8d312027ed8054aa3f Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sat, 23 Dec 2017 12:22:05 +0000
Subject: [PATCH 14/24] render sensible labels for group nodes with little/no
 metadata

The node ID of group nodes is in fact the same value as we get from
looking up the metadata key contained in the group topology id. So
just use that since a) we always have it, and b) we save a LatestMap
lookup.
---
 render/detailed/summary.go | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 5dbdfa57c..4088d5dcf 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -361,21 +361,13 @@ func weaveNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 // groupNodeSummary renders the summary for a group node. n.Topology is
 // expected to be of the form: group:container:hostname
 func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) (NodeSummary, bool) {
-	topology, key, ok := render.ParseGroupNodeTopology(n.Topology)
-	if !ok {
-		return NodeSummary{}, false
-	}
-
-	label, ok := n.Latest.Lookup(key)
-	if !ok {
-		return NodeSummary{}, false
-	}
-	base.Label, base.Rank = label, label
-
-	if t, ok := r.Topology(topology); ok {
-		base.Shape = t.GetShape()
-		if t.Label != "" {
-			base.LabelMinor = pluralize(n.Counters, topology, t.Label, t.LabelPlural)
+	base.Label, base.Rank = n.ID, n.ID
+	if topology, _, ok := render.ParseGroupNodeTopology(n.Topology); ok {
+		if t, ok := r.Topology(topology); ok {
+			base.Shape = t.GetShape()
+			if t.Label != "" {
+				base.LabelMinor = pluralize(n.Counters, topology, t.Label, t.LabelPlural)
+			}
 		}
 	}
 	base.Stack = true

From a9b8ced0a72e1e33c90153aa256567e203e18094 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 18:15:21 +0000
Subject: [PATCH 15/24] refactor: drop superfluous return value

Now that summary renderers always produce something, they no longer
need to indicate whether they did.
---
 render/detailed/summary.go | 63 ++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 34 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 4088d5dcf..0b39a5b38 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -58,7 +58,7 @@ type NodeSummary struct {
 	Adjacency  report.IDList        `json:"adjacency,omitempty"`
 }
 
-var renderers = map[string]func(NodeSummary, report.Node) (NodeSummary, bool){
+var renderers = map[string]func(NodeSummary, report.Node) NodeSummary{
 	render.Pseudo:         pseudoNodeSummary,
 	report.Process:        processNodeSummary,
 	report.Container:      containerNodeSummary,
@@ -105,8 +105,8 @@ func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool)
 	if renderer, ok := renderers[n.Topology]; ok {
 		// Skip (and don't fall through to fallback) if renderer maps to nil
 		if renderer != nil {
-			summary, b := renderer(baseNodeSummary(r, n), n)
-			return RenderMetricURLs(summary, n, rc.MetricsGraphURL), b
+			summary := renderer(baseNodeSummary(r, n), n)
+			return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
 		}
 	} else if _, ok := rc.Topology(n.Topology); ok {
 		summary := baseNodeSummary(r, n)
@@ -114,8 +114,8 @@ func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool)
 		return summary, true
 	}
 	if strings.HasPrefix(n.Topology, "group:") {
-		summary, b := groupNodeSummary(baseNodeSummary(r, n), r, n)
-		return RenderMetricURLs(summary, n, rc.MetricsGraphURL), b
+		summary := groupNodeSummary(baseNodeSummary(r, n), r, n)
+		return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
 	}
 	return NodeSummary{}, false
 }
@@ -153,7 +153,7 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary {
 	return summary
 }
 
-func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func pseudoNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	pseudoID, _ := render.ParsePseudoNodeID(n.ID)
 	base.Pseudo = true
 	base.Rank = pseudoID
@@ -192,10 +192,10 @@ func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 			base.Label = pseudoID
 		}
 	}
-	return base, true
+	return base
 }
 
-func processNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func processNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	var (
 		hostID, pid, _   = report.ParseProcessNodeID(n.ID)
 		processName, _   = n.Latest.Lookup(process.Name)
@@ -220,10 +220,10 @@ func processNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 		base.Rank = hostID
 	}
 	base.Linkable = render.IsConnected(n)
-	return base, true
+	return base
 }
 
-func containerNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func containerNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	var (
 		containerName = getRenderableContainerName(n)
 		hostName      = report.ExtractHostID(n)
@@ -238,10 +238,10 @@ func containerNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 	} else {
 		base.Rank = base.Label
 	}
-	return base, true
+	return base
 }
 
-func containerImageNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func containerImageNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	var (
 		imageName, _            = n.Latest.Lookup(docker.ImageName)
 		imageNameWithoutVersion = docker.ImageNameWithoutVersion(imageName)
@@ -260,7 +260,7 @@ func containerImageNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bo
 	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
 	base.Rank = base.Label
 	base.Stack = true
-	return base, true
+	return base
 }
 
 func addKubernetesLabelAndRank(base NodeSummary, n report.Node) NodeSummary {
@@ -277,11 +277,10 @@ func addKubernetesLabelAndRank(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func podNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func podNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	base = addKubernetesLabelAndRank(base, n)
 	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
-
-	return base, true
+	return base
 }
 
 var podGroupNodeTypeName = map[string]string{
@@ -291,10 +290,9 @@ var podGroupNodeTypeName = map[string]string{
 	report.CronJob:     "CronJob",
 }
 
-func podGroupNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func podGroupNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	base = addKubernetesLabelAndRank(base, n)
 	base.Stack = true
-
 	// NB: pods are the highest aggregation level for which we display
 	// counts.
 	count := pluralize(n.Counters, report.Pod, "pod", "pods")
@@ -303,48 +301,45 @@ func podGroupNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 	} else {
 		base.LabelMinor = count
 	}
-
-	return base, true
+	return base
 }
 
-func ecsTaskNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func ecsTaskNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	base.Label, _ = n.Latest.Lookup(awsecs.TaskFamily)
 	if base.Label == "" {
 		base.Label, _ = report.ParseECSTaskNodeID(n.ID)
 	}
-	return base, true
+	return base
 }
 
-func ecsServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func ecsServiceNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	_, base.Label, _ = report.ParseECSServiceNodeID(n.ID)
 	base.Stack = true
-	return base, true
+	return base
 }
 
-func swarmServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func swarmServiceNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	base.Label, _ = n.Latest.Lookup(docker.ServiceName)
 	if base.Label == "" {
 		base.Label, _ = report.ParseSwarmServiceNodeID(n.ID)
 	}
-	return base, true
+	return base
 }
 
-func hostNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func hostNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	var (
 		hostname, _ = report.ParseHostNodeID(n.ID)
 		parts       = strings.SplitN(hostname, ".", 2)
 	)
-
 	if len(parts) == 2 {
 		base.Label, base.LabelMinor, base.Rank = parts[0], parts[1], parts[1]
 	} else {
 		base.Label = hostname
 	}
-
-	return base, true
+	return base
 }
 
-func weaveNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func weaveNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	var (
 		nickname, _ = n.Latest.Lookup(overlay.WeavePeerNickName)
 		_, peerName = report.ParseOverlayNodeID(n.ID)
@@ -355,12 +350,12 @@ func weaveNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
 		base.Label = peerName
 	}
 	base.LabelMinor = peerName
-	return base, true
+	return base
 }
 
 // groupNodeSummary renders the summary for a group node. n.Topology is
 // expected to be of the form: group:container:hostname
-func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) (NodeSummary, bool) {
+func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) NodeSummary {
 	base.Label, base.Rank = n.ID, n.ID
 	if topology, _, ok := render.ParseGroupNodeTopology(n.Topology); ok {
 		if t, ok := r.Topology(topology); ok {
@@ -371,7 +366,7 @@ func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) (NodeSum
 		}
 	}
 	base.Stack = true
-	return base, true
+	return base
 }
 
 func pluralize(counters report.Counters, key, singular, plural string) string {

From d1149dc29ecfbcaaf9cdef01cbcd7e1d2d9f9bcc Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 21 Dec 2017 19:31:10 +0000
Subject: [PATCH 16/24] add a basic test for rendering nodes with little/no
 metadata

---
 render/detailed/summary_test.go | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go
index b110ea2f8..3926f2013 100644
--- a/render/detailed/summary_test.go
+++ b/render/detailed/summary_test.go
@@ -198,6 +198,38 @@ func TestMakeNodeSummary(t *testing.T) {
 	}
 }
 
+func TestMakeNodeSummaryNoMetadata(t *testing.T) {
+	processNameTopology := render.MakeGroupNodeTopology(report.Process, process.Name)
+	for topology, id := range map[string]string{
+		render.Pseudo:         render.MakePseudoNodeID("id"),
+		report.Process:        report.MakeProcessNodeID("ip-123-45-6-100", "1234"),
+		report.Container:      report.MakeContainerNodeID("0001accbecc2c95e650fe641926fb923b7cc307a71101a1200af3759227b6d7d"),
+		report.ContainerImage: report.MakeContainerImageNodeID("0001accbecc2c95e650fe641926fb923b7cc307a71101a1200af3759227b6d7d"),
+		report.Pod:            report.MakePodNodeID("005e2999-d429-11e7-8535-0a41257e78e8"),
+		report.Service:        report.MakeServiceNodeID("005e2999-d429-11e7-8535-0a41257e78e8"),
+		report.Deployment:     report.MakeDeploymentNodeID("005e2999-d429-11e7-8535-0a41257e78e8"),
+		report.DaemonSet:      report.MakeDaemonSetNodeID("005e2999-d429-11e7-8535-0a41257e78e8"),
+		report.StatefulSet:    report.MakeStatefulSetNodeID("005e2999-d429-11e7-8535-0a41257e78e8"),
+		report.CronJob:        report.MakeCronJobNodeID("005e2999-d429-11e7-8535-0a41257e78e8"),
+		report.ECSTask:        report.MakeECSTaskNodeID("arn:aws:ecs:us-east-1:012345678910:task/1dc5c17a-422b-4dc4-b493-371970c6c4d6"),
+		report.ECSService:     report.MakeECSServiceNodeID("cluster", "service"),
+		report.SwarmService:   report.MakeSwarmServiceNodeID("0001accbecc2c95e650fe641926fb923b7cc307a71101a1200af3759227b6d7d"),
+		report.Host:           report.MakeHostNodeID("ip-123-45-6-100"),
+		report.Overlay:        report.MakeOverlayNodeID("", "3e:ca:14:ca:12:5c"),
+		processNameTopology:   "/home/weave/scope",
+	} {
+		summary, b := detailed.MakeNodeSummary(report.RenderContext{}, report.MakeNode(id).WithTopology(topology))
+		switch {
+		case !b:
+			t.Errorf("Node Summary missing for topology %s, id %s", topology, id)
+		case summary.Label == "":
+			t.Errorf("Node Summary Label missing for topology %s, id %s", topology, id)
+		case summary.Label == id && topology != processNameTopology:
+			t.Errorf("Node Summary Label same as id (that's cheating!) for topology %s, id %s", topology, id)
+		}
+	}
+}
+
 func TestNodeMetadata(t *testing.T) {
 	inputs := []struct {
 		name string

From 367f2db0030f26c40a93d547a3d7e0d04fb82f23 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sun, 24 Dec 2017 11:07:42 +0000
Subject: [PATCH 17/24] always render metric urls

Fixes an omission.
---
 render/detailed/summary.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 0b39a5b38..7d05eae17 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -111,7 +111,7 @@ func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool)
 	} else if _, ok := rc.Topology(n.Topology); ok {
 		summary := baseNodeSummary(r, n)
 		summary.Label = n.ID // This is unlikely to look very good, but is a reasonable fallback
-		return summary, true
+		return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
 	}
 	if strings.HasPrefix(n.Topology, "group:") {
 		summary := groupNodeSummary(baseNodeSummary(r, n), r, n)

From 0237b13916b85af3a104a9fc630c08c3cab5b0c4 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sun, 24 Dec 2017 12:05:38 +0000
Subject: [PATCH 18/24] refactor: extract BasicNodeSummary

This is sufficient for rendering links to nodes, and is cheaper to
compute that the full NodeSummary.
---
 render/detailed/links_test.go         |  24 +++--
 render/detailed/node_test.go          |  50 +++++-----
 render/detailed/summary.go            | 134 ++++++++++++++------------
 render/detailed/summary_test.go       |  72 ++++++++------
 render/detailed/topology_diff_test.go |  18 ++--
 5 files changed, 167 insertions(+), 131 deletions(-)

diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go
index b88e0456d..9552e1da6 100644
--- a/render/detailed/links_test.go
+++ b/render/detailed/links_test.go
@@ -31,8 +31,17 @@ var (
 	}
 )
 
+func nodeSummaryWithMetrics(label string, metrics []report.MetricRow) detailed.NodeSummary {
+	return detailed.NodeSummary{
+		BasicNodeSummary: detailed.BasicNodeSummary{
+			Label: label,
+		},
+		Metrics: metrics,
+	}
+}
+
 func TestRenderMetricURLs_Disabled(t *testing.T) {
-	s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics}
+	s := nodeSummaryWithMetrics("foo", sampleMetrics)
 	result := detailed.RenderMetricURLs(s, samplePodNode, "")
 
 	assert.Empty(t, result.Metrics[0].URL)
@@ -40,7 +49,7 @@ func TestRenderMetricURLs_Disabled(t *testing.T) {
 }
 
 func TestRenderMetricURLs_UnknownTopology(t *testing.T) {
-	s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics}
+	s := nodeSummaryWithMetrics("foo", sampleMetrics)
 	result := detailed.RenderMetricURLs(s, sampleUnknownNode, sampleMetricsGraphURL)
 
 	assert.Empty(t, result.Metrics[0].URL)
@@ -48,7 +57,7 @@ func TestRenderMetricURLs_UnknownTopology(t *testing.T) {
 }
 
 func TestRenderMetricURLs_Pod(t *testing.T) {
-	s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics}
+	s := nodeSummaryWithMetrics("foo", sampleMetrics)
 	result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL)
 
 	checkURL(t, result.Metrics[0].URL, sampleMetricsGraphURL,
@@ -58,7 +67,7 @@ func TestRenderMetricURLs_Pod(t *testing.T) {
 }
 
 func TestRenderMetricURLs_Container(t *testing.T) {
-	s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics}
+	s := nodeSummaryWithMetrics("foo", sampleMetrics)
 	result := detailed.RenderMetricURLs(s, sampleContainerNode, sampleMetricsGraphURL)
 
 	checkURL(t, result.Metrics[0].URL, sampleMetricsGraphURL,
@@ -84,10 +93,7 @@ func TestRenderMetricURLs_EmptyMetrics(t *testing.T) {
 }
 
 func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) {
-	s := detailed.NodeSummary{
-		Label:   "foo",
-		Metrics: []report.MetricRow{{ID: docker.MemoryUsage, Priority: 1}},
-	}
+	s := nodeSummaryWithMetrics("foo", []report.MetricRow{{ID: docker.MemoryUsage, Priority: 1}})
 	result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL)
 
 	assert.NotEmpty(t, result.Metrics[0].URL)
@@ -99,7 +105,7 @@ func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) {
 }
 
 func TestRenderMetricURLs_QueryReplacement(t *testing.T) {
-	s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics}
+	s := nodeSummaryWithMetrics("foo", sampleMetrics)
 	result := detailed.RenderMetricURLs(s, samplePodNode, "http://example.test/?q=:query")
 
 	checkURL(t, result.Metrics[0].URL, "http://example.test/?q=",
diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go
index 719625241..63c9ce710 100644
--- a/render/detailed/node_test.go
+++ b/render/detailed/node_test.go
@@ -43,14 +43,16 @@ func TestMakeDetailedHostNode(t *testing.T) {
 	podNodeSummary := child(t, render.PodRenderer, fixture.ClientPodNodeID)
 	want := detailed.Node{
 		NodeSummary: detailed.NodeSummary{
-			ID:         fixture.ClientHostNodeID,
-			Label:      "client",
-			LabelMinor: "hostname.com",
-			Rank:       "hostname.com",
-			Pseudo:     false,
-			Shape:      "circle",
-			Linkable:   true,
-			Adjacency:  report.MakeIDList(fixture.ServerHostNodeID),
+			BasicNodeSummary: detailed.BasicNodeSummary{
+				ID:         fixture.ClientHostNodeID,
+				Label:      "client",
+				LabelMinor: "hostname.com",
+				Rank:       "hostname.com",
+				Pseudo:     false,
+				Shape:      "circle",
+				Linkable:   true,
+			},
+			Adjacency: report.MakeIDList(fixture.ServerHostNodeID),
 			Metadata: []report.MetadataRow{
 				{
 					ID:       "host_name",
@@ -189,13 +191,15 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 	serverProcessNodeSummary.Linkable = true
 	want := detailed.Node{
 		NodeSummary: detailed.NodeSummary{
-			ID:         id,
-			Label:      "server",
-			LabelMinor: "server.hostname.com",
-			Rank:       fixture.ServerContainerImageName,
-			Shape:      "hexagon",
-			Linkable:   true,
-			Pseudo:     false,
+			BasicNodeSummary: detailed.BasicNodeSummary{
+				ID:         id,
+				Label:      "server",
+				LabelMinor: "server.hostname.com",
+				Rank:       fixture.ServerContainerImageName,
+				Shape:      "hexagon",
+				Linkable:   true,
+				Pseudo:     false,
+			},
 			Metadata: []report.MetadataRow{
 				{ID: "docker_image_name", Label: "Image", Value: fixture.ServerContainerImageName, Priority: 1},
 				{ID: "docker_container_state_human", Label: "State", Value: "running", Priority: 3},
@@ -320,13 +324,15 @@ func TestMakeDetailedPodNode(t *testing.T) {
 	serverProcessNodeSummary.Linkable = true // Temporary workaround for: https://github.com/weaveworks/scope/issues/1295
 	want := detailed.Node{
 		NodeSummary: detailed.NodeSummary{
-			ID:         id,
-			Label:      "pong-b",
-			LabelMinor: "1 container",
-			Rank:       "ping/pong-b",
-			Shape:      "heptagon",
-			Linkable:   true,
-			Pseudo:     false,
+			BasicNodeSummary: detailed.BasicNodeSummary{
+				ID:         id,
+				Label:      "pong-b",
+				LabelMinor: "1 container",
+				Rank:       "ping/pong-b",
+				Shape:      "heptagon",
+				Linkable:   true,
+				Pseudo:     false,
+			},
 			Metadata: []report.MetadataRow{
 				{ID: "kubernetes_state", Label: "State", Value: "running", Priority: 2},
 				{ID: "container", Label: "# Containers", Value: "1", Priority: 4, Datatype: report.Number},
diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 7d05eae17..25bd05a77 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -41,24 +41,30 @@ type Column struct {
 	Datatype    string `json:"dataType"`
 }
 
-// NodeSummary is summary information about a child for a Node.
-type NodeSummary struct {
-	ID         string               `json:"id"`
-	Label      string               `json:"label"`
-	LabelMinor string               `json:"labelMinor"`
-	Rank       string               `json:"rank"`
-	Shape      string               `json:"shape,omitempty"`
-	Stack      bool                 `json:"stack,omitempty"`
-	Linkable   bool                 `json:"linkable,omitempty"` // Whether this node can be linked-to
-	Pseudo     bool                 `json:"pseudo,omitempty"`
-	Metadata   []report.MetadataRow `json:"metadata,omitempty"`
-	Parents    []Parent             `json:"parents,omitempty"`
-	Metrics    []report.MetricRow   `json:"metrics,omitempty"`
-	Tables     []report.Table       `json:"tables,omitempty"`
-	Adjacency  report.IDList        `json:"adjacency,omitempty"`
+// BasicNodeSummary is basic summary information about a Node,
+// sufficient for rendering links to the node.
+type BasicNodeSummary struct {
+	ID         string `json:"id"`
+	Label      string `json:"label"`
+	LabelMinor string `json:"labelMinor"`
+	Rank       string `json:"rank"`
+	Shape      string `json:"shape,omitempty"`
+	Stack      bool   `json:"stack,omitempty"`
+	Linkable   bool   `json:"linkable,omitempty"` // Whether this node can be linked-to
+	Pseudo     bool   `json:"pseudo,omitempty"`
 }
 
-var renderers = map[string]func(NodeSummary, report.Node) NodeSummary{
+// NodeSummary is summary information about a Node.
+type NodeSummary struct {
+	BasicNodeSummary
+	Metadata  []report.MetadataRow `json:"metadata,omitempty"`
+	Parents   []Parent             `json:"parents,omitempty"`
+	Metrics   []report.MetricRow   `json:"metrics,omitempty"`
+	Tables    []report.Table       `json:"tables,omitempty"`
+	Adjacency report.IDList        `json:"adjacency,omitempty"`
+}
+
+var renderers = map[string]func(BasicNodeSummary, report.Node) BasicNodeSummary{
 	render.Pseudo:         pseudoNodeSummary,
 	report.Process:        processNodeSummary,
 	report.Container:      containerNodeSummary,
@@ -99,25 +105,51 @@ var primaryAPITopology = map[string]string{
 	report.Host:           "hosts",
 }
 
-// MakeNodeSummary summarizes a node, if possible.
-func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool) {
-	r := rc.Report
+// MakeBasicNodeSummary returns a basic summary of a node, if
+// possible. This summary is sufficient for rendering links to the node.
+func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, bool) {
+	summary := BasicNodeSummary{
+		ID:       n.ID,
+		Linkable: true,
+	}
+	if t, ok := r.Topology(n.Topology); ok {
+		summary.Shape = t.GetShape()
+	}
 	if renderer, ok := renderers[n.Topology]; ok {
 		// Skip (and don't fall through to fallback) if renderer maps to nil
 		if renderer != nil {
-			summary := renderer(baseNodeSummary(r, n), n)
-			return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
+			return renderer(summary, n), true
 		}
-	} else if _, ok := rc.Topology(n.Topology); ok {
-		summary := baseNodeSummary(r, n)
+	} else if _, ok := r.Topology(n.Topology); ok {
 		summary.Label = n.ID // This is unlikely to look very good, but is a reasonable fallback
-		return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
+		return summary, true
 	}
 	if strings.HasPrefix(n.Topology, "group:") {
-		summary := groupNodeSummary(baseNodeSummary(r, n), r, n)
-		return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
+		return groupNodeSummary(summary, r, n), true
 	}
-	return NodeSummary{}, false
+	return summary, false
+}
+
+// MakeNodeSummary summarizes a node, if possible.
+func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool) {
+	base, ok := MakeBasicNodeSummary(rc.Report, n)
+	if !ok {
+		return NodeSummary{}, false
+	}
+	summary := NodeSummary{
+		BasicNodeSummary: base,
+		Parents:          Parents(rc.Report, n),
+		Adjacency:        n.Adjacency,
+	}
+	// Only include metadata, metrics, tables when it's not a group node
+	if _, ok := n.Counters.Lookup(n.Topology); !ok {
+		if topology, ok := rc.Topology(n.Topology); ok {
+			summary.Metadata = topology.MetadataTemplates.MetadataRows(n)
+			summary.Metrics = topology.MetricTemplates.MetricRows(n)
+			summary.Tables = topology.TableTemplates.Tables(n)
+		}
+	}
+	return RenderMetricURLs(summary, n, rc.MetricsGraphURL), true
 }
 
 // SummarizeMetrics returns a copy of the NodeSummary where the metrics are
@@ -131,29 +163,7 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary {
 	return n
 }
 
-func baseNodeSummary(r report.Report, n report.Node) NodeSummary {
-	summary := NodeSummary{
-		ID:        n.ID,
-		Linkable:  true,
-		Parents:   Parents(r, n),
-		Adjacency: n.Adjacency,
-	}
-	if t, ok := r.Topology(n.Topology); ok {
-		summary.Shape = t.GetShape()
-	}
-	if _, ok := n.Counters.Lookup(n.Topology); ok {
-		// This is a group of nodes, so no metadata, metrics, tables
-		return summary
-	}
-	if topology, ok := r.Topology(n.Topology); ok {
-		summary.Metadata = topology.MetadataTemplates.MetadataRows(n)
-		summary.Metrics = topology.MetricTemplates.MetricRows(n)
-		summary.Tables = topology.TableTemplates.Tables(n)
-	}
-	return summary
-}
-
-func pseudoNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func pseudoNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	pseudoID, _ := render.ParsePseudoNodeID(n.ID)
 	base.Pseudo = true
 	base.Rank = pseudoID
@@ -195,7 +205,7 @@ func pseudoNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func processNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func processNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		hostID, pid, _   = report.ParseProcessNodeID(n.ID)
 		processName, _   = n.Latest.Lookup(process.Name)
@@ -223,7 +233,7 @@ func processNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func containerNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func containerNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		containerName = getRenderableContainerName(n)
 		hostName      = report.ExtractHostID(n)
@@ -241,7 +251,7 @@ func containerNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func containerImageNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func containerImageNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		imageName, _            = n.Latest.Lookup(docker.ImageName)
 		imageNameWithoutVersion = docker.ImageNameWithoutVersion(imageName)
@@ -263,7 +273,7 @@ func containerImageNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func addKubernetesLabelAndRank(base NodeSummary, n report.Node) NodeSummary {
+func addKubernetesLabelAndRank(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		name, _      = n.Latest.Lookup(kubernetes.Name)
 		namespace, _ = n.Latest.Lookup(kubernetes.Namespace)
@@ -277,7 +287,7 @@ func addKubernetesLabelAndRank(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func podNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func podNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base = addKubernetesLabelAndRank(base, n)
 	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
 	return base
@@ -290,7 +300,7 @@ var podGroupNodeTypeName = map[string]string{
 	report.CronJob:     "CronJob",
 }
 
-func podGroupNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func podGroupNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base = addKubernetesLabelAndRank(base, n)
 	base.Stack = true
 	// NB: pods are the highest aggregation level for which we display
@@ -304,7 +314,7 @@ func podGroupNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func ecsTaskNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func ecsTaskNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base.Label, _ = n.Latest.Lookup(awsecs.TaskFamily)
 	if base.Label == "" {
 		base.Label, _ = report.ParseECSTaskNodeID(n.ID)
@@ -312,13 +322,13 @@ func ecsTaskNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func ecsServiceNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func ecsServiceNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	_, base.Label, _ = report.ParseECSServiceNodeID(n.ID)
 	base.Stack = true
 	return base
 }
 
-func swarmServiceNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func swarmServiceNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base.Label, _ = n.Latest.Lookup(docker.ServiceName)
 	if base.Label == "" {
 		base.Label, _ = report.ParseSwarmServiceNodeID(n.ID)
@@ -326,7 +336,7 @@ func swarmServiceNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func hostNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func hostNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		hostname, _ = report.ParseHostNodeID(n.ID)
 		parts       = strings.SplitN(hostname, ".", 2)
@@ -339,7 +349,7 @@ func hostNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 	return base
 }
 
-func weaveNodeSummary(base NodeSummary, n report.Node) NodeSummary {
+func weaveNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		nickname, _ = n.Latest.Lookup(overlay.WeavePeerNickName)
 		_, peerName = report.ParseOverlayNodeID(n.ID)
@@ -355,7 +365,7 @@ func weaveNodeSummary(base NodeSummary, n report.Node) NodeSummary {
 
 // groupNodeSummary renders the summary for a group node. n.Topology is
 // expected to be of the form: group:container:hostname
-func groupNodeSummary(base NodeSummary, r report.Report, n report.Node) NodeSummary {
+func groupNodeSummary(base BasicNodeSummary, r report.Report, n report.Node) BasicNodeSummary {
 	base.Label, base.Rank = n.ID, n.ID
 	if topology, _, ok := render.ParseGroupNodeTopology(n.Topology); ok {
 		if t, ok := r.Topology(topology); ok {
diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go
index 3926f2013..05f9c2b73 100644
--- a/render/detailed/summary_test.go
+++ b/render/detailed/summary_test.go
@@ -106,11 +106,13 @@ func TestMakeNodeSummary(t *testing.T) {
 			input: expected.RenderedProcesses[fixture.ClientProcess1NodeID],
 			ok:    true,
 			want: detailed.NodeSummary{
-				ID:         fixture.ClientProcess1NodeID,
-				Label:      fixture.Client1Name,
-				LabelMinor: "client.hostname.com (10001)",
-				Rank:       fixture.Client1Name,
-				Shape:      "square",
+				BasicNodeSummary: detailed.BasicNodeSummary{
+					ID:         fixture.ClientProcess1NodeID,
+					Label:      fixture.Client1Name,
+					LabelMinor: "client.hostname.com (10001)",
+					Rank:       fixture.Client1Name,
+					Shape:      "square",
+				},
 				Metadata: []report.MetadataRow{
 					{ID: process.PID, Label: "PID", Value: fixture.Client1PID, Priority: 1, Datatype: report.Number},
 				},
@@ -122,12 +124,14 @@ func TestMakeNodeSummary(t *testing.T) {
 			input: expected.RenderedContainers[fixture.ClientContainerNodeID],
 			ok:    true,
 			want: detailed.NodeSummary{
-				ID:         fixture.ClientContainerNodeID,
-				Label:      fixture.ClientContainerName,
-				LabelMinor: fixture.ClientHostName,
-				Rank:       fixture.ClientContainerImageName,
-				Shape:      "hexagon",
-				Linkable:   true,
+				BasicNodeSummary: detailed.BasicNodeSummary{
+					ID:         fixture.ClientContainerNodeID,
+					Label:      fixture.ClientContainerName,
+					LabelMinor: fixture.ClientHostName,
+					Rank:       fixture.ClientContainerImageName,
+					Shape:      "hexagon",
+					Linkable:   true,
+				},
 				Metadata: []report.MetadataRow{
 					{ID: docker.ImageName, Label: "Image", Value: fixture.ClientContainerImageName, Priority: 1},
 					{ID: docker.ContainerID, Label: "ID", Value: fixture.ClientContainerID, Priority: 10, Truncate: 12},
@@ -140,13 +144,15 @@ func TestMakeNodeSummary(t *testing.T) {
 			input: expected.RenderedContainerImages[expected.ClientContainerImageNodeID],
 			ok:    true,
 			want: detailed.NodeSummary{
-				ID:         expected.ClientContainerImageNodeID,
-				Label:      fixture.ClientContainerImageName,
-				LabelMinor: "1 container",
-				Rank:       fixture.ClientContainerImageName,
-				Shape:      "hexagon",
-				Linkable:   true,
-				Stack:      true,
+				BasicNodeSummary: detailed.BasicNodeSummary{
+					ID:         expected.ClientContainerImageNodeID,
+					Label:      fixture.ClientContainerImageName,
+					LabelMinor: "1 container",
+					Rank:       fixture.ClientContainerImageName,
+					Shape:      "hexagon",
+					Linkable:   true,
+					Stack:      true,
+				},
 				Metadata: []report.MetadataRow{
 					{ID: report.Container, Label: "# Containers", Value: "1", Priority: 2, Datatype: report.Number},
 				},
@@ -158,12 +164,14 @@ func TestMakeNodeSummary(t *testing.T) {
 			input: expected.RenderedHosts[fixture.ClientHostNodeID],
 			ok:    true,
 			want: detailed.NodeSummary{
-				ID:         fixture.ClientHostNodeID,
-				Label:      "client",
-				LabelMinor: "hostname.com",
-				Rank:       "hostname.com",
-				Shape:      "circle",
-				Linkable:   true,
+				BasicNodeSummary: detailed.BasicNodeSummary{
+					ID:         fixture.ClientHostNodeID,
+					Label:      "client",
+					LabelMinor: "hostname.com",
+					Rank:       "hostname.com",
+					Shape:      "circle",
+					Linkable:   true,
+				},
 				Metadata: []report.MetadataRow{
 					{ID: host.HostName, Label: "Hostname", Value: fixture.ClientHostName, Priority: 11},
 				},
@@ -175,13 +183,15 @@ func TestMakeNodeSummary(t *testing.T) {
 			input: expected.RenderedProcessNames[fixture.ServerName],
 			ok:    true,
 			want: detailed.NodeSummary{
-				ID:         "apache",
-				Label:      "apache",
-				LabelMinor: "1 process",
-				Rank:       "apache",
-				Shape:      "square",
-				Stack:      true,
-				Linkable:   true,
+				BasicNodeSummary: detailed.BasicNodeSummary{
+					ID:         "apache",
+					Label:      "apache",
+					LabelMinor: "1 process",
+					Rank:       "apache",
+					Shape:      "square",
+					Stack:      true,
+					Linkable:   true,
+				},
 			},
 		},
 	}
diff --git a/render/detailed/topology_diff_test.go b/render/detailed/topology_diff_test.go
index 3a630031a..dbceef44f 100644
--- a/render/detailed/topology_diff_test.go
+++ b/render/detailed/topology_diff_test.go
@@ -19,17 +19,21 @@ func (r ByID) Less(i, j int) bool { return r[i].ID < r[j].ID }
 
 func TestTopoDiff(t *testing.T) {
 	nodea := detailed.NodeSummary{
-		ID:         "nodea",
-		Label:      "Node A",
-		LabelMinor: "'ts an a",
-		Pseudo:     false,
-		Adjacency:  report.MakeIDList("nodeb"),
+		BasicNodeSummary: detailed.BasicNodeSummary{
+			ID:         "nodea",
+			Label:      "Node A",
+			LabelMinor: "'ts an a",
+			Pseudo:     false,
+		},
+		Adjacency: report.MakeIDList("nodeb"),
 	}
 	nodeap := nodea
 	nodeap.Adjacency = report.MakeIDList("nodeb", "nodeq") // not the same anymore
 	nodeb := detailed.NodeSummary{
-		ID:    "nodeb",
-		Label: "Node B",
+		BasicNodeSummary: detailed.BasicNodeSummary{
+			ID:    "nodeb",
+			Label: "Node B",
+		},
 	}
 
 	// Helper to make RenderableNode maps.

From 8cf65ea8b6fb34c7720572cac5ffa12cd9cf8f17 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sun, 24 Dec 2017 12:07:09 +0000
Subject: [PATCH 19/24] use BasicNodeSummary for rendering links in connection
 table

instead of the full NodeSummary
---
 render/detailed/connections.go | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/render/detailed/connections.go b/render/detailed/connections.go
index 1dd5eb859..cf252e134 100644
--- a/render/detailed/connections.go
+++ b/render/detailed/connections.go
@@ -125,10 +125,8 @@ func internetAddr(node report.Node, ep report.Node) (string, bool) {
 func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection {
 	output := []Connection{}
 	for row, count := range c.counts {
-		// Use MakeNodeSummary to render the id and label of this node
-		// TODO(paulbellamy): Would be cleaner if we hade just a
-		// MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary.
-		summary, _ := MakeNodeSummary(report.RenderContext{Report: r}, ns[row.remoteNodeID])
+		// Use MakeBasicNodeSummary to render the id and label of this node
+		summary, _ := MakeBasicNodeSummary(r, ns[row.remoteNodeID])
 		connection := Connection{
 			ID:         fmt.Sprintf("%s-%s-%s-%s", row.remoteNodeID, row.remoteAddr, row.localAddr, row.port),
 			NodeID:     summary.ID,

From b88a2e4509ead6fcb65c27048fd2c85aa639c96e Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sun, 24 Dec 2017 15:18:01 +0000
Subject: [PATCH 20/24] render sensible labels for parent nodes with little/no
 metadata

We eliminate the custom parent renderer, which was a very partial
implementation MakeBasicNodeSummary.

We also ensure that parents are always rendered in the same, sensible
order. Previously the order was an alphabetic sort of the parent
topology IDs. Now lower level topologies come before higher level
topologies.
---
 render/detailed/node_test.go    | 20 ++++-----
 render/detailed/parents.go      | 79 ++++++++++-----------------------
 render/detailed/parents_test.go |  6 +--
 3 files changed, 37 insertions(+), 68 deletions(-)

diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go
index 63c9ce710..1a43fc6f6 100644
--- a/render/detailed/node_test.go
+++ b/render/detailed/node_test.go
@@ -229,16 +229,16 @@ func TestMakeDetailedContainerNode(t *testing.T) {
 					Label:      fixture.ServerContainerImageName,
 					TopologyID: "containers-by-image",
 				},
-				{
-					ID:         fixture.ServerHostNodeID,
-					Label:      fixture.ServerHostName,
-					TopologyID: "hosts",
-				},
 				{
 					ID:         fixture.ServerPodNodeID,
 					Label:      "pong-b",
 					TopologyID: "pods",
 				},
+				{
+					ID:         fixture.ServerHostNodeID,
+					Label:      "server",
+					TopologyID: "hosts",
+				},
 			},
 		},
 		Controls: []detailed.ControlInstance{},
@@ -339,16 +339,16 @@ func TestMakeDetailedPodNode(t *testing.T) {
 				{ID: "kubernetes_namespace", Label: "Namespace", Value: "ping", Priority: 5},
 			},
 			Parents: []detailed.Parent{
-				{
-					ID:         fixture.ServerHostNodeID,
-					Label:      fixture.ServerHostName,
-					TopologyID: "hosts",
-				},
 				{
 					ID:         fixture.ServiceNodeID,
 					Label:      fixture.ServiceName,
 					TopologyID: "services",
 				},
+				{
+					ID:         fixture.ServerHostNodeID,
+					Label:      "server",
+					TopologyID: "hosts",
+				},
 			},
 		},
 		Controls: []detailed.ControlInstance{},
diff --git a/render/detailed/parents.go b/render/detailed/parents.go
index e7fa2ccca..0ad1a44c4 100644
--- a/render/detailed/parents.go
+++ b/render/detailed/parents.go
@@ -1,12 +1,6 @@
 package detailed
 
 import (
-	"sort"
-
-	"github.com/weaveworks/scope/probe/awsecs"
-	"github.com/weaveworks/scope/probe/docker"
-	"github.com/weaveworks/scope/probe/host"
-	"github.com/weaveworks/scope/probe/kubernetes"
 	"github.com/weaveworks/scope/report"
 )
 
@@ -17,24 +11,21 @@ type Parent struct {
 	TopologyID string `json:"topologyId"`
 }
 
-var (
-	kubernetesParentLabel = latestLookup(kubernetes.Name)
-
-	getLabelForTopology = map[string]func(report.Node) string{
-		report.Container:      getRenderableContainerName,
-		report.Pod:            kubernetesParentLabel,
-		report.Deployment:     kubernetesParentLabel,
-		report.DaemonSet:      kubernetesParentLabel,
-		report.StatefulSet:    kubernetesParentLabel,
-		report.CronJob:        kubernetesParentLabel,
-		report.Service:        kubernetesParentLabel,
-		report.ECSTask:        latestLookup(awsecs.TaskFamily),
-		report.ECSService:     ecsServiceParentLabel,
-		report.SwarmService:   latestLookup(docker.ServiceName),
-		report.ContainerImage: containerImageParentLabel,
-		report.Host:           latestLookup(host.HostName),
-	}
-)
+// parent topologies, in the order we want to show them
+var parentTopologies = []string{
+	report.Container,
+	report.ContainerImage,
+	report.Pod,
+	report.Deployment,
+	report.DaemonSet,
+	report.StatefulSet,
+	report.CronJob,
+	report.Service,
+	report.ECSTask,
+	report.ECSService,
+	report.SwarmService,
+	report.Host,
+}
 
 // Parents renders the parents of this report.Node, which have been aggregated
 // from the probe reports.
@@ -43,13 +34,7 @@ func Parents(r report.Report, n report.Node) []Parent {
 		return nil
 	}
 	result := make([]Parent, 0, n.Parents.Size())
-	topologyIDs := []string{}
-	for topologyID := range getLabelForTopology {
-		topologyIDs = append(topologyIDs, topologyID)
-	}
-	sort.Strings(topologyIDs)
-	for _, topologyID := range topologyIDs {
-		getLabel := getLabelForTopology[topologyID]
+	for _, topologyID := range parentTopologies {
 		topology, ok := r.Topology(topologyID)
 		if !ok {
 			continue
@@ -63,7 +48,7 @@ func Parents(r report.Report, n report.Node) []Parent {
 			var parentNode report.Node
 			// Special case: container image parents should be empty nodes for some reason
 			if topologyID == report.ContainerImage {
-				parentNode = report.MakeNode(id)
+				parentNode = report.MakeNode(id).WithTopology(topologyID)
 			} else {
 				if parent, ok := topology.Nodes[id]; ok {
 					parentNode = parent
@@ -76,12 +61,13 @@ func Parents(r report.Report, n report.Node) []Parent {
 			if !ok {
 				continue
 			}
-
-			result = append(result, Parent{
-				ID:         id,
-				Label:      getLabel(parentNode),
-				TopologyID: apiTopologyID,
-			})
+			if summary, ok := MakeBasicNodeSummary(r, parentNode); ok {
+				result = append(result, Parent{
+					ID:         summary.ID,
+					Label:      summary.Label,
+					TopologyID: apiTopologyID,
+				})
+			}
 		}
 	}
 	if len(result) == 0 {
@@ -89,20 +75,3 @@ func Parents(r report.Report, n report.Node) []Parent {
 	}
 	return result
 }
-
-func latestLookup(key string) func(report.Node) string {
-	return func(n report.Node) string {
-		value, _ := n.Latest.Lookup(key)
-		return value
-	}
-}
-
-func ecsServiceParentLabel(n report.Node) string {
-	_, name, _ := report.ParseECSServiceNodeID(n.ID)
-	return name
-}
-
-func containerImageParentLabel(n report.Node) string {
-	name, _ := report.ParseContainerImageNodeID(n.ID)
-	return name
-}
diff --git a/render/detailed/parents_test.go b/render/detailed/parents_test.go
index 1a6cdf1d3..3332d651d 100644
--- a/render/detailed/parents_test.go
+++ b/render/detailed/parents_test.go
@@ -34,7 +34,7 @@ func TestParents(t *testing.T) {
 			name: "Container image",
 			node: render.ContainerImageRenderer.Render(fixture.Report).Nodes[expected.ClientContainerImageNodeID],
 			want: []detailed.Parent{
-				{ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"},
+				{ID: fixture.ClientHostNodeID, Label: "client", TopologyID: "hosts"},
 			},
 		},
 		{
@@ -42,15 +42,15 @@ func TestParents(t *testing.T) {
 			node: render.ContainerWithImageNameRenderer.Render(fixture.Report).Nodes[fixture.ClientContainerNodeID],
 			want: []detailed.Parent{
 				{ID: expected.ClientContainerImageNodeID, Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"},
-				{ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"},
 				{ID: fixture.ClientPodNodeID, Label: "pong-a", TopologyID: "pods"},
+				{ID: fixture.ClientHostNodeID, Label: "client", TopologyID: "hosts"},
 			},
 		},
 		{
 			node: render.ProcessRenderer.Render(fixture.Report).Nodes[fixture.ClientProcess1NodeID],
 			want: []detailed.Parent{
 				{ID: fixture.ClientContainerNodeID, Label: fixture.ClientContainerName, TopologyID: "containers"},
-				{ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"},
+				{ID: fixture.ClientHostNodeID, Label: "client", TopologyID: "hosts"},
 			},
 		},
 	} {

From 7c5e9339bb467bce27a5feeab9af8be3aaed73ad Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sun, 24 Dec 2017 16:26:18 +0000
Subject: [PATCH 21/24] render parents which we cannot resolve

The id and topology are enough to get some basic info for rendering.

This isn't just a fall-through for exceptional cases.
ContainerwithImageNameRenderer produces parent references using the
image name without version as the ID, and containers-by-image topology
uses that for grouping. By contrast, the container-image topology in
the report is keyed on docker image IDs.
---
 render/detailed/parents.go | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/render/detailed/parents.go b/render/detailed/parents.go
index 0ad1a44c4..80913d7bc 100644
--- a/render/detailed/parents.go
+++ b/render/detailed/parents.go
@@ -44,19 +44,10 @@ func Parents(r report.Report, n report.Node) []Parent {
 			if topologyID == n.Topology && id == n.ID {
 				continue
 			}
-
-			var parentNode report.Node
-			// Special case: container image parents should be empty nodes for some reason
-			if topologyID == report.ContainerImage {
+			parentNode, ok := topology.Nodes[id]
+			if !ok {
 				parentNode = report.MakeNode(id).WithTopology(topologyID)
-			} else {
-				if parent, ok := topology.Nodes[id]; ok {
-					parentNode = parent
-				} else {
-					continue
-				}
 			}
-
 			apiTopologyID, ok := primaryAPITopology[topologyID]
 			if !ok {
 				continue

From 4206760021a1b6dbb44b9e8ed525af312d4b392a Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Sun, 24 Dec 2017 17:21:10 +0000
Subject: [PATCH 22/24] refactor: lift some code out of inner loop

---
 render/detailed/parents.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/render/detailed/parents.go b/render/detailed/parents.go
index 80913d7bc..2a5bcc1a9 100644
--- a/render/detailed/parents.go
+++ b/render/detailed/parents.go
@@ -39,6 +39,10 @@ func Parents(r report.Report, n report.Node) []Parent {
 		if !ok {
 			continue
 		}
+		apiTopologyID, ok := primaryAPITopology[topologyID]
+		if !ok {
+			continue
+		}
 		parents, _ := n.Parents.Lookup(topologyID)
 		for _, id := range parents {
 			if topologyID == n.Topology && id == n.ID {
@@ -48,10 +52,6 @@ func Parents(r report.Report, n report.Node) []Parent {
 			if !ok {
 				parentNode = report.MakeNode(id).WithTopology(topologyID)
 			}
-			apiTopologyID, ok := primaryAPITopology[topologyID]
-			if !ok {
-				continue
-			}
 			if summary, ok := MakeBasicNodeSummary(r, parentNode); ok {
 				result = append(result, Parent{
 					ID:         summary.ID,

From 705c6d159dfaddda87f7a0b1fc4b7064cff4e2b4 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Wed, 27 Dec 2017 13:48:46 +0000
Subject: [PATCH 23/24] sensible defaults/fallback for label and shape

---
 render/detailed/summary.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 25bd05a77..36ffbe776 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -108,8 +108,10 @@ var primaryAPITopology = map[string]string{
 // MakeBasicNodeSummary returns a basic summary of a node, if
 // possible. This summary is sufficient for rendering links to the node.
 func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, bool) {
-	summary := BasicNodeSummary{
+	summary := BasicNodeSummary{ // This is unlikely to look very good, but is a reasonable fallback
 		ID:       n.ID,
+		Label:    n.ID,
+		Shape:    report.Triangle,
 		Linkable: true,
 	}
 	if t, ok := r.Topology(n.Topology); ok {
@@ -121,7 +123,6 @@ func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, boo
 			return renderer(summary, n), true
 		}
 	} else if _, ok := r.Topology(n.Topology); ok {
-		summary.Label = n.ID // This is unlikely to look very good, but is a reasonable fallback
 		return summary, true
 	}
 	if strings.HasPrefix(n.Topology, "group:") {

From 0b4512bd9b30060ac7beeb694410b6ba51421151 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Wed, 27 Dec 2017 13:49:34 +0000
Subject: [PATCH 24/24] refactor: clarify and comment on summarisation logic

---
 render/detailed/summary.go | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/render/detailed/summary.go b/render/detailed/summary.go
index 36ffbe776..d309c2835 100644
--- a/render/detailed/summary.go
+++ b/render/detailed/summary.go
@@ -117,17 +117,28 @@ func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, boo
 	if t, ok := r.Topology(n.Topology); ok {
 		summary.Shape = t.GetShape()
 	}
+
+	// Do we have a renderer for the topology?
 	if renderer, ok := renderers[n.Topology]; ok {
-		// Skip (and don't fall through to fallback) if renderer maps to nil
-		if renderer != nil {
-			return renderer(summary, n), true
+		if renderer == nil { // we don't want to render this
+			return summary, false
 		}
-	} else if _, ok := r.Topology(n.Topology); ok {
-		return summary, true
+		return renderer(summary, n), true
 	}
+
+	// Is it a group topology?
 	if strings.HasPrefix(n.Topology, "group:") {
 		return groupNodeSummary(summary, r, n), true
 	}
+
+	// Is it any known topology?
+	if _, ok := r.Topology(n.Topology); ok {
+		// We should never get here, since all known topologies are in
+		// 'renderers'.
+		return summary, true
+	}
+
+	// We have no idea how to render this.
 	return summary, false
 }