Merge pull request #233 from tomwilkie/228-rm-mapped-node

Remove MappedNode & generalise render logic

Makefile

@@ -21,7 +21,7 @@ $(SCOPE_EXPORT): $(APP_EXE) $(PROBE_EXE) docker/*
 	$(SUDO) docker build -t $(SCOPE_IMAGE) docker/
 	$(SUDO) docker save $(SCOPE_IMAGE):latest | sudo $(DOCKER_SQUASH) -t $(SCOPE_IMAGE) | tee $@ | $(SUDO) docker load
 
-$(APP_EXE): app/*.go report/*.go xfer/*.go
+$(APP_EXE): app/*.go render/*.go report/*.go xfer/*.go
 
 $(PROBE_EXE): probe/*.go probe/tag/*.go report/*.go xfer/*.go
 

app/api_topologies.go

@@ -37,7 +37,7 @@ func makeTopologyList(rep Reporter) func(w http.ResponseWriter, r *http.Request)
 					subTopologies = append(subTopologies, APITopologyDesc{
 						Name:  subDef.human,
 						URL:   "/api/topology/" + subName,
-						Stats: stats(render(rpt, subDef.maps)),
+						Stats: stats(subDef.renderer.Render(rpt)),
 					})
 				}
 			}
@@ -45,7 +45,7 @@ func makeTopologyList(rep Reporter) func(w http.ResponseWriter, r *http.Request)
 				Name:          def.human,
 				URL:           "/api/topology/" + name,
 				SubTopologies: subTopologies,
-				Stats:         stats(render(rpt, def.maps)),
+				Stats:         stats(def.renderer.Render(rpt)),
 			})
 		}
 		respondWith(w, http.StatusOK, topologies)

app/api_topology.go

@@ -30,19 +30,10 @@ type APIEdge struct {
 	Metadata report.AggregateMetadata `json:"metadata"`
 }
 
-func render(rpt report.Report, maps []topologyMapper) report.RenderableNodes {
-	result := report.RenderableNodes{}
-	for _, m := range maps {
-		rns := m.selector(rpt).RenderBy(m.mapper, m.pseudo)
-		result.Merge(rns)
-	}
-	return result
-}
-
 // Full topology.
 func handleTopology(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Request) {
 	respondWith(w, http.StatusOK, APITopology{
-		Nodes: render(rep.Report(), t.maps),
+		Nodes: t.renderer.Render(rep.Report()),
 	})
 }
 
@@ -69,7 +60,7 @@ func handleNode(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Req
 		vars     = mux.Vars(r)
 		nodeID   = vars["id"]
 		rpt      = rep.Report()
-		node, ok = render(rpt, t.maps)[nodeID]
+		node, ok = t.renderer.Render(rep.Report())[nodeID]
 	)
 	if !ok {
 		http.NotFound(w, r)
@@ -85,13 +76,9 @@ func handleEdge(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Req
 		localID  = vars["local"]
 		remoteID = vars["remote"]
 		rpt      = rep.Report()
-		metadata = report.AggregateMetadata{}
+		metadata = t.renderer.AggregateMetadata(rpt, localID, remoteID)
 	)
 
-	for _, m := range t.maps {
-		metadata.Merge(m.selector(rpt).EdgeMetadata(m.mapper, localID, remoteID).Transform())
-	}
-
 	respondWith(w, http.StatusOK, APIEdge{Metadata: metadata})
 }
 
@@ -128,7 +115,7 @@ func handleWebsocket(
 		tick         = time.Tick(loop)
 	)
 	for {
-		newTopo := render(rep.Report(), t.maps)
+		newTopo := t.renderer.Render(rep.Report())
 		diff := report.TopoDiff(previousTopo, newTopo)
 		previousTopo = newTopo
 

app/router.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/gorilla/mux"
 
+	"github.com/weaveworks/scope/render"
 	"github.com/weaveworks/scope/report"
 )
 
@@ -47,51 +48,37 @@ func apiHandler(w http.ResponseWriter, r *http.Request) {
 
 var topologyRegistry = map[string]topologyView{
 	"applications": {
-		human:  "Applications",
-		parent: "",
-		maps: []topologyMapper{
-			{report.SelectEndpoint, report.ProcessPID, report.GenericPseudoNode},
-		},
+		human:    "Applications",
+		parent:   "",
+		renderer: render.Map{Selector: report.SelectEndpoint, Mapper: report.ProcessPID, Pseudo: report.GenericPseudoNode},
 	},
 	"applications-by-name": {
-		human:  "by name",
-		parent: "applications",
-		maps: []topologyMapper{
-			{report.SelectEndpoint, report.ProcessName, report.GenericGroupedPseudoNode},
-		},
+		human:    "by name",
+		parent:   "applications",
+		renderer: render.Map{Selector: report.SelectEndpoint, Mapper: report.ProcessName, Pseudo: report.GenericGroupedPseudoNode},
 	},
 	"containers": {
 		human:  "Containers",
 		parent: "",
-		maps: []topologyMapper{
-			{report.SelectEndpoint, report.MapEndpoint2Container, report.InternetOnlyPseudoNode},
-			{report.SelectContainer, report.MapContainerIdentity, report.InternetOnlyPseudoNode},
-		},
+		renderer: render.Reduce([]render.Renderer{
+			render.Map{Selector: report.SelectEndpoint, Mapper: report.MapEndpoint2Container, Pseudo: report.InternetOnlyPseudoNode},
+			render.Map{Selector: report.SelectContainer, Mapper: report.MapContainerIdentity, Pseudo: report.InternetOnlyPseudoNode},
+		}),
 	},
 	"containers-by-image": {
-		human:  "by image",
-		parent: "containers",
-		maps: []topologyMapper{
-			{report.SelectEndpoint, report.ProcessContainerImage, report.InternetOnlyPseudoNode},
-		},
+		human:    "by image",
+		parent:   "containers",
+		renderer: render.Map{Selector: report.SelectEndpoint, Mapper: report.ProcessContainerImage, Pseudo: report.InternetOnlyPseudoNode},
 	},
 	"hosts": {
-		human:  "Hosts",
-		parent: "",
-		maps: []topologyMapper{
-			{report.SelectAddress, report.NetworkHostname, report.GenericPseudoNode},
-		},
+		human:    "Hosts",
+		parent:   "",
+		renderer: render.Map{Selector: report.SelectAddress, Mapper: report.NetworkHostname, Pseudo: report.GenericPseudoNode},
 	},
 }
 
 type topologyView struct {
-	human  string
-	parent string
-	maps   []topologyMapper
-}
-
-type topologyMapper struct {
-	selector report.TopologySelector
-	mapper   report.MapFunc
-	pseudo   report.PseudoFunc
+	human    string
+	parent   string
+	renderer render.Renderer
 }

circle.yml

@@ -25,6 +25,7 @@ dependencies:
         mv scope_ui_build.tar $(dirname "$SCOPE_UI_BUILD");
       fi
   post:
+    - go version
     - go clean -i net
     - go install -tags netgo std
     - make deps

render/render.go

@@ -0,0 +1,51 @@
+package render
+
+import (
+	"github.com/weaveworks/scope/report"
+)
+
+// Renderer is something that can render a report to a set of RenderableNodes
+type Renderer interface {
+	Render(report.Report) report.RenderableNodes
+	AggregateMetadata(rpt report.Report, localID, remoteID string) report.AggregateMetadata
+}
+
+// Reduce renderer is a Renderer which merges together the output of several
+// other renderers
+type Reduce []Renderer
+
+// Render produces a set of RenderableNodes given a Report
+func (r Reduce) Render(rpt report.Report) report.RenderableNodes {
+	result := report.RenderableNodes{}
+	for _, renderer := range r {
+		result.Merge(renderer.Render(rpt))
+	}
+	return result
+}
+
+// AggregateMetadata produces an AggregateMetadata for a given edge
+func (r Reduce) AggregateMetadata(rpt report.Report, localID, remoteID string) report.AggregateMetadata {
+	metadata := report.AggregateMetadata{}
+	for _, renderer := range r {
+		metadata.Merge(renderer.AggregateMetadata(rpt, localID, remoteID))
+	}
+	return metadata
+}
+
+// Map is a Renderer which produces a set of RendererNodes by using a
+// Mapper functions and topology selector.
+type Map struct {
+	Selector report.TopologySelector
+	Mapper   report.MapFunc
+	Pseudo   report.PseudoFunc
+}
+
+// Render produces a set of RenderableNodes given a Report
+func (m Map) Render(rpt report.Report) report.RenderableNodes {
+	return m.Selector(rpt).RenderBy(m.Mapper, m.Pseudo)
+}
+
+// AggregateMetadata produces an AggregateMetadata for a given edge
+func (m Map) AggregateMetadata(rpt report.Report, localID, remoteID string) report.AggregateMetadata {
+	return m.Selector(rpt).EdgeMetadata(m.Mapper, localID, remoteID).Transform()
+}

render/render_test.go

@@ -0,0 +1,49 @@
+package render_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/weaveworks/scope/render"
+	"github.com/weaveworks/scope/report"
+)
+
+type mockRenderer struct {
+	report.RenderableNodes
+	aggregateMetadata report.AggregateMetadata
+}
+
+func (m mockRenderer) Render(rpt report.Report) report.RenderableNodes {
+	return m.RenderableNodes
+}
+func (m mockRenderer) AggregateMetadata(rpt report.Report, localID, remoteID string) report.AggregateMetadata {
+	return m.aggregateMetadata
+}
+
+func TestReduceRender(t *testing.T) {
+	renderer := render.Reduce([]render.Renderer{
+		mockRenderer{RenderableNodes: report.RenderableNodes{"foo": {ID: "foo"}}},
+		mockRenderer{RenderableNodes: report.RenderableNodes{"bar": {ID: "bar"}}},
+	})
+
+	want := report.RenderableNodes{"foo": {ID: "foo"}, "bar": {ID: "bar"}}
+	have := renderer.Render(report.MakeReport())
+
+	if !reflect.DeepEqual(want, have) {
+		t.Errorf("want %+v, have %+v", want, have)
+	}
+}
+
+func TestReduceEdge(t *testing.T) {
+	renderer := render.Reduce([]render.Renderer{
+		mockRenderer{aggregateMetadata: report.AggregateMetadata{"foo": 1}},
+		mockRenderer{aggregateMetadata: report.AggregateMetadata{"bar": 2}},
+	})
+
+	want := report.AggregateMetadata{"foo": 1, "bar": 2}
+	have := renderer.AggregateMetadata(report.MakeReport(), "", "")
+
+	if !reflect.DeepEqual(want, have) {
+		t.Errorf("want %+v, have %+v", want, have)
+	}
+}

report/mapping.go

@@ -7,12 +7,26 @@ import (
 
 const humanTheInternet = "the Internet"
 
-// MappedNode is returned by the MapFuncs.
-type MappedNode struct {
-	ID    string
-	Major string
-	Minor string
-	Rank  string
+func newRenderableNode(id, major, minor, rank string) RenderableNode {
+	return RenderableNode{
+		ID:         id,
+		LabelMajor: major,
+		LabelMinor: minor,
+		Rank:       rank,
+		Pseudo:     false,
+		Metadata:   AggregateMetadata{},
+	}
+}
+
+func newPseudoNode(id, major, minor string) RenderableNode {
+	return RenderableNode{
+		ID:         id,
+		LabelMajor: major,
+		LabelMinor: minor,
+		Rank:       "",
+		Pseudo:     true,
+		Metadata:   AggregateMetadata{},
+	}
 }
 
 // MapFunc is anything which can take an arbitrary NodeMetadata, which is
@@ -25,13 +39,13 @@ type MappedNode struct {
 //
 // If the final output parameter is false, the node shall be omitted from the
 // rendered topology.
-type MapFunc func(string, NodeMetadata) (MappedNode, bool)
+type MapFunc func(NodeMetadata) (RenderableNode, bool)
 
-// PseudoFunc creates MappedNode representing pseudo nodes given the dstNodeID.
+// PseudoFunc creates RenderableNode representing pseudo nodes given the dstNodeID.
 // The srcNode renderable node is essentially from MapFunc, representing one of
 // the rendered nodes this pseudo node refers to. srcNodeID and dstNodeID are
 // node IDs prior to mapping.
-type PseudoFunc func(srcNodeID string, srcNode RenderableNode, dstNodeID string) (MappedNode, bool)
+type PseudoFunc func(srcNodeID string, srcNode RenderableNode, dstNodeID string) (RenderableNode, bool)
 
 // TopologySelector selects a single topology from a report.
 type TopologySelector func(r Report) Topology
@@ -54,39 +68,29 @@ func SelectContainer(r Report) Topology {
 // ProcessPID takes a node NodeMetadata from topology, and returns a
 // representation with the ID based on the process PID and the labels based on
 // the process name.
-func ProcessPID(_ string, m NodeMetadata) (MappedNode, bool) {
+func ProcessPID(m NodeMetadata) (RenderableNode, bool) {
 	var (
 		identifier = fmt.Sprintf("%s:%s:%s", "pid", m["domain"], m["pid"])
 		minor      = fmt.Sprintf("%s (%s)", m["domain"], m["pid"])
 		show       = m["pid"] != "" && m["name"] != ""
 	)
 
-	return MappedNode{
-		ID:    identifier,
-		Major: m["name"],
-		Minor: minor,
-		Rank:  m["pid"],
-	}, show
+	return newRenderableNode(identifier, m["name"], minor, m["pid"]), show
 }
 
 // ProcessName takes a node NodeMetadata from a topology, and returns a
 // representation with the ID based on the process name (grouping all
 // processes with the same name together).
-func ProcessName(_ string, m NodeMetadata) (MappedNode, bool) {
+func ProcessName(m NodeMetadata) (RenderableNode, bool) {
 	show := m["pid"] != "" && m["name"] != ""
-	return MappedNode{
-		ID:    m["name"],
-		Major: m["name"],
-		Minor: "",
-		Rank:  m["name"],
-	}, show
+	return newRenderableNode(m["name"], m["name"], "", m["name"]), show
 }
 
 // MapEndpoint2Container maps endpoint topology nodes to the containers they run
 // in. We consider container and image IDs to be globally unique, and so don't
 // scope them further by e.g. host. If no container metadata is found, nodes are
 // grouped into the Uncontained node.
-func MapEndpoint2Container(_ string, m NodeMetadata) (MappedNode, bool) {
+func MapEndpoint2Container(m NodeMetadata) (RenderableNode, bool) {
 	var id, major, minor, rank string
 	if m["docker_container_id"] == "" {
 		id, major, minor, rank = "uncontained", "Uncontained", "", "uncontained"
@@ -94,16 +98,11 @@ func MapEndpoint2Container(_ string, m NodeMetadata) (MappedNode, bool) {
 		id, major, minor, rank = m["docker_container_id"], "", m["domain"], ""
 	}
 
-	return MappedNode{
-		ID:    id,
-		Major: major,
-		Minor: minor,
-		Rank:  rank,
-	}, true
+	return newRenderableNode(id, major, minor, rank), true
 }
 
 // MapContainerIdentity maps container topology node to container mapped nodes.
-func MapContainerIdentity(_ string, m NodeMetadata) (MappedNode, bool) {
+func MapContainerIdentity(m NodeMetadata) (RenderableNode, bool) {
 	var id, major, minor, rank string
 	if m["docker_container_id"] == "" {
 		id, major, minor, rank = "uncontained", "Uncontained", "", "uncontained"
@@ -111,18 +110,13 @@ func MapContainerIdentity(_ string, m NodeMetadata) (MappedNode, bool) {
 		id, major, minor, rank = m["docker_container_id"], m["docker_container_name"], m["domain"], m["docker_image_id"]
 	}
 
-	return MappedNode{
-		ID:    id,
-		Major: major,
-		Minor: minor,
-		Rank:  rank,
-	}, true
+	return newRenderableNode(id, major, minor, rank), true
 }
 
 // ProcessContainerImage maps topology nodes to the container images they run
 // on. If no container metadata is found, nodes are grouped into the
 // Uncontained node.
-func ProcessContainerImage(_ string, m NodeMetadata) (MappedNode, bool) {
+func ProcessContainerImage(m NodeMetadata) (RenderableNode, bool) {
 	var id, major, minor, rank string
 	if m["docker_image_id"] == "" {
 		id, major, minor, rank = "uncontained", "Uncontained", "", "uncontained"
@@ -130,18 +124,13 @@ func ProcessContainerImage(_ string, m NodeMetadata) (MappedNode, bool) {
 		id, major, minor, rank = m["docker_image_id"], m["docker_image_name"], "", m["docker_image_id"]
 	}
 
-	return MappedNode{
-		ID:    id,
-		Major: major,
-		Minor: minor,
-		Rank:  rank,
-	}, true
+	return newRenderableNode(id, major, minor, rank), true
 }
 
 // NetworkHostname takes a node NodeMetadata and returns a representation
 // based on the hostname. Major label is the hostname, the minor label is the
 // domain, if any.
-func NetworkHostname(_ string, m NodeMetadata) (MappedNode, bool) {
+func NetworkHostname(m NodeMetadata) (RenderableNode, bool) {
 	var (
 		name   = m["name"]
 		domain = ""
@@ -152,17 +141,12 @@ func NetworkHostname(_ string, m NodeMetadata) (MappedNode, bool) {
 		domain = parts[1]
 	}
 
-	return MappedNode{
-		ID:    fmt.Sprintf("host:%s", name),
-		Major: parts[0],
-		Minor: domain,
-		Rank:  parts[0],
-	}, name != ""
+	return newRenderableNode(fmt.Sprintf("host:%s", name), parts[0], domain, parts[0]), name != ""
 }
 
 // GenericPseudoNode contains heuristics for building sensible pseudo nodes.
 // It should go away.
-func GenericPseudoNode(src string, srcMapped RenderableNode, dst string) (MappedNode, bool) {
+func GenericPseudoNode(src string, srcMapped RenderableNode, dst string) (RenderableNode, bool) {
 	var maj, min, outputID string
 
 	if dst == TheInternet {
@@ -178,16 +162,12 @@ func GenericPseudoNode(src string, srcMapped RenderableNode, dst string) (Mapped
 		maj, min = dstNodeAddr, ""
 	}
 
-	return MappedNode{
-		ID:    outputID,
-		Major: maj,
-		Minor: min,
-	}, true
+	return newPseudoNode(outputID, maj, min), true
 }
 
 // GenericGroupedPseudoNode contains heuristics for building sensible pseudo nodes.
 // It should go away.
-func GenericGroupedPseudoNode(src string, srcMapped RenderableNode, dst string) (MappedNode, bool) {
+func GenericGroupedPseudoNode(src string, srcMapped RenderableNode, dst string) (RenderableNode, bool) {
 	var maj, min, outputID string
 
 	if dst == TheInternet {
@@ -201,19 +181,15 @@ func GenericGroupedPseudoNode(src string, srcMapped RenderableNode, dst string)
 		maj, min = dstNodeAddr, ""
 	}
 
-	return MappedNode{
-		ID:    outputID,
-		Major: maj,
-		Minor: min,
-	}, true
+	return newPseudoNode(outputID, maj, min), true
 }
 
 // InternetOnlyPseudoNode never creates a pseudo node, unless it's the Internet.
-func InternetOnlyPseudoNode(_ string, _ RenderableNode, dst string) (MappedNode, bool) {
+func InternetOnlyPseudoNode(_ string, _ RenderableNode, dst string) (RenderableNode, bool) {
 	if dst == TheInternet {
-		return MappedNode{ID: TheInternet, Major: humanTheInternet}, true
+		return newPseudoNode(TheInternet, humanTheInternet, ""), true
 	}
-	return MappedNode{}, false
+	return RenderableNode{}, false
 }
 
 // trySplitAddr is basically ParseArbitraryNodeID, since its callsites

report/mapping_test.go

@@ -86,17 +86,17 @@ func TestUngroupedMapping(t *testing.T) {
 	} {
 		identity := fmt.Sprintf("(%d %s %v)", i, c.id, c.meta)
 
-		m, haveOK := c.f(c.id, c.meta)
+		m, haveOK := c.f(c.meta)
 		if want, have := c.wantOK, haveOK; want != have {
 			t.Errorf("%s: map OK error: want %v, have %v", identity, want, have)
 		}
 		if want, have := c.wantID, m.ID; want != have {
 			t.Errorf("%s: map ID error: want %#v, have %#v", identity, want, have)
 		}
-		if want, have := c.wantMajor, m.Major; want != have {
+		if want, have := c.wantMajor, m.LabelMajor; want != have {
 			t.Errorf("%s: map major label: want %#v, have %#v", identity, want, have)
 		}
-		if want, have := c.wantMinor, m.Minor; want != have {
+		if want, have := c.wantMinor, m.LabelMinor; want != have {
 			t.Errorf("%s: map minor label: want %#v, have %#v", identity, want, have)
 		}
 		if want, have := c.wantRank, m.Rank; want != have {

report/topology.go

@@ -93,23 +93,21 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod
 		source2host   = map[string]string{} // source node ID -> origin host ID
 	)
 	for nodeID, metadata := range t.NodeMetadatas {
-		mapped, ok := mapFunc(nodeID, metadata)
+		mapped, ok := mapFunc(metadata)
 		if !ok {
 			continue
 		}
 
-		// mapped.ID needs not be unique over all addressIDs. If not, we just
-		// overwrite the existing data, on the assumption that the MapFunc
-		// returns the same data.
-		nodes[mapped.ID] = RenderableNode{
-			ID:         mapped.ID,
-			LabelMajor: mapped.Major,
-			LabelMinor: mapped.Minor,
-			Rank:       mapped.Rank,
-			Pseudo:     false,
-			Origins:    IDList{nodeID},
-			Metadata:   AggregateMetadata{}, // later
+		// mapped.ID needs not be unique over all addressIDs. If not, we merge with
+		// the existing data, on the assumption that the MapFunc returns the same
+		// data.
+		existing, ok := nodes[mapped.ID]
+		if ok {
+			mapped.Merge(existing)
 		}
+
+		mapped.Origins = mapped.Origins.Add(nodeID)
+		nodes[mapped.ID] = mapped
 		source2mapped[nodeID] = mapped.ID
 		source2host[nodeID] = metadata[HostNodeID]
 	}
@@ -136,13 +134,7 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod
 					continue
 				}
 				dstRenderableID = pseudoNode.ID
-				nodes[dstRenderableID] = RenderableNode{
-					ID:         pseudoNode.ID,
-					LabelMajor: pseudoNode.Major,
-					LabelMinor: pseudoNode.Minor,
-					Pseudo:     true,
-					Metadata:   AggregateMetadata{}, // populated below - or not?
-				}
+				nodes[dstRenderableID] = pseudoNode
 				source2mapped[dstNodeID] = dstRenderableID
 			}
 
@@ -174,11 +166,11 @@ func (t Topology) EdgeMetadata(mapFunc MapFunc, srcRenderableID, dstRenderableID
 			continue
 		}
 		if src != TheInternet {
-			mapped, _ := mapFunc(src, t.NodeMetadatas[src])
+			mapped, _ := mapFunc(t.NodeMetadatas[src])
 			src = mapped.ID
 		}
 		if dst != TheInternet {
-			mapped, _ := mapFunc(dst, t.NodeMetadatas[dst])
+			mapped, _ := mapFunc(t.NodeMetadatas[dst])
 			dst = mapped.ID
 		}
 		if src == srcRenderableID && dst == dstRenderableID {