Merge pull request #121 from tomwilkie/refactor

Refactor topology 'views' and their http handlers

app/api_topologies.go

@@ -34,15 +34,15 @@ func makeTopologyList(rep Reporter) func(w http.ResponseWriter, r *http.Request)
 
 			url := "/api/topology/" + name
 			var groupedURL string
-			if def.hasGrouped {
-				groupedURL = url + "grouped"
+			if def.groupedTopology != "" {
+				groupedURL = "/api/topology/" + def.groupedTopology
 			}
 
 			a = append(a, APITopologyDesc{
 				Name:       def.human,
 				URL:        url,
 				GroupedURL: groupedURL,
-				Stats:      stats(def.topologySelecter(rpt).RenderBy(def.MapFunc, def.PseudoFunc, false)),
+				Stats:      stats(def.selector(rpt).RenderBy(def.mapper, def.pseudo)),
 			})
 		}
 		respondWith(w, http.StatusOK, a)

app/api_topology.go

@@ -41,68 +41,57 @@ func selectNetwork(r report.Report) report.Topology {
 	return r.Network
 }
 
-// makeTopologyHandlers make /api/topology/* handlers.
-func makeTopologyHandlers(
-	rep Reporter,
-	topo topologySelecter,
-	mapping report.MapFunc,
-	pseudo report.PseudoFunc,
-	grouped bool,
-	get *mux.Router,
-	base string,
-) {
-	// Full topology.
-	get.HandleFunc(base, func(w http.ResponseWriter, r *http.Request) {
-		respondWith(w, http.StatusOK, APITopology{
-			Nodes: topo(rep.Report()).RenderBy(mapping, pseudo, grouped),
-		})
+// Full topology.
+func handleTopology(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Request) {
+	respondWith(w, http.StatusOK, APITopology{
+		Nodes: t.selector(rep.Report()).RenderBy(t.mapper, t.pseudo),
 	})
+}
 
-	// Websocket for the full topology. This route overlaps with the next.
-	get.HandleFunc(base+"/ws", func(w http.ResponseWriter, r *http.Request) {
-		if err := r.ParseForm(); err != nil {
-			respondWith(w, http.StatusInternalServerError, err.Error())
+// Websocket for the full topology. This route overlaps with the next.
+func handleWs(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Request) {
+	if err := r.ParseForm(); err != nil {
+		respondWith(w, http.StatusInternalServerError, err.Error())
+		return
+	}
+	loop := websocketLoop
+	if t := r.Form.Get("t"); t != "" {
+		var err error
+		if loop, err = time.ParseDuration(t); err != nil {
+			respondWith(w, http.StatusBadRequest, t)
 			return
 		}
-		loop := websocketLoop
-		if t := r.Form.Get("t"); t != "" {
-			var err error
-			if loop, err = time.ParseDuration(t); err != nil {
-				respondWith(w, http.StatusBadRequest, t)
-				return
-			}
-		}
-		handleWebsocket(w, r, rep, topo, mapping, pseudo, grouped, loop)
-	})
+	}
+	handleWebsocket(w, r, rep, t, loop)
+}
 
-	// Individual nodes.
-	get.HandleFunc(base+"/{id}", func(w http.ResponseWriter, r *http.Request) {
-		var (
-			vars     = mux.Vars(r)
-			nodeID   = vars["id"]
-			rpt      = rep.Report()
-			node, ok = topo(rpt).RenderBy(mapping, pseudo, grouped)[nodeID]
-		)
-		if !ok {
-			http.NotFound(w, r)
-			return
-		}
-		originHostFunc := func(id string) (OriginHost, bool) { return getOriginHost(rpt.HostMetadatas, id) }
-		originNodeFunc := func(id string) (OriginNode, bool) { return getOriginNode(topo(rpt), id) }
-		respondWith(w, http.StatusOK, APINode{Node: makeDetailed(node, originHostFunc, originNodeFunc)})
-	})
+// Individual nodes.
+func handleNode(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Request) {
+	var (
+		vars     = mux.Vars(r)
+		nodeID   = vars["id"]
+		rpt      = rep.Report()
+		node, ok = t.selector(rpt).RenderBy(t.mapper, t.pseudo)[nodeID]
+	)
+	if !ok {
+		http.NotFound(w, r)
+		return
+	}
+	originHostFunc := func(id string) (OriginHost, bool) { return getOriginHost(rpt.HostMetadatas, id) }
+	originNodeFunc := func(id string) (OriginNode, bool) { return getOriginNode(t.selector(rpt), id) }
+	respondWith(w, http.StatusOK, APINode{Node: makeDetailed(node, originHostFunc, originNodeFunc)})
+}
 
-	// Individual edges.
-	get.HandleFunc(base+"/{local}/{remote}", func(w http.ResponseWriter, r *http.Request) {
-		var (
-			vars     = mux.Vars(r)
-			localID  = vars["local"]
-			remoteID = vars["remote"]
-			rpt      = rep.Report()
-			metadata = topo(rpt).EdgeMetadata(mapping, grouped, localID, remoteID).Transform()
-		)
-		respondWith(w, http.StatusOK, APIEdge{Metadata: metadata})
-	})
+// Individual edges.
+func handleEdge(rep Reporter, t topologyView, w http.ResponseWriter, r *http.Request) {
+	var (
+		vars     = mux.Vars(r)
+		localID  = vars["local"]
+		remoteID = vars["remote"]
+		rpt      = rep.Report()
+		metadata = t.selector(rpt).EdgeMetadata(t.mapper, localID, remoteID).Transform()
+	)
+	respondWith(w, http.StatusOK, APIEdge{Metadata: metadata})
 }
 
 var upgrader = websocket.Upgrader{
@@ -113,10 +102,7 @@ func handleWebsocket(
 	w http.ResponseWriter,
 	r *http.Request,
 	rep Reporter,
-	topo topologySelecter,
-	mapping report.MapFunc,
-	psuedo report.PseudoFunc,
-	grouped bool,
+	t topologyView,
 	loop time.Duration,
 ) {
 	conn, err := upgrader.Upgrade(w, r, nil)
@@ -141,7 +127,7 @@ func handleWebsocket(
 		tick         = time.Tick(loop)
 	)
 	for {
-		newTopo := topo(rep.Report()).RenderBy(mapping, psuedo, grouped)
+		newTopo := t.selector(rep.Report()).RenderBy(t.mapper, t.pseudo)
 		diff := report.TopoDiff(previousTopo, newTopo)
 		previousTopo = newTopo
 

app/router.go

@@ -14,42 +14,39 @@ func Router(c Reporter) *mux.Router {
 	router := mux.NewRouter()
 	get := router.Methods("GET").Subrouter()
 	get.HandleFunc("/api/topology", makeTopologyList(c))
-	for name, def := range topologyRegistry {
-		makeTopologyHandlers(
-			c,
-			def.topologySelecter,
-			def.MapFunc,
-			def.PseudoFunc,
-			false, // not grouped
-			get,
-			"/api/topology/"+name,
-		)
-		if def.hasGrouped {
-			makeTopologyHandlers(
-				c,
-				def.topologySelecter,
-				def.MapFunc,
-				def.PseudoFunc,
-				true, // grouped
-				get,
-				"/api/topology/"+name+"grouped",
-			)
-		}
-	}
+	get.HandleFunc("/api/topology/{topology}", captureTopology(c, handleTopology))
+	get.HandleFunc("/api/topology/{topology}/ws", captureTopology(c, handleWs))
+	get.HandleFunc("/api/topology/{topology}/{id}", captureTopology(c, handleNode))
+	get.HandleFunc("/api/topology/{topology}/{local}/{remote}", captureTopology(c, handleEdge))
 	get.HandleFunc("/api/origin/host/{id}", makeOriginHostHandler(c))
 	get.HandleFunc("/api/report", makeRawReportHandler(c))
 	get.PathPrefix("/").Handler(http.FileServer(FS(false))) // everything else is static
 	return router
 }
 
-var topologyRegistry = map[string]struct {
-	human string
-	topologySelecter
-	report.MapFunc
-	report.PseudoFunc
-	hasGrouped bool
-}{
-	"applications": {"Applications", selectProcess, report.ProcessPID, report.GenericPseudoNode, true},
-	"containers":   {"Containers", selectProcess, report.ProcessContainer, report.NoPseudoNode, true},
-	"hosts":        {"Hosts", selectNetwork, report.NetworkHostname, report.GenericPseudoNode, false},
+func captureTopology(rep Reporter, f func(Reporter, topologyView, http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
+		topology, ok := topologyRegistry[mux.Vars(r)["topology"]]
+		if !ok {
+			http.NotFound(w, r)
+			return
+		}
+		f(rep, topology, w, r)
+	}
+}
+
+type topologyView struct {
+	human           string
+	selector        topologySelecter
+	mapper          report.MapFunc
+	pseudo          report.PseudoFunc
+	groupedTopology string
+}
+
+var topologyRegistry = map[string]topologyView{
+	"applications":         {"Applications", selectProcess, report.ProcessPID, report.GenericPseudoNode, "applications-grouped"},
+	"applications-grouped": {"Applications", selectProcess, report.ProcessName, report.GenericGroupedPseudoNode, ""},
+	"containers":           {"Containers", selectProcess, report.ProcessContainer, report.NoPseudoNode, "containers-grouped"},
+	"containers-grouped":   {"Containers", selectProcess, report.ProcessContainerImage, report.NoPseudoNode, ""},
+	"hosts":                {"Hosts", selectNetwork, report.NetworkHostname, report.GenericPseudoNode, ""},
 }

report/mapping_functions.go

@@ -23,29 +23,24 @@ type MappedNode struct {
 //
 // If the final output parameter is false, the node shall be omitted from the
 // rendered topology.
-type MapFunc func(string, NodeMetadata, bool) (MappedNode, bool)
+type MapFunc func(string, NodeMetadata) (MappedNode, bool)
 
 // PseudoFunc creates MappedNode 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, grouped bool) (MappedNode, bool)
+type PseudoFunc func(srcNodeID string, srcNode RenderableNode, dstNodeID string) (MappedNode, bool)
 
 // ProcessPID takes a node NodeMetadata from a Process 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, grouped bool) (MappedNode, bool) {
+func ProcessPID(_ string, m NodeMetadata) (MappedNode, 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"] != ""
 	)
 
-	if grouped {
-		identifier = m["name"] // flatten
-		minor = ""             // nothing meaningful to put here?
-	}
-
 	return MappedNode{
 		ID:    identifier,
 		Major: m["name"],
@@ -54,27 +49,47 @@ func ProcessPID(_ string, m NodeMetadata, grouped bool) (MappedNode, bool) {
 	}, show
 }
 
+// ProcessName takes a node NodeMetadata from a Process 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) {
+	show := m["pid"] != "" && m["name"] != ""
+	return MappedNode{
+		ID:    m["name"],
+		Major: m["name"],
+		Minor: "",
+		Rank:  m["name"],
+	}, show
+}
+
 // ProcessContainer maps Process 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. If grouped is true, nodes with the
-// same container image ID are merged together.
-func ProcessContainer(_ string, m NodeMetadata, grouped bool) (MappedNode, bool) {
-	var (
-		containerID   = m["docker_id"]
-		containerName = m["docker_name"]
-		imageID       = m["docker_image_id"]
-		imageName     = m["docker_image_name"]
-		domain        = m["domain"]
-	)
-
+// are grouped into the Uncontained node.
+func ProcessContainer(_ string, m NodeMetadata) (MappedNode, bool) {
 	var id, major, minor, rank string
-	if containerID == "" {
+	if m["docker_id"] == "" {
 		id, major, minor, rank = "uncontained", "Uncontained", "", "uncontained"
-	} else if grouped {
-		id, major, minor, rank = imageID, imageName, "", imageID
 	} else {
-		id, major, minor, rank = containerID, containerName, domain, imageID
+		id, major, minor, rank = m["docker_id"], m["docker_name"], m["domain"], m["docker_image_id"]
+	}
+
+	return MappedNode{
+		ID:    id,
+		Major: major,
+		Minor: minor,
+		Rank:  rank,
+	}, true
+}
+
+// ProcessContainerImage maps Process 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) {
+	var id, major, minor, rank string
+	if m["docker_image_id"] == "" {
+		id, major, minor, rank = "uncontained", "Uncontained", "", "uncontained"
+	} else {
+		id, major, minor, rank = m["docker_image_id"], m["docker_image_name"], "", m["docker_image_id"]
 	}
 
 	return MappedNode{
@@ -88,7 +103,7 @@ func ProcessContainer(_ string, m NodeMetadata, grouped bool) (MappedNode, bool)
 // NetworkHostname takes a node NodeMetadata from a Network topology, 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, _ bool) (MappedNode, bool) {
+func NetworkHostname(_ string, m NodeMetadata) (MappedNode, bool) {
 	var (
 		name   = m["name"]
 		domain = ""
@@ -109,18 +124,12 @@ func NetworkHostname(_ string, m NodeMetadata, _ bool) (MappedNode, bool) {
 
 // GenericPseudoNode contains heuristics for building sensible pseudo nodes.
 // It should go away.
-func GenericPseudoNode(src string, srcMapped RenderableNode, dst string, grouped bool) (MappedNode, bool) {
+func GenericPseudoNode(src string, srcMapped RenderableNode, dst string) (MappedNode, bool) {
 	var maj, min, outputID string
 
 	if dst == TheInternet {
 		outputID = dst
 		maj, min = "the Internet", ""
-	} else if grouped {
-		// When grouping, emit one pseudo node per (srcNodeAddress, dstNodeAddr)
-		dstNodeAddr, _ := trySplitAddr(dst)
-
-		outputID = strings.Join([]string{"pseudo:", dstNodeAddr, srcMapped.ID}, ScopeDelim)
-		maj, min = dstNodeAddr, ""
 	} else {
 		// Rule for non-internet psuedo nodes; emit 1 new node for each
 		// dstNodeAddr, srcNodeAddr, srcNodePort.
@@ -138,8 +147,31 @@ func GenericPseudoNode(src string, srcMapped RenderableNode, dst string, grouped
 	}, true
 }
 
+// GenericGroupedPseudoNode contains heuristics for building sensible pseudo nodes.
+// It should go away.
+func GenericGroupedPseudoNode(src string, srcMapped RenderableNode, dst string) (MappedNode, bool) {
+	var maj, min, outputID string
+
+	if dst == TheInternet {
+		outputID = dst
+		maj, min = "the Internet", ""
+	} else {
+		// When grouping, emit one pseudo node per (srcNodeAddress, dstNodeAddr)
+		dstNodeAddr, _ := trySplitAddr(dst)
+
+		outputID = strings.Join([]string{"pseudo:", dstNodeAddr, srcMapped.ID}, ScopeDelim)
+		maj, min = dstNodeAddr, ""
+	}
+
+	return MappedNode{
+		ID:    outputID,
+		Major: maj,
+		Minor: min,
+	}, true
+}
+
 // NoPseudoNode never creates a pseudo node.
-func NoPseudoNode(string, RenderableNode, string, bool) (MappedNode, bool) {
+func NoPseudoNode(string, RenderableNode, string) (MappedNode, bool) {
 	return MappedNode{}, false
 }
 

report/mapping_test.go

@@ -86,7 +86,7 @@ func TestUngroupedMapping(t *testing.T) {
 	} {
 		identity := fmt.Sprintf("(%d %s %v)", i, c.id, c.meta)
 
-		m, haveOK := c.f(c.id, c.meta, false)
+		m, haveOK := c.f(c.id, c.meta)
 		if want, have := c.wantOK, haveOK; want != have {
 			t.Errorf("%s: map OK error: want %v, have %v", identity, want, have)
 		}

report/topology.go

@@ -67,9 +67,9 @@ func NewTopology() Topology {
 // the UI will render collectively as a graph. Note that a RenderableNode will
 // always be rendered with other nodes, and therefore contains limited detail.
 //
-// RenderBy takes a a MapFunc, which defines how to group and label nodes. If
-// grouped is true, nodes that belong to the same "class" will be merged.
-func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc, grouped bool) map[string]RenderableNode {
+// RenderBy takes a a MapFunc, which defines how to group and label nodes. Npdes
+// with the same mapped IDs will be merged.
+func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) map[string]RenderableNode {
 	nodes := map[string]RenderableNode{}
 
 	// Build a set of RenderableNodes for all non-pseudo probes, and an
@@ -77,7 +77,7 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc, grouped bool)
 	// RenderableNodes.
 	address2mapped := map[string]string{}
 	for addressID, metadata := range t.NodeMetadatas {
-		mapped, ok := mapFunc(addressID, metadata, grouped)
+		mapped, ok := mapFunc(addressID, metadata)
 		if !ok {
 			continue
 		}
@@ -112,7 +112,7 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc, grouped bool)
 		for _, dstNodeAddress := range dsts {
 			dstRenderableID, ok := address2mapped[dstNodeAddress]
 			if !ok {
-				pseudoNode, ok := pseudoFunc(srcNodeAddress, srcRenderableNode, dstNodeAddress, grouped)
+				pseudoNode, ok := pseudoFunc(srcNodeAddress, srcRenderableNode, dstNodeAddress)
 				if !ok {
 					continue
 				}
@@ -146,18 +146,18 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc, grouped bool)
 // srcRenderableID. Since an edgeID can have multiple edges on the address
 // level, it uses the supplied mapping function to translate address IDs to
 // renderable node (mapped) IDs.
-func (t Topology) EdgeMetadata(mapFunc MapFunc, grouped bool, srcRenderableID, dstRenderableID string) EdgeMetadata {
+func (t Topology) EdgeMetadata(mapFunc MapFunc, srcRenderableID, dstRenderableID string) EdgeMetadata {
 	metadata := EdgeMetadata{}
 	for edgeID, edgeMeta := range t.EdgeMetadatas {
 		edgeParts := strings.SplitN(edgeID, IDDelim, 2)
 		src := edgeParts[0]
 		if src != TheInternet {
-			mapped, _ := mapFunc(src, t.NodeMetadatas[src], grouped)
+			mapped, _ := mapFunc(src, t.NodeMetadatas[src])
 			src = mapped.ID
 		}
 		dst := edgeParts[1]
 		if dst != TheInternet {
-			mapped, _ := mapFunc(dst, t.NodeMetadatas[dst], grouped)
+			mapped, _ := mapFunc(dst, t.NodeMetadatas[dst])
 			dst = mapped.ID
 		}
 		if src == srcRenderableID && dst == dstRenderableID {

report/topology_test.go

@@ -195,7 +195,7 @@ func TestRenderByProcessPID(t *testing.T) {
 			Metadata:   AggregateMetadata{},
 		},
 	}
-	have := report.Process.RenderBy(ProcessPID, GenericPseudoNode, false)
+	have := report.Process.RenderBy(ProcessPID, GenericPseudoNode)
 	if !reflect.DeepEqual(want, have) {
 		t.Error("\n" + diff(want, have))
 	}
@@ -210,7 +210,7 @@ func TestRenderByProcessPIDGrouped(t *testing.T) {
 			ID:          "curl",
 			LabelMajor:  "curl",
 			LabelMinor:  "",
-			Rank:        "10001",
+			Rank:        "curl",
 			Pseudo:      false,
 			Adjacency:   NewIDList("apache"),
 			OriginHosts: NewIDList("client.hostname.com"),
@@ -224,7 +224,7 @@ func TestRenderByProcessPIDGrouped(t *testing.T) {
 			ID:         "apache",
 			LabelMajor: "apache",
 			LabelMinor: "",
-			Rank:       "215",
+			Rank:       "apache",
 			Pseudo:     false,
 			Adjacency: NewIDList(
 				"curl",
@@ -251,7 +251,7 @@ func TestRenderByProcessPIDGrouped(t *testing.T) {
 			Metadata:   AggregateMetadata{},
 		},
 	}
-	have := report.Process.RenderBy(ProcessPID, GenericPseudoNode, true)
+	have := report.Process.RenderBy(ProcessName, GenericGroupedPseudoNode)
 	if !reflect.DeepEqual(want, have) {
 		t.Error("\n" + diff(want, have))
 	}
@@ -310,7 +310,7 @@ func TestRenderByNetworkHostname(t *testing.T) {
 			Metadata:    AggregateMetadata{},
 		},
 	}
-	have := report.Network.RenderBy(NetworkHostname, GenericPseudoNode, false)
+	have := report.Network.RenderBy(NetworkHostname, GenericPseudoNode)
 	if !reflect.DeepEqual(want, have) {
 		t.Error("\n" + diff(want, have))
 	}