Fixing grouped node count for filtered children nodes

Squash of:

* We have to keep all the container hostnames until the end so we can
  count how many we've filtered

* Adding tests for ContainerHostnameRenderer and PodServiceRenderer with
  filters

* Because we filter on image name we need the image name before
  filtering

* Alternative approach to passing decorators.

* Refactor out some of the decorator capture

* Don't memoise decorated calls to Render

* Fixing filtered counts on containers topology

  Tricky, because we need the filters to be silent sometimes (when they're
  in the middle), but not when they're at the top, so we take the "top"
  filter's stats. However, this means we have to compose all
  user-specified filters into a single Filter layer, so we can get all
  stats.

  There are no more Silent filters, as all filters are silent (unless they
  are at the top).

  Additionally, I clarified some of the filters as their usage/terminology
  was inconsistent and confused. Now Filter(IsFoo, ...) *keeps* only nodes
  where IsFoo is true.

app/api_topologies.go

@@ -26,9 +26,9 @@ func init() {
 			ID:      "system",
 			Default: "application",
 			Options: []APITopologyOption{
-				{"system", "System services", render.FilterApplication},
-				{"application", "Application services", render.FilterSystem},
-				{"both", "Both", render.FilterNoop},
+				{"system", "System services", render.IsSystem},
+				{"application", "Application services", render.IsApplication},
+				{"both", "Both", render.Noop},
 			},
 		},
 	}
@@ -38,9 +38,9 @@ func init() {
 			ID:      "system",
 			Default: "application",
 			Options: []APITopologyOption{
-				{"system", "System pods", render.FilterApplication},
-				{"application", "Application pods", render.FilterSystem},
-				{"both", "Both", render.FilterNoop},
+				{"system", "System pods", render.IsSystem},
+				{"application", "Application pods", render.IsApplication},
+				{"both", "Both", render.Noop},
 			},
 		},
 	}
@@ -50,18 +50,18 @@ func init() {
 			ID:      "system",
 			Default: "application",
 			Options: []APITopologyOption{
-				{"system", "System containers", render.FilterApplication},
-				{"application", "Application containers", render.FilterSystem},
-				{"both", "Both", render.FilterNoop},
+				{"system", "System containers", render.IsSystem},
+				{"application", "Application containers", render.IsApplication},
+				{"both", "Both", render.Noop},
 			},
 		},
 		{
 			ID:      "stopped",
 			Default: "running",
 			Options: []APITopologyOption{
-				{"stopped", "Stopped containers", render.FilterRunning},
-				{"running", "Running containers", render.FilterStopped},
-				{"both", "Both", render.FilterNoop},
+				{"stopped", "Stopped containers", render.IsStopped},
+				{"running", "Running containers", render.IsRunning},
+				{"both", "Both", render.Noop},
 			},
 		},
 	}
@@ -73,7 +73,7 @@ func init() {
 			Options: []APITopologyOption{
 				// Show the user why there are filtered nodes in this view.
 				// Don't give them the option to show those nodes.
-				{"hide", "Unconnected nodes hidden", render.FilterNoop},
+				{"hide", "Unconnected nodes hidden", render.Noop},
 			},
 		},
 	}
@@ -181,7 +181,7 @@ type APITopologyOption struct {
 	Value string `json:"value"`
 	Label string `json:"label"`
 
-	decorator func(render.Renderer) render.Renderer
+	filter render.FilterFunc
 }
 
 type topologyStats struct {
@@ -247,31 +247,31 @@ func (r *registry) makeTopologyList(rep Reporter) CtxHandlerFunc {
 func (r *registry) renderTopologies(rpt report.Report, req *http.Request) []APITopologyDesc {
 	topologies := []APITopologyDesc{}
 	r.walk(func(desc APITopologyDesc) {
-		renderer := renderedForRequest(req, desc)
-		desc.Stats = decorateWithStats(rpt, renderer)
+		renderer, decorator := renderedForRequest(req, desc)
+		desc.Stats = decorateWithStats(rpt, renderer, decorator)
 		for i := range desc.SubTopologies {
-			renderer := renderedForRequest(req, desc.SubTopologies[i])
-			desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer)
+			renderer, decorator := renderedForRequest(req, desc.SubTopologies[i])
+			desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator)
 		}
 		topologies = append(topologies, desc)
 	})
 	return topologies
 }
 
-func decorateWithStats(rpt report.Report, renderer render.Renderer) topologyStats {
+func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator render.Decorator) topologyStats {
 	var (
 		nodes     int
 		realNodes int
 		edges     int
 	)
-	for _, n := range renderer.Render(rpt) {
+	for _, n := range renderer.Render(rpt, decorator) {
 		nodes++
 		if n.Topology != render.Pseudo {
 			realNodes++
 		}
 		edges += len(n.Adjacency)
 	}
-	renderStats := renderer.Stats(rpt)
+	renderStats := renderer.Stats(rpt, decorator)
 	return topologyStats{
 		NodeCount:          nodes,
 		NonpseudoNodeCount: realNodes,
@@ -280,20 +280,26 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer) topologyStat
 	}
 }
 
