From 8f7e00f46a49913bb07179a87c900f9152f28292 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 7 Nov 2017 21:40:01 +0000 Subject: [PATCH 1/3] Stats are easily produced as part of Rendering ...so there is no need for a separate Stats method. step 1: return stats from Render --- app/api_topologies.go | 6 +-- app/api_topologies_test.go | 6 +-- app/api_topology.go | 6 +-- render/benchmark_test.go | 2 +- render/container.go | 18 ++++----- render/container_test.go | 12 +++--- render/detailed/node_test.go | 8 ++-- render/detailed/parents_test.go | 10 ++--- render/detailed/summary_test.go | 4 +- render/filters.go | 36 ++++++++--------- render/filters_test.go | 12 +++--- render/host.go | 6 +-- render/host_test.go | 2 +- render/memoise.go | 8 ++-- render/memoise_test.go | 10 ++--- render/pod.go | 4 +- render/pod_test.go | 8 ++-- render/process.go | 18 ++++----- render/process_test.go | 6 +-- render/render.go | 54 ++++++++++++++++++-------- render/render_test.go | 12 +++--- render/selectors.go | 4 +- render/short_lived_connections_test.go | 4 +- 23 files changed, 138 insertions(+), 118 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 848a1c1c8..78481e355 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -504,19 +504,19 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re realNodes int edges int ) - for _, n := range renderer.Render(rpt, decorator) { + r := renderer.Render(rpt, decorator) + for _, n := range r.Nodes { nodes++ if n.Topology != render.Pseudo { realNodes++ } edges += len(n.Adjacency) } - renderStats := renderer.Stats(rpt, decorator) return topologyStats{ NodeCount: nodes, NonpseudoNodeCount: realNodes, EdgeCount: edges, - FilteredNodes: renderStats.FilteredNodes, + FilteredNodes: r.Filtered, } } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index cb2fa6efe..64fda080f 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -118,7 +118,7 @@ func TestRendererForTopologyWithFiltering(t *testing.T) { input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{ docker.LabelPrefix + "works.weave.role": "system", }) - have := utils.Prune(renderer.Render(input, decorator)) + have := utils.Prune(renderer.Render(input, decorator).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, fixture.ClientContainerNodeID) delete(want, render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID)) @@ -149,7 +149,7 @@ func TestRendererForTopologyNoFiltering(t *testing.T) { input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{ docker.LabelPrefix + "works.weave.role": "system", }) - have := utils.Prune(renderer.Render(input, decorator)) + have := utils.Prune(renderer.Render(input, decorator).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID)) delete(want, render.OutgoingInternetID) @@ -183,7 +183,7 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det return nil, err } - return detailed.Summaries(report.RenderContext{Report: fixture.Report}, renderer.Render(fixture.Report, decorator)), nil + return detailed.Summaries(report.RenderContext{Report: fixture.Report}, renderer.Render(fixture.Report, decorator).Nodes), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { diff --git a/app/api_topology.go b/app/api_topology.go index a30f554bd..6c95e7642 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -31,7 +31,7 @@ type APINode struct { // Full topology. func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(rc, renderer.Render(rc.Report, decorator)), + Nodes: detailed.Summaries(rc, renderer.Render(rc.Report, decorator).Nodes), }) } @@ -42,7 +42,7 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. topologyID = vars["topology"] nodeID = vars["id"] preciousRenderer = render.PreciousNodeRenderer{PreciousNodeID: nodeID, Renderer: renderer} - rendered = preciousRenderer.Render(rc.Report, decorator) + rendered = preciousRenderer.Render(rc.Report, decorator).Nodes node, ok = rendered[nodeID] ) if !ok { @@ -123,7 +123,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(RenderContextForReporter(rep, re), renderer.Render(re, decorator)) + newTopo := detailed.Summaries(RenderContextForReporter(rep, re), renderer.Render(re, decorator).Nodes) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/render/benchmark_test.go b/render/benchmark_test.go index 40e4c480b..24c34fb5b 100644 --- a/render/benchmark_test.go +++ b/render/benchmark_test.go @@ -74,7 +74,7 @@ func benchmarkRender(b *testing.B, r render.Renderer) { b.StopTimer() render.ResetCache() b.StartTimer() - benchmarkRenderResult = r.Render(report, FilterNoop) + benchmarkRenderResult = r.Render(report, FilterNoop).Nodes if len(benchmarkRenderResult) == 0 { b.Errorf("Rendered topology contained no nodes") } diff --git a/render/container.go b/render/container.go index 5ee885d2d..4d32c857e 100644 --- a/render/container.go +++ b/render/container.go @@ -53,14 +53,14 @@ type connectionJoin struct { r Renderer } -func (c connectionJoin) Render(rpt report.Report, dct Decorator) report.Nodes { +func (c connectionJoin) Render(rpt report.Report, dct Decorator) Nodes { local := LocalNetworks(rpt) inputNodes := c.r.Render(rpt, dct) endpoints := SelectEndpoint.Render(rpt, dct) // Collect all the IPs we are trying to map to, and which ID they map from var ipNodes = map[string]string{} - for _, n := range inputNodes { + for _, n := range inputNodes.Nodes { for _, ip := range c.toIPs(n) { if _, exists := ipNodes[ip]; exists { // If an IP is shared between multiple nodes, we can't reliably @@ -74,7 +74,7 @@ func (c connectionJoin) Render(rpt report.Report, dct Decorator) report.Nodes { ret := newJoinResults() // Now look at all the endpoints and see which map to IP nodes - for _, m := range endpoints { + for _, m := range endpoints.Nodes { scope, addr, port, ok := report.ParseEndpointNodeID(m.ID) if !ok { continue @@ -95,14 +95,14 @@ func (c connectionJoin) Render(rpt report.Report, dct Decorator) report.Nodes { } if found && id != "" { // not one we blanked out earlier ret.addToResults(m, id, func(id string) report.Node { - return inputNodes[id] + return inputNodes.Nodes[id] }) } } ret.copyUnmatched(inputNodes) ret.fixupAdjacencies(inputNodes) ret.fixupAdjacencies(endpoints) - return ret.nodes + return ret.result() } func (c connectionJoin) Stats(rpt report.Report, _ Decorator) Stats { @@ -135,18 +135,18 @@ type containerWithImageNameRenderer struct { // Render produces a container graph where the the latest metadata contains the // container image name, if found. -func (r containerWithImageNameRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { +func (r containerWithImageNameRenderer) Render(rpt report.Report, dct Decorator) Nodes { containers := r.Renderer.Render(rpt, dct) images := SelectContainerImage.Render(rpt, dct) outputs := report.Nodes{} - for id, c := range containers { + for id, c := range containers.Nodes { outputs[id] = c imageID, ok := c.Latest.Lookup(docker.ImageID) if !ok { continue } - image, ok := images[report.MakeContainerImageNodeID(imageID)] + image, ok := images.Nodes[report.MakeContainerImageNodeID(imageID)] if !ok { continue } @@ -166,7 +166,7 @@ func (r containerWithImageNameRenderer) Render(rpt report.Report, dct Decorator) Add(report.ContainerImage, report.MakeStringSet(imageNodeID)) outputs[id] = c } - return outputs + return Nodes{Nodes: outputs, Filtered: containers.Filtered} } // ContainerWithImageNameRenderer is a Renderer which produces a container diff --git a/render/container_test.go b/render/container_test.go index 0eb5d66b9..0d765b21d 100644 --- a/render/container_test.go +++ b/render/container_test.go @@ -59,7 +59,7 @@ func testMap(t *testing.T, f render.MapFunc, input testcase) { } func TestContainerRenderer(t *testing.T) { - have := utils.Prune(render.ContainerWithImageNameRenderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(render.ContainerWithImageNameRenderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedContainers) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -76,7 +76,7 @@ func TestContainerFilterRenderer(t *testing.T) { docker.LabelPrefix + "works.weave.role": "system", }) - have := utils.Prune(renderer.Render(input, FilterApplication)) + have := utils.Prune(renderer.Render(input, FilterApplication).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, fixture.ClientContainerNodeID) if !reflect.DeepEqual(want, have) { @@ -86,7 +86,7 @@ func TestContainerFilterRenderer(t *testing.T) { func TestContainerHostnameRenderer(t *testing.T) { renderer := render.ApplyDecorator(render.ContainerHostnameRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(renderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedContainerHostnames) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -95,7 +95,7 @@ func TestContainerHostnameRenderer(t *testing.T) { func TestContainerHostnameFilterRenderer(t *testing.T) { renderer := render.ApplyDecorator(render.ContainerHostnameRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterSystem)) + have := utils.Prune(renderer.Render(fixture.Report, FilterSystem).Nodes) want := utils.Prune(expected.RenderedContainerHostnames.Copy()) delete(want, fixture.ClientContainerHostname) delete(want, fixture.ServerContainerHostname) @@ -107,7 +107,7 @@ func TestContainerHostnameFilterRenderer(t *testing.T) { func TestContainerImageRenderer(t *testing.T) { renderer := render.ApplyDecorator(render.ContainerImageRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(renderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedContainerImages) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -116,7 +116,7 @@ func TestContainerImageRenderer(t *testing.T) { func TestContainerImageFilterRenderer(t *testing.T) { renderer := render.ApplyDecorator(render.ContainerImageRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterSystem)) + have := utils.Prune(renderer.Render(fixture.Report, FilterSystem).Nodes) want := utils.Prune(expected.RenderedContainerHostnames.Copy()) delete(want, fixture.ClientContainerHostname) delete(want, fixture.ServerContainerHostname) diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 785bfb43f..5d04cf2b9 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -18,7 +18,7 @@ import ( ) func child(t *testing.T, r render.Renderer, id string) detailed.NodeSummary { - s, ok := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, r.Render(fixture.Report, nil)[id]) + s, ok := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, r.Render(fixture.Report, nil).Nodes[id]) if !ok { t.Fatalf("Expected node %s to be summarizable, but wasn't", id) } @@ -30,7 +30,7 @@ func connectionID(nodeID string, addr string) string { } func TestMakeDetailedHostNode(t *testing.T) { - renderableNodes := render.HostRenderer.Render(fixture.Report, nil) + renderableNodes := render.HostRenderer.Render(fixture.Report, nil).Nodes renderableNode := renderableNodes[fixture.ClientHostNodeID] have := detailed.MakeNode("hosts", report.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) @@ -178,7 +178,7 @@ func TestMakeDetailedHostNode(t *testing.T) { func TestMakeDetailedContainerNode(t *testing.T) { id := fixture.ServerContainerNodeID - renderableNodes := render.ContainerWithImageNameRenderer.Render(fixture.Report, nil) + renderableNodes := render.ContainerWithImageNameRenderer.Render(fixture.Report, nil).Nodes renderableNode, ok := renderableNodes[id] if !ok { t.Fatalf("Node not found: %s", id) @@ -308,7 +308,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { func TestMakeDetailedPodNode(t *testing.T) { id := fixture.ServerPodNodeID - renderableNodes := render.PodRenderer.Render(fixture.Report, nil) + renderableNodes := render.PodRenderer.Render(fixture.Report, nil).Nodes renderableNode, ok := renderableNodes[id] if !ok { t.Fatalf("Node not found: %s", id) diff --git a/render/detailed/parents_test.go b/render/detailed/parents_test.go index b4d18bb23..28251d9c1 100644 --- a/render/detailed/parents_test.go +++ b/render/detailed/parents_test.go @@ -21,25 +21,25 @@ func TestParents(t *testing.T) { }{ { name: "Node accidentally tagged with itself", - node: render.HostRenderer.Render(fixture.Report, nil)[fixture.ClientHostNodeID].WithParents( + node: render.HostRenderer.Render(fixture.Report, nil).Nodes[fixture.ClientHostNodeID].WithParents( report.MakeSets().Add(report.Host, report.MakeStringSet(fixture.ClientHostNodeID)), ), want: nil, }, { - node: render.HostRenderer.Render(fixture.Report, nil)[fixture.ClientHostNodeID], + node: render.HostRenderer.Render(fixture.Report, nil).Nodes[fixture.ClientHostNodeID], want: nil, }, { name: "Container image", - node: render.ContainerImageRenderer.Render(fixture.Report, nil)[expected.ClientContainerImageNodeID], + node: render.ContainerImageRenderer.Render(fixture.Report, nil).Nodes[expected.ClientContainerImageNodeID], want: []detailed.Parent{ {ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"}, }, }, { name: "Container", - node: render.ContainerWithImageNameRenderer.Render(fixture.Report, nil)[fixture.ClientContainerNodeID], + node: render.ContainerWithImageNameRenderer.Render(fixture.Report, nil).Nodes[fixture.ClientContainerNodeID], want: []detailed.Parent{ {ID: expected.ClientContainerImageNodeID, Label: fixture.ClientContainerImageName, TopologyID: "containers-by-image"}, {ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"}, @@ -47,7 +47,7 @@ func TestParents(t *testing.T) { }, }, { - node: render.ProcessRenderer.Render(fixture.Report, nil)[fixture.ClientProcess1NodeID], + node: render.ProcessRenderer.Render(fixture.Report, nil).Nodes[fixture.ClientProcess1NodeID], want: []detailed.Parent{ {ID: fixture.ClientContainerNodeID, Label: fixture.ClientContainerName, TopologyID: "containers"}, {ID: fixture.ClientHostNodeID, Label: fixture.ClientHostName, TopologyID: "hosts"}, diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index 5d7abd62c..e04827f1a 100644 --- a/render/detailed/summary_test.go +++ b/render/detailed/summary_test.go @@ -21,7 +21,7 @@ import ( func TestSummaries(t *testing.T) { { // Just a convenient source of some rendered nodes - have := detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.ProcessRenderer.Render(fixture.Report, nil)) + have := detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.ProcessRenderer.Render(fixture.Report, nil).Nodes) // The ids of the processes rendered above expectedIDs := []string{ fixture.ClientProcess1NodeID, @@ -51,7 +51,7 @@ func TestSummaries(t *testing.T) { input := fixture.Report.Copy() input.Process.Nodes[fixture.ClientProcess1NodeID].Metrics[process.CPUUsage] = metric - have := detailed.Summaries(report.RenderContext{Report: input}, render.ProcessRenderer.Render(input, nil)) + have := detailed.Summaries(report.RenderContext{Report: input}, render.ProcessRenderer.Render(input, nil).Nodes) node, ok := have[fixture.ClientProcess1NodeID] if !ok { diff --git a/render/filters.go b/render/filters.go index 6d9f46272..f2a434a98 100644 --- a/render/filters.go +++ b/render/filters.go @@ -21,12 +21,13 @@ type PreciousNodeRenderer struct { } // Render implements Renderer -func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { +func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) Nodes { undecoratedNodes := p.Renderer.Render(rpt, nil) - preciousNode, foundBeforeDecoration := undecoratedNodes[p.PreciousNodeID] - finalNodes := applyDecorator{ConstantRenderer(undecoratedNodes)}.Render(rpt, dct) - if _, ok := finalNodes[p.PreciousNodeID]; !ok && foundBeforeDecoration { - finalNodes[p.PreciousNodeID] = preciousNode + preciousNode, foundBeforeDecoration := undecoratedNodes.Nodes[p.PreciousNodeID] + finalNodes := applyDecorator{ConstantRenderer{undecoratedNodes}}.Render(rpt, dct) + if _, ok := finalNodes.Nodes[p.PreciousNodeID]; !ok && foundBeforeDecoration { + finalNodes.Nodes[p.PreciousNodeID] = preciousNode + finalNodes.Filtered-- } return finalNodes } @@ -42,12 +43,12 @@ func (p PreciousNodeRenderer) Stats(rpt report.Report, dct Decorator) Stats { // in one call - useful for functions that need to consider the entire graph. // We should minimise the use of this renderer type, as it is very inflexible. type CustomRenderer struct { - RenderFunc func(report.Nodes) report.Nodes + RenderFunc func(Nodes) Nodes Renderer } // Render implements Renderer -func (c CustomRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { +func (c CustomRenderer) Render(rpt report.Report, dct Decorator) Nodes { return c.RenderFunc(c.Renderer.Render(rpt, dct)) } @@ -57,11 +58,11 @@ func (c CustomRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { func ColorConnected(r Renderer) Renderer { return CustomRenderer{ Renderer: r, - RenderFunc: func(input report.Nodes) report.Nodes { + RenderFunc: func(input Nodes) Nodes { connected := map[string]struct{}{} void := struct{}{} - for id, node := range input { + for id, node := range input.Nodes { for _, adj := range node.Adjacency { if adj != id { connected[id] = void @@ -74,7 +75,7 @@ func ColorConnected(r Renderer) Renderer { for id := range connected { output[id] = output[id].WithLatest(IsConnected, mtime.Now(), "true") } - return output + return Nodes{Nodes: output, Filtered: input.Filtered} }, } } @@ -147,16 +148,15 @@ func MakeFilterPseudoDecorator(f FilterFunc) Decorator { } // Render implements Renderer -func (f *Filter) Render(rpt report.Report, dct Decorator) report.Nodes { - nodes, _ := f.render(rpt, dct) - return nodes +func (f *Filter) Render(rpt report.Report, dct Decorator) Nodes { + return f.render(rpt, dct) } -func (f *Filter) render(rpt report.Report, dct Decorator) (report.Nodes, int) { +func (f *Filter) render(rpt report.Report, dct Decorator) Nodes { output := report.Nodes{} inDegrees := map[string]int{} filtered := 0 - for id, node := range f.Renderer.Render(rpt, dct) { + for id, node := range f.Renderer.Render(rpt, dct).Nodes { if f.FilterFunc(node) { output[id] = node inDegrees[id] = 0 @@ -190,7 +190,7 @@ func (f *Filter) render(rpt report.Report, dct Decorator) (report.Nodes, int) { delete(output, id) filtered++ } - return output, filtered + return Nodes{Nodes: output, Filtered: filtered} } // Stats implements Renderer. General logic is to take the first (i.e. @@ -198,8 +198,8 @@ func (f *Filter) render(rpt report.Report, dct Decorator) (report.Nodes, int) { // if we want to count the stats from multiple filters we need to compose their // FilterFuncs, into a single Filter. func (f Filter) Stats(rpt report.Report, dct Decorator) Stats { - _, filtered := f.render(rpt, dct) - return Stats{FilteredNodes: filtered} + nodes := f.render(rpt, dct) + return Stats{FilteredNodes: nodes.Filtered} } // IsConnected is the key added to Node.Metadata by ColorConnected diff --git a/render/filters_test.go b/render/filters_test.go index 3b83d6862..7e2f7434e 100644 --- a/render/filters_test.go +++ b/render/filters_test.go @@ -16,7 +16,7 @@ func TestFilterRender(t *testing.T) { "baz": report.MakeNode("baz"), }} have := report.MakeIDList() - for id := range renderer.Render(report.MakeReport(), render.FilterUnconnected) { + for id := range renderer.Render(report.MakeReport(), render.FilterUnconnected).Nodes { have = have.Add(id) } want := report.MakeIDList("foo", "bar") @@ -41,7 +41,7 @@ func TestFilterRender2(t *testing.T) { "baz": report.MakeNode("baz"), }} - have := renderer.Render(report.MakeReport(), filter) + have := renderer.Render(report.MakeReport(), filter).Nodes if have["foo"].Adjacency.Contains("bar") { t.Error("adjacencies for removed nodes should have been removed") } @@ -66,7 +66,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { } } want := nodes - have := renderer.Render(report.MakeReport(), filter) + have := renderer.Render(report.MakeReport(), filter).Nodes if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } @@ -85,7 +85,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("baz"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo), }} - have := renderer.Render(report.MakeReport(), filter) + have := renderer.Render(report.MakeReport(), filter).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -104,7 +104,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("foo"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo).WithAdjacent("bar"), }} - have := renderer.Render(report.MakeReport(), filter) + have := renderer.Render(report.MakeReport(), filter).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -118,7 +118,7 @@ func TestFilterUnconnectedSelf(t *testing.T) { "foo": report.MakeNode("foo").WithAdjacent("foo"), } renderer := mockRenderer{Nodes: nodes} - have := renderer.Render(report.MakeReport(), render.FilterUnconnected) + have := renderer.Render(report.MakeReport(), render.FilterUnconnected).Nodes if len(have) > 0 { t.Error("expected node only connected to self to be removed") } diff --git a/render/host.go b/render/host.go index bcb484b85..5af0b5dbc 100644 --- a/render/host.go +++ b/render/host.go @@ -65,12 +65,12 @@ func MapX2Host(n report.Node, _ report.Networks) report.Nodes { type endpoints2Hosts struct { } -func (e endpoints2Hosts) Render(rpt report.Report, dct Decorator) report.Nodes { +func (e endpoints2Hosts) Render(rpt report.Report, dct Decorator) Nodes { local := LocalNetworks(rpt) endpoints := SelectEndpoint.Render(rpt, dct) ret := newJoinResults() - for _, n := range endpoints { + for _, n := range endpoints.Nodes { // Nodes without a hostid are treated as pseudo nodes hostNodeID, timestamp, ok := n.Latest.LookupEntry(report.HostNodeID) if !ok { @@ -88,7 +88,7 @@ func (e endpoints2Hosts) Render(rpt report.Report, dct Decorator) report.Nodes { } } ret.fixupAdjacencies(endpoints) - return ret.nodes + return ret.result() } func (e endpoints2Hosts) Stats(rpt report.Report, _ Decorator) Stats { diff --git a/render/host_test.go b/render/host_test.go index 93660bc03..8596793b3 100644 --- a/render/host_test.go +++ b/render/host_test.go @@ -12,7 +12,7 @@ import ( ) func TestHostRenderer(t *testing.T) { - have := utils.Prune(render.HostRenderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(render.HostRenderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedHosts) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) diff --git a/render/memoise.go b/render/memoise.go index b39a2d3c1..55cfdab3e 100644 --- a/render/memoise.go +++ b/render/memoise.go @@ -39,7 +39,7 @@ func Memoise(r Renderer) Renderer { // m.Renderer. // // The cache is bypassed when rendering a report with a decorator. -func (m *memoise) Render(rpt report.Report, dct Decorator) report.Nodes { +func (m *memoise) Render(rpt report.Report, dct Decorator) Nodes { if dct != nil { return m.Renderer.Render(rpt, dct) } @@ -64,7 +64,7 @@ func (m *memoise) Render(rpt report.Report, dct Decorator) report.Nodes { } type promise struct { - val report.Nodes + val Nodes done chan struct{} } @@ -72,12 +72,12 @@ func newPromise() *promise { return &promise{done: make(chan struct{})} } -func (p *promise) Set(val report.Nodes) { +func (p *promise) Set(val Nodes) { p.val = val close(p.done) } -func (p *promise) Get() report.Nodes { +func (p *promise) Get() Nodes { <-p.done return p.val } diff --git a/render/memoise_test.go b/render/memoise_test.go index 677ca252a..8159a906f 100644 --- a/render/memoise_test.go +++ b/render/memoise_test.go @@ -9,23 +9,23 @@ import ( "github.com/weaveworks/scope/test/reflect" ) -type renderFunc func(r report.Report) report.Nodes +type renderFunc func(r report.Report) render.Nodes -func (f renderFunc) Render(r report.Report, _ render.Decorator) report.Nodes { return f(r) } +func (f renderFunc) Render(r report.Report, _ render.Decorator) render.Nodes { return f(r) } func (f renderFunc) Stats(r report.Report, _ render.Decorator) render.Stats { return render.Stats{} } func TestMemoise(t *testing.T) { calls := 0 - r := renderFunc(func(rpt report.Report) report.Nodes { + r := renderFunc(func(rpt report.Report) render.Nodes { calls++ - return report.Nodes{rpt.ID: report.MakeNode(rpt.ID)} + return render.Nodes{Nodes: report.Nodes{rpt.ID: report.MakeNode(rpt.ID)}} }) m := render.Memoise(r) rpt1 := report.MakeReport() result1 := m.Render(rpt1, nil) // it should have rendered it. - if _, ok := result1[rpt1.ID]; !ok { + if _, ok := result1.Nodes[rpt1.ID]; !ok { t.Errorf("Expected rendered report to contain a node, but got: %v", result1) } if calls != 1 { diff --git a/render/pod.go b/render/pod.go index 28e3a9f67..f76cd3ea4 100644 --- a/render/pod.go +++ b/render/pod.go @@ -117,7 +117,7 @@ func renderParents(childTopology string, parentTopologies []string, noParentsPse // to deployments where applicable. type selectPodsWithDeployments struct{} -func (s selectPodsWithDeployments) Render(rpt report.Report, dct Decorator) report.Nodes { +func (s selectPodsWithDeployments) Render(rpt report.Report, dct Decorator) Nodes { result := report.Nodes{} // For each pod, we check for any replica sets, and merge any deployments they point to // into a replacement Parents value. @@ -135,7 +135,7 @@ func (s selectPodsWithDeployments) Render(rpt report.Report, dct Decorator) repo } result[podID] = pod } - return result + return Nodes{Nodes: result} } func (s selectPodsWithDeployments) Stats(rpt report.Report, _ Decorator) Stats { diff --git a/render/pod_test.go b/render/pod_test.go index 1fa56f934..9be35ffb1 100644 --- a/render/pod_test.go +++ b/render/pod_test.go @@ -13,7 +13,7 @@ import ( ) func TestPodRenderer(t *testing.T) { - have := utils.Prune(render.PodRenderer.Render(fixture.Report, nil)) + have := utils.Prune(render.PodRenderer.Render(fixture.Report, nil).Nodes) want := utils.Prune(expected.RenderedPods) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -34,7 +34,7 @@ func TestPodFilterRenderer(t *testing.T) { kubernetes.Namespace: "kube-system", }) - have := utils.Prune(renderer.Render(input, filterNonKubeSystem)) + have := utils.Prune(renderer.Render(input, filterNonKubeSystem).Nodes) want := utils.Prune(expected.RenderedPods.Copy()) delete(want, fixture.ClientPodNodeID) if !reflect.DeepEqual(want, have) { @@ -43,7 +43,7 @@ func TestPodFilterRenderer(t *testing.T) { } func TestPodServiceRenderer(t *testing.T) { - have := utils.Prune(render.PodServiceRenderer.Render(fixture.Report, nil)) + have := utils.Prune(render.PodServiceRenderer.Render(fixture.Report, nil).Nodes) want := utils.Prune(expected.RenderedPodServices) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -60,7 +60,7 @@ func TestPodServiceFilterRenderer(t *testing.T) { kubernetes.Namespace: "kube-system", }) - have := utils.Prune(renderer.Render(input, filterNonKubeSystem)) + have := utils.Prune(renderer.Render(input, filterNonKubeSystem).Nodes) want := utils.Prune(expected.RenderedPodServices.Copy()) delete(want, fixture.ServiceNodeID) delete(want, render.IncomingInternetID) diff --git a/render/process.go b/render/process.go index 54d4a14b6..e23c352d6 100644 --- a/render/process.go +++ b/render/process.go @@ -40,18 +40,18 @@ type processWithContainerNameRenderer struct { Renderer } -func (r processWithContainerNameRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { +func (r processWithContainerNameRenderer) Render(rpt report.Report, dct Decorator) Nodes { processes := r.Renderer.Render(rpt, dct) containers := SelectContainer.Render(rpt, dct) outputs := report.Nodes{} - for id, p := range processes { + for id, p := range processes.Nodes { outputs[id] = p containerID, timestamp, ok := p.Latest.LookupEntry(docker.ContainerID) if !ok { continue } - container, ok := containers[report.MakeContainerNodeID(containerID)] + container, ok := containers.Nodes[report.MakeContainerNodeID(containerID)] if !ok { continue } @@ -61,7 +61,7 @@ func (r processWithContainerNameRenderer) Render(rpt report.Report, dct Decorato } outputs[id] = p } - return outputs + return Nodes{Nodes: outputs, Filtered: processes.Filtered} } // ProcessWithContainerNameRenderer is a Renderer which produces a process @@ -86,16 +86,16 @@ var ProcessNameRenderer = ConditionalRenderer(renderProcesses, type endpoints2Processes struct { } -func (e endpoints2Processes) Render(rpt report.Report, dct Decorator) report.Nodes { +func (e endpoints2Processes) Render(rpt report.Report, dct Decorator) Nodes { if len(rpt.Process.Nodes) == 0 { - return report.Nodes{} + return Nodes{} } local := LocalNetworks(rpt) processes := SelectProcess.Render(rpt, dct) endpoints := SelectEndpoint.Render(rpt, dct) ret := newJoinResults() - for _, n := range endpoints { + for _, n := range endpoints.Nodes { // Nodes without a hostid are treated as pseudo nodes if hostNodeID, ok := n.Latest.Lookup(report.HostNodeID); !ok { if id, ok := pseudoNodeID(n, local); ok { @@ -117,7 +117,7 @@ func (e endpoints2Processes) Render(rpt report.Report, dct Decorator) report.Nod hostID, _, _ := report.ParseNodeID(hostNodeID) id := report.MakeProcessNodeID(hostID, pid) ret.addToResults(n, id, func(id string) report.Node { - if processNode, found := processes[id]; found { + if processNode, found := processes.Nodes[id]; found { return processNode } // we have a pid, but no matching process node; create a new one rather than dropping the data @@ -129,7 +129,7 @@ func (e endpoints2Processes) Render(rpt report.Report, dct Decorator) report.Nod ret.copyUnmatched(processes) ret.fixupAdjacencies(processes) ret.fixupAdjacencies(endpoints) - return ret.nodes + return ret.result() } func (e endpoints2Processes) Stats(rpt report.Report, _ Decorator) Stats { diff --git a/render/process_test.go b/render/process_test.go index a9f620958..0cbbf0c4e 100644 --- a/render/process_test.go +++ b/render/process_test.go @@ -12,7 +12,7 @@ import ( ) func TestEndpointRenderer(t *testing.T) { - have := utils.Prune(render.EndpointRenderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(render.EndpointRenderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedEndpoints) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -20,7 +20,7 @@ func TestEndpointRenderer(t *testing.T) { } func TestProcessRenderer(t *testing.T) { - have := utils.Prune(render.ProcessRenderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(render.ProcessRenderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedProcesses) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -28,7 +28,7 @@ func TestProcessRenderer(t *testing.T) { } func TestProcessNameRenderer(t *testing.T) { - have := utils.Prune(render.ProcessNameRenderer.Render(fixture.Report, FilterNoop)) + have := utils.Prune(render.ProcessNameRenderer.Render(fixture.Report, FilterNoop).Nodes) want := utils.Prune(expected.RenderedProcessNames) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) diff --git a/render/render.go b/render/render.go index 32205eb8a..f95961e4c 100644 --- a/render/render.go +++ b/render/render.go @@ -12,7 +12,7 @@ type MapFunc func(report.Node, report.Networks) report.Nodes // Renderer is something that can render a report to a set of Nodes. type Renderer interface { - Render(report.Report, Decorator) report.Nodes + Render(report.Report, Decorator) Nodes Stats(report.Report, Decorator) Stats } @@ -21,6 +21,20 @@ type Stats struct { FilteredNodes int } +// Nodes is the result of Rendering +type Nodes struct { + report.Nodes + Filtered int +} + +// Merge merges the results of Rendering +func (r Nodes) Merge(o Nodes) Nodes { + return Nodes{ + Nodes: r.Nodes.Merge(o.Nodes), + Filtered: r.Filtered + o.Filtered, + } +} + func (s Stats) merge(other Stats) Stats { return Stats{ FilteredNodes: s.FilteredNodes + other.FilteredNodes, @@ -37,13 +51,13 @@ func MakeReduce(renderers ...Renderer) Renderer { } // Render produces a set of Nodes given a Report. -func (r Reduce) Render(rpt report.Report, dct Decorator) report.Nodes { +func (r Reduce) Render(rpt report.Report, dct Decorator) Nodes { l := len(r) switch l { case 0: - return report.Nodes{} + return Nodes{} } - c := make(chan report.Nodes, l) + c := make(chan Nodes, l) for _, renderer := range r { renderer := renderer // Pike!! go func() { @@ -82,7 +96,7 @@ func MakeMap(f MapFunc, r Renderer) Renderer { // Render transforms a set of Nodes produces by another Renderer. // using a map function -func (m *Map) Render(rpt report.Report, dct Decorator) report.Nodes { +func (m *Map) Render(rpt report.Report, dct Decorator) Nodes { var ( input = m.Renderer.Render(rpt, dct) output = report.Nodes{} @@ -92,7 +106,7 @@ func (m *Map) Render(rpt report.Report, dct Decorator) report.Nodes { ) // Rewrite all the nodes according to the map function - for _, inRenderable := range input { + for _, inRenderable := range input.Nodes { for _, outRenderable := range m.MapFunc(inRenderable, localNetworks) { if existing, ok := output[outRenderable.ID]; ok { outRenderable = outRenderable.Merge(existing) @@ -115,7 +129,7 @@ func (m *Map) Render(rpt report.Report, dct Decorator) report.Nodes { output[outNodeID] = outNode } - return output + return Nodes{Nodes: output} } // Stats implements Renderer @@ -143,7 +157,7 @@ type applyDecorator struct { Renderer } -func (ad applyDecorator) Render(rpt report.Report, dct Decorator) report.Nodes { +func (ad applyDecorator) Render(rpt report.Report, dct Decorator) Nodes { if dct != nil { return dct(ad.Renderer).Render(rpt, nil) } @@ -182,11 +196,11 @@ func ConditionalRenderer(c Condition, r Renderer) Renderer { return conditionalRenderer{c, r} } -func (cr conditionalRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { +func (cr conditionalRenderer) Render(rpt report.Report, dct Decorator) Nodes { if cr.Condition(rpt) { return cr.Renderer.Render(rpt, dct) } - return report.Nodes{} + return Nodes{} } func (cr conditionalRenderer) Stats(rpt report.Report, dct Decorator) Stats { if cr.Condition(rpt) { @@ -196,11 +210,13 @@ func (cr conditionalRenderer) Stats(rpt report.Report, dct Decorator) Stats { } // ConstantRenderer renders a fixed set of nodes -type ConstantRenderer report.Nodes +type ConstantRenderer struct { + Nodes +} // Render implements Renderer -func (c ConstantRenderer) Render(_ report.Report, _ Decorator) report.Nodes { - return report.Nodes(c) +func (c ConstantRenderer) Render(_ report.Report, _ Decorator) Nodes { + return c.Nodes } // Stats implements Renderer @@ -234,8 +250,8 @@ func (ret *joinResults) addToResults(m report.Node, id string, create func(strin } // Rewrite Adjacency for new nodes in ret for original nodes in input -func (ret *joinResults) fixupAdjacencies(input report.Nodes) { - for _, n := range input { +func (ret *joinResults) fixupAdjacencies(input Nodes) { + for _, n := range input.Nodes { outID, ok := ret.mapped[n.ID] if !ok { continue @@ -252,10 +268,14 @@ func (ret *joinResults) fixupAdjacencies(input report.Nodes) { } } -func (ret *joinResults) copyUnmatched(input report.Nodes) { - for _, n := range input { +func (ret *joinResults) copyUnmatched(input Nodes) { + for _, n := range input.Nodes { if _, found := ret.nodes[n.ID]; !found { ret.nodes[n.ID] = n } } } + +func (ret *joinResults) result() Nodes { + return Nodes{Nodes: ret.nodes} +} diff --git a/render/render_test.go b/render/render_test.go index e6a0dc48b..e0fb7a318 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -13,11 +13,11 @@ type mockRenderer struct { report.Nodes } -func (m mockRenderer) Render(rpt report.Report, d render.Decorator) report.Nodes { +func (m mockRenderer) Render(rpt report.Report, d render.Decorator) render.Nodes { if d != nil { return d(mockRenderer{m.Nodes}).Render(rpt, nil) } - return m.Nodes + return render.Nodes{Nodes: m.Nodes} } func (m mockRenderer) Stats(rpt report.Report, _ render.Decorator) render.Stats { return render.Stats{} } @@ -31,7 +31,7 @@ func TestReduceRender(t *testing.T) { "foo": report.MakeNode("foo"), "bar": report.MakeNode("bar"), } - have := renderer.Render(report.MakeReport(), FilterNoop) + have := renderer.Render(report.MakeReport(), FilterNoop).Nodes if !reflect.DeepEqual(want, have) { t.Errorf("want %+v, have %+v", want, have) } @@ -48,7 +48,7 @@ func TestMapRender1(t *testing.T) { }}, } want := report.Nodes{} - have := mapper.Render(report.MakeReport(), FilterNoop) + have := mapper.Render(report.MakeReport(), FilterNoop).Nodes if !reflect.DeepEqual(want, have) { t.Errorf("want %+v, have %+v", want, have) } @@ -70,7 +70,7 @@ func TestMapRender2(t *testing.T) { want := report.Nodes{ "bar": report.MakeNode("bar"), } - have := mapper.Render(report.MakeReport(), FilterNoop) + have := mapper.Render(report.MakeReport(), FilterNoop).Nodes if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } @@ -92,7 +92,7 @@ func TestMapRender3(t *testing.T) { "_foo": report.MakeNode("_foo").WithAdjacent("_baz"), "_baz": report.MakeNode("_baz").WithAdjacent("_foo"), } - have := mapper.Render(report.MakeReport(), FilterNoop) + have := mapper.Render(report.MakeReport(), FilterNoop).Nodes if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } diff --git a/render/selectors.go b/render/selectors.go index cf990fe6a..7d6f46262 100644 --- a/render/selectors.go +++ b/render/selectors.go @@ -9,9 +9,9 @@ import ( type TopologySelector string // Render implements Renderer -func (t TopologySelector) Render(r report.Report, _ Decorator) report.Nodes { +func (t TopologySelector) Render(r report.Report, _ Decorator) Nodes { topology, _ := r.Topology(string(t)) - return topology.Nodes + return Nodes{Nodes: topology.Nodes} } // Stats implements Renderer diff --git a/render/short_lived_connections_test.go b/render/short_lived_connections_test.go index 536eb4074..ebf2fd37a 100644 --- a/render/short_lived_connections_test.go +++ b/render/short_lived_connections_test.go @@ -109,7 +109,7 @@ var ( ) func TestShortLivedInternetNodeConnections(t *testing.T) { - have := utils.Prune(render.ContainerWithImageNameRenderer.Render(rpt, FilterNoop)) + have := utils.Prune(render.ContainerWithImageNameRenderer.Render(rpt, FilterNoop).Nodes) // Conntracked-only connections from the internet should be assigned to the internet pseudonode internet, ok := have[render.IncomingInternetID] @@ -123,7 +123,7 @@ func TestShortLivedInternetNodeConnections(t *testing.T) { } func TestPauseContainerDiscarded(t *testing.T) { - have := utils.Prune(render.ContainerWithImageNameRenderer.Render(rpt, FilterNoop)) + have := utils.Prune(render.ContainerWithImageNameRenderer.Render(rpt, FilterNoop).Nodes) // There should only be a connection from container1 and the destination should be container2 container1, ok := have[container1NodeID] if !ok { From 9bd8bd825b3d5ee794cca8d4efe2b04499594e28 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 7 Nov 2017 22:10:22 +0000 Subject: [PATCH 2/3] remove now superfluous Renderer.Stats method step 2 (and final step) in producing stats as part of Rendering --- render/benchmark_test.go | 45 +++------------------------------------ render/container.go | 4 ---- render/filters.go | 16 -------------- render/host.go | 4 ---- render/memoise_test.go | 1 - render/pod.go | 4 ---- render/process.go | 4 ---- render/render.go | 46 ---------------------------------------- render/render_test.go | 1 - render/selectors.go | 5 ----- 10 files changed, 3 insertions(+), 127 deletions(-) diff --git a/render/benchmark_test.go b/render/benchmark_test.go index 24c34fb5b..9be78cdd4 100644 --- a/render/benchmark_test.go +++ b/render/benchmark_test.go @@ -14,52 +14,30 @@ import ( var ( benchReportFile = flag.String("bench-report-file", "", "json report file to use for benchmarking (relative to this package)") - benchmarkRenderResult map[string]report.Node - benchmarkStatsResult render.Stats + benchmarkRenderResult render.Nodes ) func BenchmarkEndpointRender(b *testing.B) { benchmarkRender(b, render.EndpointRenderer) } -func BenchmarkEndpointStats(b *testing.B) { benchmarkStats(b, render.EndpointRenderer) } func BenchmarkProcessRender(b *testing.B) { benchmarkRender(b, render.ProcessRenderer) } -func BenchmarkProcessStats(b *testing.B) { benchmarkStats(b, render.ProcessRenderer) } func BenchmarkProcessWithContainerNameRender(b *testing.B) { benchmarkRender(b, render.ProcessWithContainerNameRenderer) } -func BenchmarkProcessWithContainerNameStats(b *testing.B) { - benchmarkStats(b, render.ProcessWithContainerNameRenderer) -} func BenchmarkProcessNameRender(b *testing.B) { benchmarkRender(b, render.ProcessNameRenderer) } -func BenchmarkProcessNameStats(b *testing.B) { benchmarkStats(b, render.ProcessNameRenderer) } func BenchmarkContainerRender(b *testing.B) { benchmarkRender(b, render.ContainerRenderer) } -func BenchmarkContainerStats(b *testing.B) { benchmarkStats(b, render.ContainerRenderer) } func BenchmarkContainerWithImageNameRender(b *testing.B) { benchmarkRender(b, render.ContainerWithImageNameRenderer) } -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 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 benchmarkRender(b *testing.B, r render.Renderer) { @@ -74,30 +52,13 @@ func benchmarkRender(b *testing.B, r render.Renderer) { b.StopTimer() render.ResetCache() b.StartTimer() - benchmarkRenderResult = r.Render(report, FilterNoop).Nodes - if len(benchmarkRenderResult) == 0 { + benchmarkRenderResult = r.Render(report, FilterNoop) + if len(benchmarkRenderResult.Nodes) == 0 { b.Errorf("Rendered topology contained no nodes") } } } -func benchmarkStats(b *testing.B, r render.Renderer) { - report, err := loadReport() - if err != nil { - b.Fatal(err) - } - b.ReportAllocs() - b.ResetTimer() - - for i := 0; i < b.N; i++ { - // No way to tell if this was successful :( - b.StopTimer() - render.ResetCache() - b.StartTimer() - benchmarkStatsResult = r.Stats(report, FilterNoop) - } -} - func loadReport() (report.Report, error) { if *benchReportFile == "" { return fixture.Report, nil diff --git a/render/container.go b/render/container.go index 4d32c857e..21fa42e0f 100644 --- a/render/container.go +++ b/render/container.go @@ -105,10 +105,6 @@ func (c connectionJoin) Render(rpt report.Report, dct Decorator) Nodes { return ret.result() } -func (c connectionJoin) Stats(rpt report.Report, _ Decorator) Stats { - return Stats{} // nothing to report -} - // FilterEmpty is a Renderer which filters out nodes which have no children // from the specified topology. func FilterEmpty(topology string, r Renderer) Renderer { diff --git a/render/filters.go b/render/filters.go index f2a434a98..877149cb0 100644 --- a/render/filters.go +++ b/render/filters.go @@ -32,13 +32,6 @@ func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) Nodes { return finalNodes } -// Stats implements Renderer -func (p PreciousNodeRenderer) Stats(rpt report.Report, dct Decorator) Stats { - // default to the underlying renderer - // TODO: we don't take into account the precious node, so we may be off by one - return p.Renderer.Stats(rpt, dct) -} - // CustomRenderer allow for mapping functions that received the entire topology // in one call - useful for functions that need to consider the entire graph. // We should minimise the use of this renderer type, as it is very inflexible. @@ -193,15 +186,6 @@ func (f *Filter) render(rpt report.Report, dct Decorator) Nodes { return Nodes{Nodes: output, Filtered: filtered} } -// Stats implements Renderer. General logic is to take the first (i.e. -// highest-level) stats we find, so upstream stats are ignored. This means that -// if we want to count the stats from multiple filters we need to compose their -// FilterFuncs, into a single Filter. -func (f Filter) Stats(rpt report.Report, dct Decorator) Stats { - nodes := f.render(rpt, dct) - return Stats{FilteredNodes: nodes.Filtered} -} - // IsConnected is the key added to Node.Metadata by ColorConnected // to indicate a node has an edge pointing to it or from it const IsConnected = "is_connected" diff --git a/render/host.go b/render/host.go index 5af0b5dbc..2b71560a4 100644 --- a/render/host.go +++ b/render/host.go @@ -90,7 +90,3 @@ func (e endpoints2Hosts) Render(rpt report.Report, dct Decorator) Nodes { ret.fixupAdjacencies(endpoints) return ret.result() } - -func (e endpoints2Hosts) Stats(rpt report.Report, _ Decorator) Stats { - return Stats{} // nothing to report -} diff --git a/render/memoise_test.go b/render/memoise_test.go index 8159a906f..ea1acfd5f 100644 --- a/render/memoise_test.go +++ b/render/memoise_test.go @@ -12,7 +12,6 @@ import ( type renderFunc func(r report.Report) render.Nodes func (f renderFunc) Render(r report.Report, _ render.Decorator) render.Nodes { return f(r) } -func (f renderFunc) Stats(r report.Report, _ render.Decorator) render.Stats { return render.Stats{} } func TestMemoise(t *testing.T) { calls := 0 diff --git a/render/pod.go b/render/pod.go index f76cd3ea4..ccff7fbaa 100644 --- a/render/pod.go +++ b/render/pod.go @@ -138,10 +138,6 @@ func (s selectPodsWithDeployments) Render(rpt report.Report, dct Decorator) Node return Nodes{Nodes: result} } -func (s selectPodsWithDeployments) Stats(rpt report.Report, _ Decorator) Stats { - return Stats{} -} - // MapPod2IP maps pod nodes to their IP address. This allows pods to // be joined directly with the endpoint topology. func MapPod2IP(m report.Node) []string { diff --git a/render/process.go b/render/process.go index e23c352d6..c2415bbc7 100644 --- a/render/process.go +++ b/render/process.go @@ -132,10 +132,6 @@ func (e endpoints2Processes) Render(rpt report.Report, dct Decorator) Nodes { return ret.result() } -func (e endpoints2Processes) Stats(rpt report.Report, _ Decorator) Stats { - return Stats{} // nothing to report -} - // MapProcess2Name maps process Nodes to Nodes // for each process name. // diff --git a/render/render.go b/render/render.go index f95961e4c..80c85a831 100644 --- a/render/render.go +++ b/render/render.go @@ -13,12 +13,6 @@ type MapFunc func(report.Node, report.Networks) report.Nodes // Renderer is something that can render a report to a set of Nodes. type Renderer interface { Render(report.Report, Decorator) Nodes - Stats(report.Report, Decorator) Stats -} - -// Stats is the type returned by Renderer.Stats -type Stats struct { - FilteredNodes int } // Nodes is the result of Rendering @@ -35,12 +29,6 @@ func (r Nodes) Merge(o Nodes) Nodes { } } -func (s Stats) merge(other Stats) Stats { - return Stats{ - FilteredNodes: s.FilteredNodes + other.FilteredNodes, - } -} - // Reduce renderer is a Renderer which merges together the output of several // other renderers. type Reduce []Renderer @@ -73,15 +61,6 @@ func (r Reduce) Render(rpt report.Report, dct Decorator) Nodes { return <-c } -// Stats implements Renderer -func (r Reduce) Stats(rpt report.Report, dct Decorator) Stats { - var result Stats - for _, renderer := range r { - result = result.merge(renderer.Stats(rpt, dct)) - } - return result -} - // Map is a Renderer which produces a set of Nodes from the set of // Nodes produced by another Renderer. type Map struct { @@ -132,14 +111,6 @@ func (m *Map) Render(rpt report.Report, dct Decorator) Nodes { return Nodes{Nodes: output} } -// Stats implements Renderer -func (m *Map) Stats(_ report.Report, _ Decorator) Stats { - // There doesn't seem to be an instance where we want stats to recurse - // through Maps - for instance we don't want to see the number of filtered - // processes in the container renderer. - return Stats{} -} - // Decorator transforms one renderer to another. e.g. Filters. type Decorator func(Renderer) Renderer @@ -163,12 +134,6 @@ func (ad applyDecorator) Render(rpt report.Report, dct Decorator) Nodes { } return ad.Renderer.Render(rpt, nil) } -func (ad applyDecorator) Stats(rpt report.Report, dct Decorator) Stats { - if dct != nil { - return dct(ad.Renderer).Stats(rpt, nil) - } - return Stats{} -} // ApplyDecorator returns a renderer which will apply the given decorator to the child render. func ApplyDecorator(renderer Renderer) Renderer { @@ -202,12 +167,6 @@ func (cr conditionalRenderer) Render(rpt report.Report, dct Decorator) Nodes { } return Nodes{} } -func (cr conditionalRenderer) Stats(rpt report.Report, dct Decorator) Stats { - if cr.Condition(rpt) { - return cr.Renderer.Stats(rpt, dct) - } - return Stats{} -} // ConstantRenderer renders a fixed set of nodes type ConstantRenderer struct { @@ -219,11 +178,6 @@ func (c ConstantRenderer) Render(_ report.Report, _ Decorator) Nodes { return c.Nodes } -// Stats implements Renderer -func (c ConstantRenderer) Stats(_ report.Report, _ Decorator) Stats { - return Stats{} -} - // joinResults is used by Renderers that join sets of nodes type joinResults struct { nodes report.Nodes diff --git a/render/render_test.go b/render/render_test.go index e0fb7a318..b671effb2 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -19,7 +19,6 @@ func (m mockRenderer) Render(rpt report.Report, d render.Decorator) render.Nodes } return render.Nodes{Nodes: m.Nodes} } -func (m mockRenderer) Stats(rpt report.Report, _ render.Decorator) render.Stats { return render.Stats{} } func TestReduceRender(t *testing.T) { renderer := render.Reduce([]render.Renderer{ diff --git a/render/selectors.go b/render/selectors.go index 7d6f46262..e134aad5b 100644 --- a/render/selectors.go +++ b/render/selectors.go @@ -14,11 +14,6 @@ func (t TopologySelector) Render(r report.Report, _ Decorator) Nodes { return Nodes{Nodes: topology.Nodes} } -// Stats implements Renderer -func (t TopologySelector) Stats(r report.Report, _ Decorator) Stats { - return Stats{} -} - // The topology selectors implement a Renderer which fetch the nodes from the // various report topologies. var ( From 21a91fe9ddf9d1dd6ed9d96a1818054be66f41cb Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 7 Nov 2017 22:26:44 +0000 Subject: [PATCH 3/3] remove HostRenderer memoisation since it was only introduced to because producing stats would hit the renderer twice, which is no longer the case. --- render/host.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/render/host.go b/render/host.go index 2b71560a4..44e19a36d 100644 --- a/render/host.go +++ b/render/host.go @@ -7,7 +7,8 @@ import ( // HostRenderer is a Renderer which produces a renderable host // graph from the host topology. // -var HostRenderer = Memoise(MakeReduce( +// not memoised +var HostRenderer = MakeReduce( endpoints2Hosts{}, MakeMap( MapX2Host, @@ -26,7 +27,7 @@ var HostRenderer = Memoise(MakeReduce( PodRenderer, ), SelectHost, -)) +) // MapX2Host maps any Nodes to host Nodes. //