Merge pull request #2998 from weaveworks/node-label-fallbacks

render sensible labels for nodes with little/no metadata

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,

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=",

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},
@@ -225,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{},
@@ -320,29 +324,31 @@ 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},
 				{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{},

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,45 +34,31 @@ 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
 		}
+		apiTopologyID, ok := primaryAPITopology[topologyID]
+		if !ok {
+			continue
+		}
 		parents, _ := n.Parents.Lookup(topologyID)
 		for _, id := range parents {
 			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 = report.MakeNode(id)
-			} else {
-				if parent, ok := topology.Nodes[id]; ok {
-					parentNode = parent
-				} else {
-					continue
-				}
-			}
-
-			apiTopologyID, ok := primaryAPITopology[topologyID]
+			parentNode, ok := topology.Nodes[id]
 			if !ok {
-				continue
+				parentNode = report.MakeNode(id).WithTopology(topologyID)
+			}
+			if summary, ok := MakeBasicNodeSummary(r, parentNode); ok {
+				result = append(result, Parent{
+					ID:         summary.ID,
+					Label:      summary.Label,
+					TopologyID: apiTopologyID,
+				})
 			}
-
-			result = append(result, Parent{
-				ID:         id,
-				Label:      getLabel(parentNode),
-				TopologyID: apiTopologyID,
-			})
 		}
 	}
 	if len(result) == 0 {
@@ -89,20 +66,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
-}

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"},
 			},
 		},
 	} {

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"
@@ -42,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, bool){
+// 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,
@@ -100,25 +105,63 @@ 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{ // 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 {
+		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 {
-			summary, b := renderer(baseNodeSummary(r, n), n)
-			return RenderMetricURLs(summary, n, rc.MetricsGraphURL), b
+		if renderer == nil { // we don't want to render this
+			return summary, false
 		}
-	} 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 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
 	}
-	if strings.HasPrefix(n.Topology, "group:") {
-		summary, b := groupNodeSummary(baseNodeSummary(r, n), r, n)
-		return RenderMetricURLs(summary, n, rc.MetricsGraphURL), b
+
+	// We have no idea how to render this.
+	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
 	}
-	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
@@ -132,140 +175,134 @@ 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, bool) {
+func pseudoNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
+	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
-	}
-
-	// 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
-}
-
-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
-	}
-	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
-}
-
-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 {
-		base.Rank = docker.ImageNameWithoutVersion(imageName)
-	}
-
-	return base, true
-}
-
-func containerImageNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
-	imageName, ok := n.Latest.Lookup(docker.ImageName)
-	if !ok {
-		return NodeSummary{}, false
-	}
-
-	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]
+	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
 		}
 	}
-
-	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
-
-	return base, true
+	return base
 }
 
-func addKubernetesLabelAndRank(base NodeSummary, n report.Node) NodeSummary {
-	base.Label, _ = n.Latest.Lookup(kubernetes.Name)
-	namespace, _ := n.Latest.Lookup(kubernetes.Namespace)
+func processNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
+	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
+	}
+	base.Linkable = render.IsConnected(n)
+	return base
+}
+
+func containerNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
+	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
+}
+
+func containerImageNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
+	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)
+	}
+	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
+	base.Rank = base.Label
+	base.Stack = true
+	return base
+}
+
+func addKubernetesLabelAndRank(base BasicNodeSummary, n report.Node) BasicNodeSummary {
+	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
 }
 
-func podNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func podNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base = addKubernetesLabelAndRank(base, n)
 	base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers")
-
-	return base, true
+	return base
 }
 
 var podGroupNodeTypeName = map[string]string{
@@ -275,10 +312,9 @@ var podGroupNodeTypeName = map[string]string{
 	report.CronJob:     "CronJob",
 }
 
-func podGroupNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+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
 	// counts.
 	count := pluralize(n.Counters, report.Pod, "pod", "pods")
@@ -287,75 +323,72 @@ 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 BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base.Label, _ = n.Latest.Lookup(awsecs.TaskFamily)
-	return base, true
+	if base.Label == "" {
+		base.Label, _ = report.ParseECSTaskNodeID(n.ID)
+	}
+	return base
 }
 
-func ecsServiceNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func ecsServiceNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	_, 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 BasicNodeSummary, n report.Node) BasicNodeSummary {
 	base.Label, _ = n.Latest.Lookup(docker.ServiceName)
-	return base, true
+	if base.Label == "" {
+		base.Label, _ = report.ParseSwarmServiceNodeID(n.ID)
+	}
+	return base
 }
 
-func hostNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
+func hostNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
-		hostname, _ = n.Latest.Lookup(host.HostName)
+		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 BasicNodeSummary, n report.Node) BasicNodeSummary {
 	var (
 		nickname, _ = n.Latest.Lookup(overlay.WeavePeerNickName)
+		_, peerName = report.ParseOverlayNodeID(n.ID)
 	)
-
-	_, peerName := report.ParseOverlayNodeID(n.ID)
-
-	base.Label, base.LabelMinor = nickname, peerName
-
-	return base, true
+	if nickname != "" {
+		base.Label = nickname
+	} else {
+		base.Label = peerName
+	}
+	base.LabelMinor = peerName
+	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) {
-	parts := strings.Split(n.Topology, ":")
-	if len(parts) != 3 {
-		return NodeSummary{}, false
+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 {
+			base.Shape = t.GetShape()
+			if t.Label != "" {
+				base.LabelMinor = pluralize(n.Counters, topology, t.Label, t.LabelPlural)
+			}
+		}
 	}
-
-	label, ok := n.Latest.Lookup(parts[2])
-	if !ok {
-		return NodeSummary{}, false
-	}
-	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)
-	}
-
-	base.Shape = t.GetShape()
 	base.Stack = true
-	return base, true
+	return base
 }
 
 func pluralize(counters report.Counters, key, singular, plural string) string {
@@ -418,5 +451,9 @@ func getRenderableContainerName(nmd report.Node) string {
 			return label
 		}
 	}
-	return ""
+	containerID, _ := report.ParseContainerNodeID(nmd.ID)
+	if len(containerID) > 12 {
+		containerID = containerID[:12]
+	}
+	return containerID
 }

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,
+				},
 			},
 		},
 	}
@@ -198,6 +208,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

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.

render/id.go

@@ -23,11 +23,34 @@ 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}, ":")
 }
 
+// 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))

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)