-func renderedForRequest(r *http.Request, topology APITopologyDesc) render.Renderer {
-	renderer := topology.renderer
+func renderedForRequest(r *http.Request, topology APITopologyDesc) (render.Renderer, render.Decorator) {
+	var filters []render.FilterFunc
 	for _, group := range topology.Options {
 		value := r.FormValue(group.ID)
 		for _, opt := range group.Options {
 			if (value == "" && group.Default == opt.Value) || (opt.Value != "" && opt.Value == value) {
-				renderer = opt.decorator(renderer)
+				filters = append(filters, opt.filter)
 			}
 		}
 	}
-	return renderer
+	return topology.renderer, func(renderer render.Renderer) render.Renderer {
+		return render.MakeFilter(render.ComposeFilterFuncs(filters...), renderer)
+	}
 }
 
-type reportRenderHandler func(context.Context, Reporter, render.Renderer, http.ResponseWriter, *http.Request)
+type reportRenderHandler func(
+	context.Context,
+	Reporter, render.Renderer, render.Decorator,
+	http.ResponseWriter, *http.Request,
+)
 
 func (r *registry) captureRenderer(rep Reporter, f reportRenderHandler) CtxHandlerFunc {
 	return func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
@@ -302,18 +308,7 @@ func (r *registry) captureRenderer(rep Reporter, f reportRenderHandler) CtxHandl
 			http.NotFound(w, req)
 			return
 		}
-		renderer := renderedForRequest(req, topology)
-		f(ctx, rep, renderer, w, req)
-	}
-}
-
-func (r *registry) captureRendererWithoutFilters(rep Reporter, f reportRenderHandler) CtxHandlerFunc {
-	return func(ctx context.Context, w http.ResponseWriter, req *http.Request) {
-		topology, ok := r.get(mux.Vars(req)["topology"])
-		if !ok {
-			http.NotFound(w, req)
-			return
-		}
-		f(ctx, rep, topology.renderer, w, req)
+		renderer, decorator := renderedForRequest(req, topology)
+		f(ctx, rep, renderer, decorator, w, req)
 	}
 }

app/api_topology.go

@@ -28,19 +28,27 @@ type APINode struct {
 }
 
 // Full topology.
