From 30bef738c00da3adbf1e2fa9b78d6b9cc81cad87 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 28 Dec 2017 17:08:20 +0000 Subject: [PATCH 1/2] refactor: make PropagateSingleMetrics a renderer instead of a Map, since it's not really a map as it just updates the given node. This is more efficient and also matches what we do in similar situations elsewhere, e.g. in ContainerWithImageNameRenderer and ProcessWithContainerNameRenderer. --- render/metrics.go | 26 ++++++++++++++++++++------ render/metrics_test.go | 2 +- render/pod.go | 6 ++---- 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/render/metrics.go b/render/metrics.go index 155bf6895..79b008ea0 100644 --- a/render/metrics.go +++ b/render/metrics.go @@ -4,13 +4,26 @@ import ( "github.com/weaveworks/scope/report" ) -// PropagateSingleMetrics puts metrics from one of the children onto the parent -// iff there is only one child of that type. -func PropagateSingleMetrics(topology string) MapFunc { - return func(n report.Node) report.Nodes { +// PropagateSingleMetrics creates a renderer which propagates metrics +// from a node's child to the node. The child is selected based on the +// specified topology. No metrics are propagated when there is more +// than one such child. +func PropagateSingleMetrics(topology string, r Renderer) Renderer { + return propagateSingleMetrics{topology: topology, r: r} +} + +type propagateSingleMetrics struct { + topology string + r Renderer +} + +func (p propagateSingleMetrics) Render(rpt report.Report) Nodes { + nodes := p.r.Render(rpt) + outputs := make(report.Nodes, len(nodes.Nodes)) + for id, n := range nodes.Nodes { var found []report.Node n.Children.ForEach(func(child report.Node) { - if child.Topology == topology { + if child.Topology == p.topology { if _, ok := child.Latest.Lookup(report.DoesNotMakeConnections); !ok { found = append(found, child) } @@ -19,6 +32,7 @@ func PropagateSingleMetrics(topology string) MapFunc { if len(found) == 1 { n = n.WithMetrics(found[0].Metrics) } - return report.Nodes{n.ID: n} + outputs[id] = n } + return Nodes{Nodes: outputs, Filtered: nodes.Filtered} } diff --git a/render/metrics_test.go b/render/metrics_test.go index f4f81210c..f604d2fab 100644 --- a/render/metrics_test.go +++ b/render/metrics_test.go @@ -159,7 +159,7 @@ func TestPropagateSingleMetrics(t *testing.T) { }, }, } { - got := render.PropagateSingleMetrics(c.topology)(c.input) + got := render.PropagateSingleMetrics(c.topology, mockRenderer{report.Nodes{c.input.ID: c.input}}).Render(report.Report{}).Nodes if !reflect.DeepEqual(got, c.output) { t.Errorf("[%s] Diff: %s", c.name, test.Diff(c.output, got)) } diff --git a/render/pod.go b/render/pod.go index 34bbb5115..26b895f32 100644 --- a/render/pod.go +++ b/render/pod.go @@ -49,8 +49,7 @@ var PodRenderer = Memoise(ConditionalRenderer(renderKubernetesTopologies, return (!ok || state != kubernetes.StateDeleted) }, MakeReduce( - MakeMap( - PropagateSingleMetrics(report.Container), + PropagateSingleMetrics(report.Container, MakeMap( Map2Parent([]string{report.Pod}, UnmanagedID), MakeFilter( @@ -101,8 +100,7 @@ func renderParents(childTopology string, parentTopologies []string, noParentsPse } return MakeReduce(append( selectors, - MakeMap( - PropagateSingleMetrics(childTopology), + PropagateSingleMetrics(childTopology, MakeMap( Map2Parent(parentTopologies, noParentsPseudoID), childRenderer, From 4984da6f043e38008d4c6a4d7ed292276296ef82 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Thu, 28 Dec 2017 18:41:18 +0000 Subject: [PATCH 2/2] fly-by optimisation save a few allocations --- render/metrics.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/render/metrics.go b/render/metrics.go index 79b008ea0..92566519b 100644 --- a/render/metrics.go +++ b/render/metrics.go @@ -21,16 +21,20 @@ func (p propagateSingleMetrics) Render(rpt report.Report) Nodes { nodes := p.r.Render(rpt) outputs := make(report.Nodes, len(nodes.Nodes)) for id, n := range nodes.Nodes { - var found []report.Node + var first report.Node + found := 0 n.Children.ForEach(func(child report.Node) { if child.Topology == p.topology { if _, ok := child.Latest.Lookup(report.DoesNotMakeConnections); !ok { - found = append(found, child) + if found == 0 { + first = child + } + found++ } } }) - if len(found) == 1 { - n = n.WithMetrics(found[0].Metrics) + if found == 1 { + n = n.WithMetrics(first.Metrics) } outputs[id] = n }