diff --git a/app/api_topologies.go b/app/api_topologies.go index 78481e355..0a8ca0a98 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -504,7 +504,7 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re realNodes int edges int ) - r := renderer.Render(rpt, decorator) + r := render.Decorate(rpt, renderer, decorator) for _, n := range r.Nodes { nodes++ if n.Topology != render.Pseudo { @@ -541,10 +541,7 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt } } if len(decorators) > 0 { - // Here we tell the topology renderer to apply the filtering decorator - // that we construct as a composition of all the selected filters. - composedFilterDecorator := render.ComposeDecorators(decorators...) - return render.ApplyDecorator(topology.renderer), composedFilterDecorator, nil + return topology.renderer, render.ComposeDecorators(decorators...), nil } return topology.renderer, nil, nil } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index 64fda080f..d468c7947 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).Nodes) + have := utils.Prune(render.Decorate(input, renderer, 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).Nodes) + have := utils.Prune(render.Decorate(input, renderer, 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).Nodes), nil + return detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.Decorate(fixture.Report, renderer, decorator).Nodes), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { diff --git a/app/api_topology.go b/app/api_topology.go index 6c95e7642..ffa339e86 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -31,25 +31,42 @@ 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), + Nodes: detailed.Summaries(rc, render.Decorate(rc.Report, renderer, decorator).Nodes), }) } // Individual nodes. func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { var ( - vars = mux.Vars(r) - topologyID = vars["topology"] - nodeID = vars["id"] - preciousRenderer = render.PreciousNodeRenderer{PreciousNodeID: nodeID, Renderer: renderer} - rendered = preciousRenderer.Render(rc.Report, decorator).Nodes - node, ok = rendered[nodeID] + vars = mux.Vars(r) + topologyID = vars["topology"] + nodeID = vars["id"] ) + // We must not lose the node during decoration. We achieve that by + // (1) rendering the report with the base renderer, without + // decoration, which gives us the node (if it exists at all), then + // (2) performing a normal decorated render of the report. If the + // node is lost in the second step, we simply put it back. + // + // To avoid repeating the work from step (1) in step (2), we + // replace the renderer in the latter with a constant renderer of + // the result obtained in step (1). + nodes := renderer.Render(rc.Report, nil) + node, ok := nodes.Nodes[nodeID] if !ok { http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, rendered, node)}) + if decorator != nil { + nodes = render.Decorate(rc.Report, render.ConstantRenderer{Nodes: nodes}, decorator) + if decoratedNode, ok := nodes.Nodes[nodeID]; ok { + node = decoratedNode + } else { // we've lost the node during decoration; put it back + nodes.Nodes[nodeID] = node + nodes.Filtered-- + } + } + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, nodes.Nodes, node)}) } // Websocket for the full topology. @@ -123,7 +140,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(RenderContextForReporter(rep, re), renderer.Render(re, decorator).Nodes) + newTopo := detailed.Summaries(RenderContextForReporter(rep, re), render.Decorate(re, renderer, decorator).Nodes) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/benchmark_internal_test.go b/app/benchmark_internal_test.go index 9eae97bfb..bbdd87c5d 100644 --- a/app/benchmark_internal_test.go +++ b/app/benchmark_internal_test.go @@ -69,6 +69,6 @@ func benchmarkOneTopology(b *testing.B, topologyID string) { if err != nil { b.Fatal(err) } - renderer.Render(report, decorator) + render.Decorate(report, renderer, decorator) }) } diff --git a/render/container_test.go b/render/container_test.go index 0d765b21d..156a41ade 100644 --- a/render/container_test.go +++ b/render/container_test.go @@ -70,13 +70,12 @@ func TestContainerFilterRenderer(t *testing.T) { // tag on of the containers in the topology and ensure // it is filtered out correctly. input := fixture.Report.Copy() - renderer := render.ApplyDecorator(render.ContainerWithImageNameRenderer) 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, FilterApplication).Nodes) + have := utils.Prune(render.Decorate(input, render.ContainerWithImageNameRenderer, FilterApplication).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, fixture.ClientContainerNodeID) if !reflect.DeepEqual(want, have) { @@ -85,8 +84,7 @@ func TestContainerFilterRenderer(t *testing.T) { } func TestContainerHostnameRenderer(t *testing.T) { - renderer := render.ApplyDecorator(render.ContainerHostnameRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterNoop).Nodes) + have := utils.Prune(render.Decorate(fixture.Report, render.ContainerHostnameRenderer, FilterNoop).Nodes) want := utils.Prune(expected.RenderedContainerHostnames) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -94,8 +92,7 @@ func TestContainerHostnameRenderer(t *testing.T) { } func TestContainerHostnameFilterRenderer(t *testing.T) { - renderer := render.ApplyDecorator(render.ContainerHostnameRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterSystem).Nodes) + have := utils.Prune(render.Decorate(fixture.Report, render.ContainerHostnameRenderer, FilterSystem).Nodes) want := utils.Prune(expected.RenderedContainerHostnames.Copy()) delete(want, fixture.ClientContainerHostname) delete(want, fixture.ServerContainerHostname) @@ -106,8 +103,7 @@ func TestContainerHostnameFilterRenderer(t *testing.T) { } func TestContainerImageRenderer(t *testing.T) { - renderer := render.ApplyDecorator(render.ContainerImageRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterNoop).Nodes) + have := utils.Prune(render.Decorate(fixture.Report, render.ContainerImageRenderer, FilterNoop).Nodes) want := utils.Prune(expected.RenderedContainerImages) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -115,8 +111,7 @@ func TestContainerImageRenderer(t *testing.T) { } func TestContainerImageFilterRenderer(t *testing.T) { - renderer := render.ApplyDecorator(render.ContainerImageRenderer) - have := utils.Prune(renderer.Render(fixture.Report, FilterSystem).Nodes) + have := utils.Prune(render.Decorate(fixture.Report, render.ContainerImageRenderer, FilterSystem).Nodes) want := utils.Prune(expected.RenderedContainerHostnames.Copy()) delete(want, fixture.ClientContainerHostname) delete(want, fixture.ServerContainerHostname) diff --git a/render/filters.go b/render/filters.go index aa60ebe0b..6bb888ec4 100644 --- a/render/filters.go +++ b/render/filters.go @@ -14,24 +14,6 @@ const ( swarmNamespaceLabel = "com.docker.stack.namespace" ) -// PreciousNodeRenderer ensures a node is never filtered out by decorators -type PreciousNodeRenderer struct { - PreciousNodeID string - Renderer -} - -// Render implements Renderer -func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) Nodes { - undecoratedNodes := p.Renderer.Render(rpt, nil) - 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 -} - // 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. diff --git a/render/pod_test.go b/render/pod_test.go index 9be35ffb1..92a7d3a9c 100644 --- a/render/pod_test.go +++ b/render/pod_test.go @@ -28,13 +28,11 @@ func TestPodFilterRenderer(t *testing.T) { // tag on containers or pod namespace in the topology and ensure // it is filtered out correctly. input := fixture.Report.Copy() - renderer := render.ApplyDecorator(render.PodRenderer) - input.Pod.Nodes[fixture.ClientPodNodeID] = input.Pod.Nodes[fixture.ClientPodNodeID].WithLatests(map[string]string{ kubernetes.Namespace: "kube-system", }) - have := utils.Prune(renderer.Render(input, filterNonKubeSystem).Nodes) + have := utils.Prune(render.Decorate(input, render.PodRenderer, filterNonKubeSystem).Nodes) want := utils.Prune(expected.RenderedPods.Copy()) delete(want, fixture.ClientPodNodeID) if !reflect.DeepEqual(want, have) { @@ -54,13 +52,11 @@ func TestPodServiceFilterRenderer(t *testing.T) { // tag on containers or pod namespace in the topology and ensure // it is filtered out correctly. input := fixture.Report.Copy() - renderer := render.ApplyDecorator(render.PodServiceRenderer) - input.Service.Nodes[fixture.ServiceNodeID] = input.Service.Nodes[fixture.ServiceNodeID].WithLatests(map[string]string{ kubernetes.Namespace: "kube-system", }) - have := utils.Prune(renderer.Render(input, filterNonKubeSystem).Nodes) + have := utils.Prune(render.Decorate(input, render.PodServiceRenderer, filterNonKubeSystem).Nodes) want := utils.Prune(expected.RenderedPodServices.Copy()) delete(want, fixture.ServiceNodeID) delete(want, render.IncomingInternetID) diff --git a/render/render.go b/render/render.go index 3741e89f4..317901d01 100644 --- a/render/render.go +++ b/render/render.go @@ -29,6 +29,14 @@ func (r Nodes) Merge(o Nodes) Nodes { } } +// Decorate renders the report with a decorated renderer +func Decorate(rpt report.Report, renderer Renderer, dct Decorator) Nodes { + if dct != nil { + renderer = dct(renderer) + } + return renderer.Render(rpt, nil) +} + // Reduce renderer is a Renderer which merges together the output of several // other renderers. type Reduce []Renderer @@ -124,22 +132,6 @@ func ComposeDecorators(decorators ...Decorator) Decorator { } } -type applyDecorator struct { - Renderer -} - -func (ad applyDecorator) Render(rpt report.Report, dct Decorator) Nodes { - if dct != nil { - return dct(ad.Renderer).Render(rpt, nil) - } - return ad.Renderer.Render(rpt, nil) -} - -// ApplyDecorator returns a renderer which will apply the given decorator to the child render. -func ApplyDecorator(renderer Renderer) Renderer { - return applyDecorator{renderer} -} - func propagateLatest(key string, from, to report.Node) report.Node { if value, timestamp, ok := from.Latest.LookupEntry(key); ok { to.Latest = to.Latest.Set(key, timestamp, value)