-func handleTopology(ctx context.Context, rep Reporter, renderer render.Renderer, w http.ResponseWriter, r *http.Request) {
+func handleTopology(
+	ctx context.Context,
+	rep Reporter, renderer render.Renderer, decorator render.Decorator,
+	w http.ResponseWriter, r *http.Request,
+) {
 	report, err := rep.Report(ctx)
 	if err != nil {
 		respondWith(w, http.StatusInternalServerError, err.Error())
 		return
 	}
 	respondWith(w, http.StatusOK, APITopology{
-		Nodes: detailed.Summaries(report, renderer.Render(report)),
+		Nodes: detailed.Summaries(report, renderer.Render(report, decorator)),
 	})
 }
 
 // Websocket for the full topology. This route overlaps with the next.
-func handleWs(ctx context.Context, rep Reporter, renderer render.Renderer, w http.ResponseWriter, r *http.Request) {
+func handleWs(
+	ctx context.Context,
+	rep Reporter, renderer render.Renderer, decorator render.Decorator,
+	w http.ResponseWriter, r *http.Request,
+) {
 	if err := r.ParseForm(); err != nil {
 		respondWith(w, http.StatusInternalServerError, err.Error())
 		return
@@ -53,17 +61,21 @@ func handleWs(ctx context.Context, rep Reporter, renderer render.Renderer, w htt
 			return
 		}
 	}
-	handleWebsocket(ctx, w, r, rep, renderer, loop)
+	handleWebsocket(ctx, w, r, rep, renderer, decorator, loop)
 }
 
 // Individual nodes.
-func handleNode(ctx context.Context, rep Reporter, renderer render.Renderer, w http.ResponseWriter, r *http.Request) {
+func handleNode(
+	ctx context.Context,
+	rep Reporter, renderer render.Renderer, _ render.Decorator,
+	w http.ResponseWriter, r *http.Request,
+) {
 	var (
 		vars        = mux.Vars(r)
 		topologyID  = vars["topology"]
 		nodeID      = vars["id"]
 		report, err = rep.Report(ctx)
-		rendered    = renderer.Render(report)
+		rendered    = renderer.Render(report, render.FilterNoop)
 		node, ok    = rendered[nodeID]
 	)
 	if err != nil {
@@ -83,6 +95,7 @@ func handleWebsocket(
 	r *http.Request,
 	rep Reporter,
 	renderer render.Renderer,
+	decorator render.Decorator,
 	loop time.Duration,
 ) {
 	conn, err := xfer.Upgrade(w, r, nil)
@@ -119,7 +132,7 @@ func handleWebsocket(
 			log.Errorf("Error generating report: %v", err)
 			return
 		}
-		newTopo := detailed.Summaries(report, renderer.Render(report))
+		newTopo := detailed.Summaries(report, renderer.Render(report, decorator))
 		diff := detailed.TopoDiff(previousTopo, newTopo)
 		previousTopo = newTopo
 

app/router.go

@@ -93,7 +93,7 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter) {
 	get.HandleFunc("/api/topology/{topology}/ws",
 		requestContextDecorator(topologyRegistry.captureRenderer(r, handleWs))) // NB not gzip!
 	get.MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc(
-		gzipHandler(requestContextDecorator(topologyRegistry.captureRendererWithoutFilters(r, handleNode))))
+		gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode))))
 	get.HandleFunc("/api/report",
 		gzipHandler(requestContextDecorator(makeRawReportHandler(r))))
 	get.HandleFunc("/api/probes",

experimental/graphviz/render.go

@@ -19,5 +19,5 @@ func renderTo(rpt report.Report, topology string) (detailed.NodeSummaries, error
 	if !ok {
 		return detailed.NodeSummaries{}, fmt.Errorf("unknown topology %v", topology)
 	}
-	return detailed.Summaries(rpt, renderer.Render(rpt)), nil
+	return detailed.Summaries(rpt, renderer.Render(rpt, render.FilterNoop)), nil
 }

render/benchmark_test.go

@@ -38,22 +38,31 @@ func BenchmarkContainerWithImageNameRender(b *testing.B) {
 func BenchmarkContainerWithImageNameStats(b *testing.B) {
 	benchmarkStats(b, render.ContainerWithImageNameRenderer)
 }
-func BenchmarkContainerImageRender(b *testing.B) { benchmarkRender(b, render.ContainerImageRenderer) }
-func BenchmarkContainerImageStats(b *testing.B)  { benchmarkStats(b, render.ContainerImageRenderer) }
+func BenchmarkContainerImageRender(b *testing.B) {
+	benchmarkRender(b, render.ContainerImageRenderer)
+}
+func BenchmarkContainerImageStats(b *testing.B) {
+	benchmarkStats(b, render.ContainerImageRenderer)
+}
 func BenchmarkContainerHostnameRender(b *testing.B) {
 	benchmarkRender(b, render.ContainerHostnameRenderer)
 }
 func BenchmarkContainerHostnameStats(b *testing.B) {
 	benchmarkStats(b, render.ContainerHostnameRenderer)
 }
-func BenchmarkHostRender(b *testing.B)       { benchmarkRender(b, render.HostRenderer) }
-func BenchmarkHostStats(b *testing.B)        { benchmarkStats(b, render.HostRenderer) }
-func BenchmarkPodRender(b *testing.B)        { benchmarkRender(b, render.PodRenderer) }
-func BenchmarkPodStats(b *testing.B)         { benchmarkStats(b, render.PodRenderer) }
-func BenchmarkPodServiceRender(b *testing.B) { benchmarkRender(b, render.PodServiceRenderer) }
-func BenchmarkPodServiceStats(b *testing.B)  { benchmarkStats(b, render.PodServiceRenderer) }
+func BenchmarkHostRender(b *testing.B) { benchmarkRender(b, render.HostRenderer) }
+func BenchmarkHostStats(b *testing.B)  { benchmarkStats(b, render.HostRenderer) }
+func BenchmarkPodRender(b *testing.B)  { benchmarkRender(b, render.PodRenderer) }
+func BenchmarkPodStats(b *testing.B)   { benchmarkStats(b, render.PodRenderer) }
+func BenchmarkPodServiceRender(b *testing.B) {
+	benchmarkRender(b, render.PodServiceRenderer)
+}
+func BenchmarkPodServiceStats(b *testing.B) {
+	benchmarkStats(b, render.PodServiceRenderer)
+}
 
 func benchmarkRender(b *testing.B, r render.Renderer) {
+
 	report, err := loadReport()
 	if err != nil {
 		b.Fatal(err)
@@ -65,7 +74,7 @@ func benchmarkRender(b *testing.B, r render.Renderer) {
 		b.StopTimer()
 		render.ResetCache()
 		b.StartTimer()
-		benchmarkRenderResult = r.Render(report)
+		benchmarkRenderResult = r.Render(report, render.FilterNoop)
 		if len(benchmarkRenderResult) == 0 {
 			b.Errorf("Rendered topology contained no nodes")
 		}
@@ -85,7 +94,7 @@ func benchmarkStats(b *testing.B, r render.Renderer) {
 		b.StopTimer()
 		render.ResetCache()
 		b.StartTimer()
-		benchmarkStatsResult = r.Stats(report)
+		benchmarkStatsResult = r.Stats(report, render.FilterNoop)
 	}
 }