diff --git a/app/api_topologies.go b/app/api_topologies.go index 0a8ca0a98..003742bb4 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -336,9 +336,8 @@ type APITopologyOptionGroup struct { NoneLabel string `json:"noneLabel,omitempty"` } -// Get the render filters to use for this option group as a Decorator, if any. -// If second arg is false, no decorator was needed. -func (g APITopologyOptionGroup) getFilterDecorator(value string) (render.Decorator, bool) { +// Get the render filters to use for this option group, if any, or nil otherwise. +func (g APITopologyOptionGroup) filter(value string) render.FilterFunc { selectType := g.SelectType if selectType == "" { selectType = "one" @@ -351,7 +350,7 @@ func (g APITopologyOptionGroup) getFilterDecorator(value string) (render.Decorat values = strings.Split(value, ",") default: log.Errorf("Invalid select type %s for option group %s, ignoring option", selectType, g.ID) - return nil, false + return nil } filters := []render.FilterFunc{} for _, opt := range g.Options { @@ -374,11 +373,9 @@ func (g APITopologyOptionGroup) getFilterDecorator(value string) (render.Decorat } } if len(filters) == 0 { - return nil, false + return nil } - // Since we've encoded whether to ignore pseudo topologies into each subfilter, - // we want no special behaviour for pseudo topologies here, which corresponds to MakePseudo - return render.MakeFilterPseudoDecorator(render.AnyFilterFunc(filters...)), true + return render.AnyFilterFunc(filters...) } // APITopologyOption describes a ¶m=value to a given topology. @@ -487,24 +484,24 @@ func (r *Registry) renderTopologies(rpt report.Report, req *http.Request) []APIT topologies := []APITopologyDesc{} req.ParseForm() r.walk(func(desc APITopologyDesc) { - renderer, decorator, _ := r.RendererForTopology(desc.id, req.Form, rpt) - desc.Stats = decorateWithStats(rpt, renderer, decorator) + renderer, filter, _ := r.RendererForTopology(desc.id, req.Form, rpt) + desc.Stats = decorateWithStats(rpt, renderer, filter) for i, sub := range desc.SubTopologies { - renderer, decorator, _ := r.RendererForTopology(sub.id, req.Form, rpt) - desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, decorator) + renderer, filter, _ := r.RendererForTopology(sub.id, req.Form, rpt) + desc.SubTopologies[i].Stats = decorateWithStats(rpt, renderer, filter) } topologies = append(topologies, desc) }) return updateFilters(rpt, topologies) } -func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator render.Decorator) topologyStats { +func decorateWithStats(rpt report.Report, renderer render.Renderer, filter render.FilterFunc) topologyStats { var ( nodes int realNodes int edges int ) - r := render.Decorate(rpt, renderer, decorator) + r := render.Render(rpt, renderer, filter) for _, n := range r.Nodes { nodes++ if n.Topology != render.Pseudo { @@ -521,7 +518,7 @@ func decorateWithStats(rpt report.Report, renderer render.Renderer, decorator re } // RendererForTopology .. -func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.Decorator, error) { +func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt report.Report) (render.Renderer, render.FilterFunc, error) { topology, ok := r.get(topologyID) if !ok { return nil, nil, fmt.Errorf("topology not found: %s", topologyID) @@ -533,15 +530,15 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt return topology.renderer, nil, nil } - var decorators []render.Decorator + var filters []render.FilterFunc for _, group := range topology.Options { value := values.Get(group.ID) - if decorator, ok := group.getFilterDecorator(value); ok { - decorators = append(decorators, decorator) + if filter := group.filter(value); filter != nil { + filters = append(filters, filter) } } - if len(decorators) > 0 { - return topology.renderer, render.ComposeDecorators(decorators...), nil + if len(filters) > 0 { + return topology.renderer, render.ComposeFilterFuncs(filters...), nil } return topology.renderer, nil, nil } @@ -554,7 +551,7 @@ func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { } } -type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.RenderContext, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.FilterFunc, report.RenderContext, http.ResponseWriter, *http.Request) func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { @@ -572,11 +569,11 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu return } req.ParseForm() - renderer, decorator, err := r.RendererForTopology(topologyID, req.Form, rpt) + renderer, filter, err := r.RendererForTopology(topologyID, req.Form, rpt) if err != nil { respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, decorator, RenderContextForReporter(rep, rpt), w, req) + f(ctx, renderer, filter, RenderContextForReporter(rep, rpt), w, req) } } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index d468c7947..a238adcf4 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -109,7 +109,7 @@ func TestRendererForTopologyWithFiltering(t *testing.T) { urlvalues.Set(systemGroupID, customAPITopologyOptionFilterID) urlvalues.Set("stopped", "running") urlvalues.Set("pseudo", "hide") - renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) + renderer, filter, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) if err != nil { t.Fatalf("Topology Registry Report error: %s", err) } @@ -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(render.Decorate(input, renderer, decorator).Nodes) + have := utils.Prune(render.Render(input, renderer, filter).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, fixture.ClientContainerNodeID) delete(want, render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID)) @@ -140,7 +140,7 @@ func TestRendererForTopologyNoFiltering(t *testing.T) { urlvalues.Set(systemGroupID, customAPITopologyOptionFilterID) urlvalues.Set("stopped", "running") urlvalues.Set("pseudo", "hide") - renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) + renderer, filter, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) if err != nil { t.Fatalf("Topology Registry Report error: %s", err) } @@ -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(render.Decorate(input, renderer, decorator).Nodes) + have := utils.Prune(render.Render(input, renderer, filter).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID)) delete(want, render.OutgoingInternetID) @@ -178,12 +178,12 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det urlvalues.Set(systemGroupID, customAPITopologyOptionFilterID) urlvalues.Set("stopped", "running") urlvalues.Set("pseudo", "hide") - renderer, decorator, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) + renderer, filter, err := topologyRegistry.RendererForTopology("containers", urlvalues, fixture.Report) if err != nil { return nil, err } - return detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.Decorate(fixture.Report, renderer, decorator).Nodes), nil + return detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.Render(fixture.Report, renderer, filter).Nodes), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { diff --git a/app/api_topology.go b/app/api_topology.go index e0efd4e06..c418afd14 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,23 +29,23 @@ 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) { +func handleTopology(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(rc, render.Decorate(rc.Report, renderer, decorator).Nodes), + Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, filter).Nodes), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleNode(ctx context.Context, renderer render.Renderer, filter render.FilterFunc, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] nodeID = vars["id"] ) - // We must not lose the node during decoration. We achieve that by + // We must not lose the node during filtering. 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 + // filtering, which gives us the node (if it exists at all), then + // (2) performing a normal filtered 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 @@ -57,11 +57,11 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. http.NotFound(w, r) return } - 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 + if filter != nil { + nodes = render.Render(rc.Report, render.ConstantRenderer{Nodes: nodes}, filter) + if filteredNode, ok := nodes.Nodes[nodeID]; ok { + node = filteredNode + } else { // we've lost the node during filtering; put it back nodes.Nodes[nodeID] = node nodes.Filtered-- } @@ -135,12 +135,12 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re) + renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re) if err != nil { log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(RenderContextForReporter(rep, re), render.Decorate(re, renderer, decorator).Nodes) + newTopo := detailed.Summaries(RenderContextForReporter(rep, re), render.Render(re, renderer, filter).Nodes) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/benchmark_internal_test.go b/app/benchmark_internal_test.go index bbdd87c5d..537883b3c 100644 --- a/app/benchmark_internal_test.go +++ b/app/benchmark_internal_test.go @@ -65,10 +65,10 @@ func BenchmarkTopologyContainers(b *testing.B) { func benchmarkOneTopology(b *testing.B, topologyID string) { benchmarkRender(b, func(report report.Report) { - renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, url.Values{}, report) + renderer, filter, err := topologyRegistry.RendererForTopology(topologyID, url.Values{}, report) if err != nil { b.Fatal(err) } - render.Decorate(report, renderer, decorator) + render.Render(report, renderer, filter) }) } diff --git a/render/container_test.go b/render/container_test.go index 1001f17be..9ec27887c 100644 --- a/render/container_test.go +++ b/render/container_test.go @@ -15,18 +15,10 @@ import ( "github.com/weaveworks/scope/test/utils" ) -// FilterApplication is a Renderer which filters out application nodes. -func FilterApplication(r render.Renderer) render.Renderer { - return render.MakeFilter(render.IsApplication, r) -} - -// FilterSystem is a Renderer which filters out system nodes. -func FilterSystem(r render.Renderer) render.Renderer { - return render.MakeFilter(render.IsSystem, r) -} - -// FilterNoop does nothing. -func FilterNoop(r render.Renderer) render.Renderer { return r } +var ( + filterApplication = render.AnyFilterFunc(render.IsPseudoTopology, render.IsApplication) + filterSystem = render.AnyFilterFunc(render.IsPseudoTopology, render.IsSystem) +) func TestMapProcess2Container(t *testing.T) { for _, input := range []testcase{ @@ -70,12 +62,10 @@ func TestContainerFilterRenderer(t *testing.T) { // tag on of the containers in the topology and ensure // it is filtered out correctly. input := fixture.Report.Copy() - input.Container.Nodes[fixture.ClientContainerNodeID] = input.Container.Nodes[fixture.ClientContainerNodeID].WithLatests(map[string]string{ docker.LabelPrefix + "works.weave.role": "system", }) - - have := utils.Prune(render.Decorate(input, render.ContainerWithImageNameRenderer, FilterApplication).Nodes) + have := utils.Prune(render.Render(input, render.ContainerWithImageNameRenderer, filterApplication).Nodes) want := utils.Prune(expected.RenderedContainers.Copy()) delete(want, fixture.ClientContainerNodeID) if !reflect.DeepEqual(want, have) { @@ -84,7 +74,7 @@ func TestContainerFilterRenderer(t *testing.T) { } func TestContainerHostnameRenderer(t *testing.T) { - have := utils.Prune(render.Decorate(fixture.Report, render.ContainerHostnameRenderer, FilterNoop).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, nil).Nodes) want := utils.Prune(expected.RenderedContainerHostnames) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -92,7 +82,7 @@ func TestContainerHostnameRenderer(t *testing.T) { } func TestContainerHostnameFilterRenderer(t *testing.T) { - have := utils.Prune(render.Decorate(fixture.Report, render.ContainerHostnameRenderer, FilterSystem).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerHostnameRenderer, filterSystem).Nodes) want := utils.Prune(expected.RenderedContainerHostnames.Copy()) delete(want, fixture.ClientContainerHostname) delete(want, fixture.ServerContainerHostname) @@ -103,7 +93,7 @@ func TestContainerHostnameFilterRenderer(t *testing.T) { } func TestContainerImageRenderer(t *testing.T) { - have := utils.Prune(render.Decorate(fixture.Report, render.ContainerImageRenderer, FilterNoop).Nodes) + have := utils.Prune(render.Render(fixture.Report, render.ContainerImageRenderer, nil).Nodes) want := utils.Prune(expected.RenderedContainerImages) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -111,7 +101,7 @@ func TestContainerImageRenderer(t *testing.T) { } func TestContainerImageFilterRenderer(t *testing.T) { - have := utils.Prune(render.Decorate(fixture.Report, render.ContainerImageRenderer, FilterSystem).Nodes) + have := utils.Prune(render.Render(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 a88fcff53..e5bdf6278 100644 --- a/render/filters.go +++ b/render/filters.go @@ -106,22 +106,6 @@ func MakeFilterPseudo(f FilterFunc, r Renderer) Renderer { } } -// MakeFilterDecorator makes a decorator that filters out non-pseudo nodes -// which match the predicate. -func MakeFilterDecorator(f FilterFunc) Decorator { - return func(renderer Renderer) Renderer { - return MakeFilter(f, renderer) - } -} - -// MakeFilterPseudoDecorator makes a decorator that filters out all nodes -// (including pseudo nodes) which match the predicate. -func MakeFilterPseudoDecorator(f FilterFunc) Decorator { - return func(renderer Renderer) Renderer { - return MakeFilterPseudo(f, renderer) - } -} - // Render implements Renderer func (f Filter) Render(rpt report.Report) Nodes { return f.render(rpt) diff --git a/render/filters_test.go b/render/filters_test.go index 2933ad5ea..5854641fd 100644 --- a/render/filters_test.go +++ b/render/filters_test.go @@ -9,13 +9,8 @@ import ( "github.com/weaveworks/scope/test/reflect" ) -func isNotBar(renderer render.Renderer) render.Renderer { - return &render.Filter{ - FilterFunc: func(node report.Node) bool { - return node.ID != "bar" - }, - Renderer: renderer, - } +func isNotBar(node report.Node) bool { + return node.ID != "bar" } func TestFilterRender(t *testing.T) { @@ -25,7 +20,7 @@ func TestFilterRender(t *testing.T) { "baz": report.MakeNode("baz"), }} have := report.MakeIDList() - for id := range render.Decorate(report.MakeReport(), renderer, render.FilterUnconnected).Nodes { + for id := range render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes { have = have.Add(id) } want := report.MakeIDList("foo", "bar") @@ -41,7 +36,7 @@ func TestFilterRender2(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("foo"), "baz": report.MakeNode("baz"), }} - have := render.Decorate(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes if have["foo"].Adjacency.Contains("bar") { t.Error("adjacencies for removed nodes should have been removed") } @@ -57,16 +52,8 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "baz": report.MakeNode("baz").WithTopology(render.Pseudo), } renderer := mockRenderer{Nodes: nodes} - filter := func(renderer render.Renderer) render.Renderer { - return &render.Filter{ - FilterFunc: func(node report.Node) bool { - return true - }, - Renderer: renderer, - } - } want := nodes - have := render.Decorate(report.MakeReport(), renderer, filter).Nodes + have := render.Render(report.MakeReport(), renderer, nil).Nodes if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } @@ -77,7 +64,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("baz"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo), }} - have := render.Decorate(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -88,7 +75,7 @@ func TestFilterUnconnectedPseudoNodes(t *testing.T) { "bar": report.MakeNode("bar").WithAdjacent("foo"), "baz": report.MakeNode("baz").WithTopology(render.Pseudo).WithAdjacent("bar"), }} - have := render.Decorate(report.MakeReport(), renderer, isNotBar).Nodes + have := render.Render(report.MakeReport(), renderer, isNotBar).Nodes if _, ok := have["baz"]; ok { t.Error("expected the unconnected pseudonode baz to have been removed") } @@ -102,7 +89,7 @@ func TestFilterUnconnectedSelf(t *testing.T) { "foo": report.MakeNode("foo").WithAdjacent("foo"), } renderer := mockRenderer{Nodes: nodes} - have := render.Decorate(report.MakeReport(), renderer, render.FilterUnconnected).Nodes + have := render.Render(report.MakeReport(), render.ColorConnected(renderer), render.IsConnected).Nodes if len(have) > 0 { t.Error("expected node only connected to self to be removed") } diff --git a/render/pod_test.go b/render/pod_test.go index 67264fe4b..01c598341 100644 --- a/render/pod_test.go +++ b/render/pod_test.go @@ -20,9 +20,7 @@ func TestPodRenderer(t *testing.T) { } } -func filterNonKubeSystem(renderer render.Renderer) render.Renderer { - return render.MakeFilter(render.Complement(render.IsNamespace("kube-system")), renderer) -} +var filterNonKubeSystem = render.Complement(render.IsNamespace("kube-system")) func TestPodFilterRenderer(t *testing.T) { // tag on containers or pod namespace in the topology and ensure @@ -32,7 +30,7 @@ func TestPodFilterRenderer(t *testing.T) { kubernetes.Namespace: "kube-system", }) - have := utils.Prune(render.Decorate(input, render.PodRenderer, filterNonKubeSystem).Nodes) + have := utils.Prune(render.Render(input, render.PodRenderer, filterNonKubeSystem).Nodes) want := utils.Prune(expected.RenderedPods.Copy()) delete(want, fixture.ClientPodNodeID) if !reflect.DeepEqual(want, have) { @@ -56,7 +54,7 @@ func TestPodServiceFilterRenderer(t *testing.T) { kubernetes.Namespace: "kube-system", }) - have := utils.Prune(render.Decorate(input, render.PodServiceRenderer, filterNonKubeSystem).Nodes) + have := utils.Prune(render.Render(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 4379e829f..502f68715 100644 --- a/render/render.go +++ b/render/render.go @@ -29,10 +29,10 @@ 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) +// Render renders the report and then applies the filter +func Render(rpt report.Report, renderer Renderer, filter FilterFunc) Nodes { + if filter != nil { + renderer = MakeFilterPseudo(filter, renderer) } return renderer.Render(rpt) } @@ -119,19 +119,6 @@ func (m Map) Render(rpt report.Report) Nodes { return Nodes{Nodes: output} } -// Decorator transforms one renderer to another. e.g. Filters. -type Decorator func(Renderer) Renderer - -// ComposeDecorators composes decorators into one. -func ComposeDecorators(decorators ...Decorator) Decorator { - return func(r Renderer) Renderer { - for _, decorator := range decorators { - r = decorator(r) - } - return r - } -} - 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)