From d76da767d897560acd5d35095b34ccdfc51f1f2c Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2020 16:39:50 +0000 Subject: [PATCH] optimization: store just IDs of child nodes IDs are a lot smaller, hence quicker to manage. Any time we want to refer back to the full node we look it up in its topology. (This relies on nobody trying to store a rendered node as a child). --- render/container.go | 8 +- render/detailed/connections.go | 36 +++--- render/detailed/node.go | 20 +-- render/expected/expected.go | 216 ++++++++++++++++----------------- render/id.go | 2 +- render/metrics.go | 8 +- render/metrics_test.go | 154 +++++++---------------- render/persistentvolume.go | 10 +- render/render.go | 8 +- report/id.go | 13 ++ report/id_test.go | 16 +++ report/node.go | 16 +-- test/utils/prune.go | 6 +- 13 files changed, 233 insertions(+), 280 deletions(-) diff --git a/render/container.go b/render/container.go index 5a20984ea..ada6f03ba 100644 --- a/render/container.go +++ b/render/container.go @@ -103,13 +103,7 @@ func FilterEmpty(topology string, r Renderer) Renderer { // topology. func HasChildren(topology string) FilterFunc { return func(n report.Node) bool { - count := 0 - n.Children.ForEach(func(child report.Node) { - if child.Topology == topology { - count++ - } - }) - return count > 0 + return len(n.ChildIDs) > 0 } } diff --git a/render/detailed/connections.go b/render/detailed/connections.go index dbf35b607..3159fc79a 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -167,7 +167,7 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal } func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { - localEndpointIDs, localEndpointIDCopies := endpointChildIDsAndCopyMapOf(n) + localEndpointIDs, localEndpointIDCopies := endpointChildIDsAndCopyMapOf(r, n) counts := newConnectionCounters() // For each node which has an edge TO me @@ -175,7 +175,7 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod if !node.Adjacency.Contains(n.ID) { continue } - for _, remoteEndpoint := range endpointChildrenOf(node) { + for _, remoteEndpoint := range endpointChildrenOf(r, node) { for _, localEndpointID := range remoteEndpoint.Adjacency.Intersection(localEndpointIDs) { localEndpointID = canonicalEndpointID(localEndpointIDCopies, localEndpointID) counts.add(r.DNS, false, n, node, r.Endpoint.Nodes[localEndpointID], remoteEndpoint) @@ -197,7 +197,7 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod } func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { - localEndpoints := endpointChildrenOf(n) + localEndpoints := endpointChildrenOf(r, n) counts := newConnectionCounters() // For each node which has an edge FROM me @@ -206,7 +206,7 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod if !ok { continue } - remoteEndpointIDs, remoteEndpointIDCopies := endpointChildIDsAndCopyMapOf(node) + remoteEndpointIDs, remoteEndpointIDCopies := endpointChildIDsAndCopyMapOf(r, node) for _, localEndpoint := range localEndpoints { for _, remoteEndpointID := range localEndpoint.Adjacency.Intersection(remoteEndpointIDs) { remoteEndpointID = canonicalEndpointID(remoteEndpointIDCopies, remoteEndpointID) @@ -228,27 +228,31 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod } } -func endpointChildrenOf(n report.Node) []report.Node { +func endpointChildrenOf(r report.Report, n report.Node) []report.Node { result := []report.Node{} - n.Children.ForEach(func(child report.Node) { - if child.Topology == report.Endpoint { - result = append(result, child) + for _, childID := range n.ChildIDs { + if ty, ok := report.NodeIDType(childID); ok && ty == report.Endpoint { + if child, found := r.Endpoint.Nodes[childID]; found { + result = append(result, child) + } } - }) + } return result } -func endpointChildIDsAndCopyMapOf(n report.Node) (report.IDList, map[string]string) { +func endpointChildIDsAndCopyMapOf(r report.Report, n report.Node) (report.IDList, map[string]string) { ids := report.MakeIDList() copies := map[string]string{} - n.Children.ForEach(func(child report.Node) { - if child.Topology == report.Endpoint { - ids = ids.Add(child.ID) - if copyID, _, ok := child.Latest.LookupEntry(endpoint.CopyOf); ok { - copies[child.ID] = copyID + for _, childID := range n.ChildIDs { + if ty, ok := report.NodeIDType(childID); ok && ty == report.Endpoint { + ids = ids.Add(childID) + if child, found := r.Endpoint.Nodes[childID]; found { + if copyID, _, ok := child.Latest.LookupEntry(endpoint.CopyOf); ok { + copies[child.ID] = copyID + } } } - }) + } return ids, copies } diff --git a/render/detailed/node.go b/render/detailed/node.go index d96a739ed..2dc246e47 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -223,16 +223,20 @@ var nodeSummaryGroupSpecs = []struct { func children(rc RenderContext, n report.Node) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} - n.Children.ForEach(func(child report.Node) { - if child.ID == n.ID { - return + for _, childID := range n.ChildIDs { + if childID == n.ID { + continue } - summary, ok := MakeNodeSummary(rc, child) - if !ok { - return + if ty, ok := report.NodeIDType(childID); ok { + if topology, ok := rc.Report.Topology(ty); ok { + if child, ok := topology.Nodes[childID]; ok { + if summary, ok := MakeNodeSummary(rc, child); ok { + summaries[ty] = append(summaries[ty], summary.SummarizeMetrics()) + } + } + } } - summaries[child.Topology] = append(summaries[child.Topology], summary.SummarizeMetrics()) - }) + } nodeSummaryGroups := []NodeSummaryGroup{} // Apply specific group specs in the order they're listed diff --git a/render/expected/expected.go b/render/expected/expected.go index 7ce945e74..efa9038f8 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -51,28 +51,26 @@ var ( unknownPseudoNode1 = func(adjacent ...string) report.Node { return pseudo(UnknownPseudoNode1ID, adjacent...). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.UnknownClient1NodeID], - RenderedEndpoints[fixture.UnknownClient2NodeID], + WithChildren(report.MakeIDList( + fixture.UnknownClient1NodeID, + fixture.UnknownClient2NodeID, )) } unknownPseudoNode2 = func(adjacent ...string) report.Node { return pseudo(UnknownPseudoNode2ID, adjacent...). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.UnknownClient3NodeID], + WithChildren(report.MakeIDList( + fixture.UnknownClient3NodeID, )) } theIncomingInternetNode = func(adjacent ...string) report.Node { return pseudo(render.IncomingInternetID, adjacent...). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.RandomClientNodeID], + WithChildren(report.MakeIDList( + fixture.RandomClientNodeID, )) } - theOutgoingInternetNode = pseudo(render.OutgoingInternetID).WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.GoogleEndpointNodeID], - )) + theOutgoingInternetNode = pseudo(render.OutgoingInternetID).WithChildID(fixture.GoogleEndpointNodeID) RenderedEndpoints = report.Nodes{ fixture.Client54001NodeID: endpoint(fixture.Client54001NodeID, fixture.Server80NodeID), @@ -93,24 +91,16 @@ var ( process.PID: fixture.Client1PID, process.Name: fixture.Client1Name, }). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - )), + WithChildID(fixture.Client54001NodeID), fixture.ClientProcess2NodeID: processNode(fixture.ClientProcess2NodeID, fixture.ServerProcessNodeID). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54002NodeID], - )), + WithChildID(fixture.Client54002NodeID), fixture.ServerProcessNodeID: processNode(fixture.ServerProcessNodeID). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - )), + WithChildID(fixture.Server80NodeID), fixture.NonContainerProcessNodeID: processNode(fixture.NonContainerProcessNodeID, render.OutgoingInternetID). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.NonContainerNodeID], - )), + WithChildID(fixture.NonContainerNodeID), // due to https://github.com/weaveworks/scope/issues/1323 we are dropping // all non-internet pseudo nodes for now. @@ -124,26 +114,26 @@ var ( fixture.Client1Name: processNameNode(fixture.Client1Name, fixture.ServerName). WithLatests(map[string]string{process.Name: fixture.Client1Name}). AddCounter(report.Process, 2). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, )), fixture.ServerName: processNameNode(fixture.ServerName). WithLatests(map[string]string{process.Name: fixture.ServerName}). AddCounter(report.Process, 1). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - RenderedProcesses[fixture.ServerProcessNodeID], + WithChildren(report.MakeIDList( + fixture.Server80NodeID, + fixture.ServerProcessNodeID, )), fixture.NonContainerName: processNameNode(fixture.NonContainerName, render.OutgoingInternetID). AddCounter(report.Process, 1). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.NonContainerNodeID], - RenderedProcesses[fixture.NonContainerProcessNodeID], + WithChildren(report.MakeIDList( + fixture.NonContainerNodeID, + fixture.NonContainerProcessNodeID, )), // due to https://github.com/weaveworks/scope/issues/1323 we are dropping @@ -155,9 +145,9 @@ var ( } uncontainedServerID = render.MakePseudoNodeID(render.UncontainedID, fixture.ServerHostID) - uncontainedServerNode = pseudo(uncontainedServerID, render.OutgoingInternetID).WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.NonContainerNodeID], - RenderedProcesses[fixture.NonContainerProcessNodeID], + uncontainedServerNode = pseudo(uncontainedServerID, render.OutgoingInternetID).WithChildren(report.MakeIDList( + fixture.NonContainerNodeID, + fixture.NonContainerProcessNodeID, )) RenderedContainers = report.Nodes{ @@ -168,17 +158,17 @@ var ( docker.ContainerName: fixture.ClientContainerName, docker.ImageName: fixture.ClientContainerImageName, }). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, )), fixture.ServerContainerNodeID: container(fixture.ServerContainerNodeID). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - RenderedProcesses[fixture.ServerProcessNodeID], + WithChildren(report.MakeIDList( + fixture.Server80NodeID, + fixture.ServerProcessNodeID, )), fixture.ServerContainer2NodeID: container(fixture.ServerContainer2NodeID), @@ -194,12 +184,12 @@ var ( docker.ContainerHostname: fixture.ClientContainerHostname, }). AddCounter(report.Container, 1). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], - RenderedContainers[fixture.ClientContainerNodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, + fixture.ClientContainerNodeID, )), fixture.ServerContainerHostname: containerHostnameNode(fixture.ServerContainerHostname). @@ -207,11 +197,11 @@ var ( docker.ContainerHostname: fixture.ServerContainerHostname, }). AddCounter(report.Container, 2). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - RenderedProcesses[fixture.ServerProcessNodeID], - RenderedContainers[fixture.ServerContainerNodeID], - RenderedContainers[fixture.ServerContainer2NodeID], + WithChildren(report.MakeIDList( + fixture.Server80NodeID, + fixture.ServerProcessNodeID, + fixture.ServerContainerNodeID, + fixture.ServerContainer2NodeID, )), uncontainedServerID: uncontainedServerNode, @@ -230,21 +220,21 @@ var ( docker.ImageName: fixture.ClientContainerImageName, }). AddCounter(report.Container, 1). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], - RenderedContainers[fixture.ClientContainerNodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, + fixture.ClientContainerNodeID, )), ServerContainerImageNodeID: containerImage(ServerContainerImageNodeID). AddCounter(report.Container, 2). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - RenderedProcesses[fixture.ServerProcessNodeID], - RenderedContainers[fixture.ServerContainerNodeID], - RenderedContainers[fixture.ServerContainer2NodeID], + WithChildren(report.MakeIDList( + fixture.Server80NodeID, + fixture.ServerProcessNodeID, + fixture.ServerContainerNodeID, + fixture.ServerContainer2NodeID, )), uncontainedServerID: uncontainedServerNode, @@ -253,11 +243,11 @@ var ( } UnmanagedServerID = render.MakePseudoNodeID(render.UnmanagedID, fixture.ServerHostID) - unmanagedServerNode = pseudo(UnmanagedServerID, render.OutgoingInternetID).WithChildren(report.MakeNodeSet( - uncontainedServerNode, - RenderedEndpoints[fixture.NonContainerNodeID], - RenderedProcesses[fixture.NonContainerProcessNodeID], - RenderedContainers[fixture.ServerContainer2NodeID], + unmanagedServerNode = pseudo(UnmanagedServerID, render.OutgoingInternetID).WithChildren(report.MakeIDList( + uncontainedServerID, + fixture.NonContainerNodeID, + fixture.NonContainerProcessNodeID, + fixture.ServerContainer2NodeID, )). AddCounter(report.Container, 1). AddCounter(render.Pseudo, 1) @@ -265,20 +255,20 @@ var ( RenderedPods = report.Nodes{ fixture.ClientPodNodeID: pod(fixture.ClientPodNodeID, fixture.ServerPodNodeID). AddCounter(report.Container, 1). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], - RenderedContainers[fixture.ClientContainerNodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, + fixture.ClientContainerNodeID, )), fixture.ServerPodNodeID: pod(fixture.ServerPodNodeID). AddCounter(report.Container, 1). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - RenderedProcesses[fixture.ServerProcessNodeID], - RenderedContainers[fixture.ServerContainerNodeID], + WithChildren(report.MakeIDList( + fixture.Server80NodeID, + fixture.ServerProcessNodeID, + fixture.ServerContainerNodeID, )), fixture.PersistentVolumeClaimNodeID: persistentVolumeClaim(fixture.PersistentVolumeClaimNodeID, fixture.PersistentVolumeNodeID). @@ -289,7 +279,7 @@ var ( kubernetes.VolumeName: "pongvolume", kubernetes.AccessModes: "ReadWriteOnce", kubernetes.StorageClassName: "standard", - }).WithChild(report.MakeNode(fixture.PersistentVolumeNodeID).WithTopology(report.PersistentVolume)), + }).WithChildID(fixture.PersistentVolumeNodeID), fixture.PersistentVolumeNodeID: persistentVolume(fixture.PersistentVolumeNodeID, fixture.VolumeSnapshotNodeID). WithLatests(map[string]string{ @@ -300,13 +290,13 @@ var ( kubernetes.AccessModes: "ReadWriteOnce", kubernetes.StorageClassName: "standard", kubernetes.StorageDriver: "iSCSI", - }).WithChild(report.MakeNode(fixture.VolumeSnapshotNodeID).WithTopology(report.VolumeSnapshot)), + }).WithChildID(fixture.VolumeSnapshotNodeID), fixture.StorageClassNodeID: StorageClass(fixture.StorageClassNodeID, fixture.PersistentVolumeClaimNodeID). WithLatests(map[string]string{ kubernetes.Name: "standard", kubernetes.Provisioner: "pong", - }).WithChild(report.MakeNode(fixture.PersistentVolumeClaimNodeID).WithTopology(report.PersistentVolumeClaim)), + }).WithChildID(fixture.PersistentVolumeClaimNodeID), fixture.VolumeSnapshotNodeID: volumeSnapshot(fixture.VolumeSnapshotNodeID, fixture.VolumeSnapshotDataNodeID). WithLatests(map[string]string{ @@ -315,7 +305,7 @@ var ( kubernetes.VolumeClaim: "pvc-6124", kubernetes.SnapshotData: "vsd-1234", kubernetes.VolumeName: "pongvolume", - }).WithChild(report.MakeNode(fixture.VolumeSnapshotDataNodeID).WithTopology(report.VolumeSnapshotData)), + }).WithChildID(fixture.VolumeSnapshotDataNodeID), fixture.VolumeSnapshotDataNodeID: volumeSnapshotData(fixture.VolumeSnapshotDataNodeID). WithLatests(map[string]string{ @@ -332,17 +322,17 @@ var ( RenderedPodServices = report.Nodes{ fixture.ServiceNodeID: service(fixture.ServiceNodeID, fixture.ServiceNodeID). AddCounter(report.Pod, 2). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedEndpoints[fixture.Server80NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], - RenderedProcesses[fixture.ServerProcessNodeID], - RenderedContainers[fixture.ClientContainerNodeID], - RenderedContainers[fixture.ServerContainerNodeID], - RenderedPods[fixture.ClientPodNodeID], - RenderedPods[fixture.ServerPodNodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.Server80NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, + fixture.ServerProcessNodeID, + fixture.ClientContainerNodeID, + fixture.ServerContainerNodeID, + fixture.ClientPodNodeID, + fixture.ServerPodNodeID, )), UnmanagedServerID: unmanagedServerNode, @@ -359,14 +349,14 @@ var ( AddCounter(report.ContainerImage, 1). AddCounter(report.Pod, 1). AddCounter(report.Process, 2). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Client54001NodeID], - RenderedEndpoints[fixture.Client54002NodeID], - RenderedProcesses[fixture.ClientProcess1NodeID], - RenderedProcesses[fixture.ClientProcess2NodeID], - RenderedContainers[fixture.ClientContainerNodeID], - RenderedContainerImages[ClientContainerImageNodeID], - RenderedPods[fixture.ClientPodNodeID], + WithChildren(report.MakeIDList( + fixture.Client54001NodeID, + fixture.Client54002NodeID, + fixture.ClientProcess1NodeID, + fixture.ClientProcess2NodeID, + fixture.ClientContainerNodeID, + ClientContainerImageNodeID, + fixture.ClientPodNodeID, )), fixture.ServerHostNodeID: hostNode(fixture.ServerHostNodeID, render.OutgoingInternetID). @@ -374,15 +364,15 @@ var ( AddCounter(report.ContainerImage, 1). AddCounter(report.Pod, 1). AddCounter(report.Process, 2). - WithChildren(report.MakeNodeSet( - RenderedEndpoints[fixture.Server80NodeID], - RenderedEndpoints[fixture.NonContainerNodeID], - RenderedProcesses[fixture.ServerProcessNodeID], - RenderedProcesses[fixture.NonContainerProcessNodeID], - RenderedContainers[fixture.ServerContainerNodeID], - RenderedContainers[fixture.ServerContainer2NodeID], - RenderedContainerImages[ServerContainerImageNodeID], - RenderedPods[fixture.ServerPodNodeID], + WithChildren(report.MakeIDList( + fixture.Server80NodeID, + fixture.NonContainerNodeID, + fixture.ServerProcessNodeID, + fixture.NonContainerProcessNodeID, + fixture.ServerContainerNodeID, + fixture.ServerContainer2NodeID, + ServerContainerImageNodeID, + fixture.ServerPodNodeID, )), // due to https://github.com/weaveworks/scope/issues/1323 we are dropping diff --git a/render/id.go b/render/id.go index 72ca6feb9..68c358069 100644 --- a/render/id.go +++ b/render/id.go @@ -52,7 +52,7 @@ func ParseGroupNodeTopology(topology string) (string, string, bool) { // NewDerivedNode makes a node based on node, but with a new ID func NewDerivedNode(id string, node report.Node) report.Node { - return report.MakeNode(id).WithChildren(node.Children.Add(node)) + return report.MakeNode(id).WithChildren(node.ChildIDs.Add(node.ID)) } // NewDerivedPseudoNode makes a new pseudo node with the node as a child diff --git a/render/metrics.go b/render/metrics.go index 2658cdd44..2f8a46633 100644 --- a/render/metrics.go +++ b/render/metrics.go @@ -22,11 +22,13 @@ type propagateSingleMetrics struct { func (p propagateSingleMetrics) Render(ctx context.Context, rpt report.Report) Nodes { nodes := p.r.Render(ctx, rpt) outputs := make(report.Nodes, len(nodes.Nodes)) + topology, _ := rpt.Topology(p.topology) for id, n := range nodes.Nodes { var first report.Node found := 0 - n.Children.ForEach(func(child report.Node) { - if child.Topology == p.topology { + for _, childID := range n.ChildIDs { + if ty, ok := report.NodeIDType(childID); ok && ty == p.topology { + child := topology.Nodes[childID] if _, ok := child.Latest.Lookup(report.DoesNotMakeConnections); !ok { if found == 0 { first = child @@ -34,7 +36,7 @@ func (p propagateSingleMetrics) Render(ctx context.Context, rpt report.Report) N found++ } } - }) + } if found == 1 { n = n.WithMetrics(first.Metrics) } diff --git a/render/metrics_test.go b/render/metrics_test.go index 5e6f74e31..9bb32ea81 100644 --- a/render/metrics_test.go +++ b/render/metrics_test.go @@ -13,6 +13,36 @@ import ( func TestPropagateSingleMetrics(t *testing.T) { now := time.Now() + empty := report.MakeNode("empty") + child1 := report.MakeNode(report.MakeContainerNodeID("child1")). + WithTopology(report.Container). + WithMetrics(report.Metrics{ + "metric1": report.MakeMetric(nil), + }) + child2 := report.MakeNode(report.MakeContainerNodeID("child2")). + WithTopology(report.Container). + WithMetrics(report.Metrics{ + "metric2": report.MakeMetric(nil), + }) + child2p := report.MakeNode(report.MakeProcessNodeID("host1", "child2")). + WithTopology("Process"). + WithMetrics(report.Metrics{ + "metric2": report.MakeMetric(nil), + }) + pause := report.MakeNode(report.MakeContainerNodeID("pause")). + WithLatest(report.DoesNotMakeConnections, now, ""). + WithTopology(report.Container). + WithMetrics(report.Metrics{ + "metric2": report.MakeMetric(nil), + }) + a := report.MakeNode("a").WithChildID(child1.ID) + rpt := report.MakeReport() + rpt.Container.AddNode(empty) + rpt.Container.AddNode(a) + rpt.Container.AddNode(child1) + rpt.Container.AddNode(child2) + rpt.Container.AddNode(pause) + rpt.Process.AddNode(child2p) for _, c := range []struct { name string input report.Node @@ -21,146 +51,50 @@ func TestPropagateSingleMetrics(t *testing.T) { }{ { name: "empty", - input: report.MakeNode("empty"), + input: empty, topology: "", output: report.Nodes{"empty": report.MakeNode("empty")}, }, { - name: "one child", - input: report.MakeNode("a").WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - ), - ), + name: "one child", + input: a, topology: report.Container, output: report.Nodes{ "a": report.MakeNode("a").WithMetrics(report.Metrics{ "metric1": report.MakeMetric(nil), - }).WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - ), - ), + }).WithChildID(child1.ID), }, }, { - name: "ignores other topologies", - input: report.MakeNode("a").WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - report.MakeNode("child2"). - WithTopology("otherTopology"). - WithMetrics(report.Metrics{ - "metric2": report.MakeMetric(nil), - }), - ), - ), + name: "ignores other topologies", + input: report.MakeNode("a").WithChildren(report.MakeIDList(child1.ID, child2p.ID)), topology: report.Container, output: report.Nodes{ "a": report.MakeNode("a").WithMetrics(report.Metrics{ "metric1": report.MakeMetric(nil), - }).WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - report.MakeNode("child2"). - WithTopology("otherTopology"). - WithMetrics(report.Metrics{ - "metric2": report.MakeMetric(nil), - }), - ), - ), + }).WithChildren(report.MakeIDList(child1.ID, child2p.ID)), }, }, { - name: "two children", - input: report.MakeNode("a").WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - report.MakeNode("child2"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric2": report.MakeMetric(nil), - }), - ), - ), + name: "two children", + input: report.MakeNode("a").WithChildren(report.MakeIDList(child1.ID, child2.ID)), topology: report.Container, output: report.Nodes{ - "a": report.MakeNode("a").WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - report.MakeNode("child2"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric2": report.MakeMetric(nil), - }), - ), - ), + "a": report.MakeNode("a").WithChildren(report.MakeIDList(child1.ID, child2.ID)), }, }, { - name: "ignores k8s pause container", - input: report.MakeNode("a").WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - report.MakeNode("child2"). - WithLatest(report.DoesNotMakeConnections, now, ""). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric2": report.MakeMetric(nil), - }), - ), - ), + name: "ignores k8s pause container", + input: report.MakeNode("a").WithChildren(report.MakeIDList(child1.ID, pause.ID)), topology: report.Container, output: report.Nodes{ "a": report.MakeNode("a").WithMetrics(report.Metrics{ "metric1": report.MakeMetric(nil), - }).WithChildren( - report.MakeNodeSet( - report.MakeNode("child1"). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric1": report.MakeMetric(nil), - }), - report.MakeNode("child2"). - WithLatest(report.DoesNotMakeConnections, now, ""). - WithTopology(report.Container). - WithMetrics(report.Metrics{ - "metric2": report.MakeMetric(nil), - }), - ), - ), + }).WithChildren(report.MakeIDList(child1.ID, pause.ID)), }, }, } { - got := render.PropagateSingleMetrics(c.topology, mockRenderer{report.Nodes{c.input.ID: c.input}}).Render(context.Background(), report.Report{}).Nodes + got := render.PropagateSingleMetrics(c.topology, mockRenderer{report.Nodes{c.input.ID: c.input}}).Render(context.Background(), rpt).Nodes if !reflect.DeepEqual(got, c.output) { t.Errorf("[%s] Diff: %s", c.name, test.Diff(c.output, got)) } diff --git a/render/persistentvolume.go b/render/persistentvolume.go index 8c9b281f4..ebe947271 100644 --- a/render/persistentvolume.go +++ b/render/persistentvolume.go @@ -33,7 +33,7 @@ func (v volumesRenderer) Render(ctx context.Context, rpt report.Report) Nodes { volumeName, _ := p.Latest.Lookup(report.KubernetesName) if volume == volumeName { n.Adjacency = n.Adjacency.Add(p.ID) - n.Children = n.Children.Add(p) + n = n.WithChildID(p.ID) } } nodes[id] = n @@ -65,7 +65,7 @@ func (v podToVolumesRenderer) Render(ctx context.Context, rpt report.Report) Nod pvcNamespace, _ := pvcNode.Latest.Lookup(report.KubernetesNamespace) if (pvcName == ClaimName) && (podNamespace == pvcNamespace) { podNode.Adjacency = podNode.Adjacency.Add(pvcNode.ID) - podNode.Children = podNode.Children.Add(pvcNode) + podNode = podNode.WithChildID(pvcNode.ID) break } } @@ -91,7 +91,7 @@ func (v pvcToStorageClassRenderer) Render(ctx context.Context, rpt report.Report storageClassName, _ := pvcNode.Latest.Lookup(report.KubernetesStorageClassName) if storageClassName == storageClass { scNode.Adjacency = scNode.Adjacency.Add(pvcNode.ID) - scNode.Children = scNode.Children.Add(pvcNode) + scNode = scNode.WithChildID(pvcNode.ID) } } nodes[scID] = scNode @@ -114,7 +114,7 @@ func (v pvToSnapshotRenderer) Render(ctx context.Context, rpt report.Report) Nod snapshotPVName, _ := volumeSnapshotNode.Latest.Lookup(report.KubernetesVolumeName) if volumeName == snapshotPVName { p.Adjacency = p.Adjacency.Add(volumeSnapshotNode.ID) - p.Children = p.Children.Add(volumeSnapshotNode) + p = p.WithChildID(volumeSnapshotNode.ID) } } nodes[pvNodeID] = p @@ -138,7 +138,7 @@ func (v volumeSnapshotRenderer) Render(ctx context.Context, rpt report.Report) N snapshotDataName, _ := volumeSnapshotDataNode.Latest.Lookup(report.KubernetesName) if snapshotDataName == snapshotData { volumeSnapshotNode.Adjacency = volumeSnapshotNode.Adjacency.Add(volumeSnapshotDataNode.ID) - volumeSnapshotNode.Children = volumeSnapshotNode.Children.Add(volumeSnapshotDataNode) + volumeSnapshotNode = volumeSnapshotNode.WithChildID(volumeSnapshotDataNode.ID) } nodes[volumeSnapshotDataID] = volumeSnapshotDataNode } diff --git a/render/render.go b/render/render.go index 4327277de..38d6d684a 100644 --- a/render/render.go +++ b/render/render.go @@ -161,8 +161,7 @@ type joinResults struct { func newJoinResults(inputNodes report.Nodes) joinResults { nodes := make(report.Nodes, len(inputNodes)) for id, n := range inputNodes { - n.Adjacency = nil // result() assumes all nodes start with no adjacencies - n.Children = n.Children.Copy() // so we can do unsafe adds + n.Adjacency = nil // result() assumes all nodes start with no adjacencies nodes[id] = n } return joinResults{nodes: nodes, mapped: map[string]string{}, multi: map[string][]string{}} @@ -193,7 +192,7 @@ func (ret *joinResults) addUnmappedChild(m report.Node, id string, topology stri if !exists { result = report.MakeNode(id).WithTopology(topology) } - result.Children.UnsafeAdd(m) + result = result.WithChildID(m.ID) if m.Topology != report.Endpoint { // optimisation: we never look at endpoint counts result = result.AddCounter(m.Topology, 1) } @@ -212,7 +211,7 @@ func (ret *joinResults) addChild(m report.Node, id string, topology string) { func (ret *joinResults) addChildAndChildren(m report.Node, id string, topology string) { ret.addUnmappedChild(m, id, topology) result := ret.nodes[id] - result.Children.UnsafeMerge(m.Children) + result = result.WithChildren(m.ChildIDs) ret.nodes[id] = result ret.mapChild(m.ID, id) } @@ -221,7 +220,6 @@ func (ret *joinResults) addChildAndChildren(m report.Node, id string, topology s func (ret *joinResults) passThrough(n report.Node) { n.Adjacency = nil // result() assumes all nodes start with no adjacencies ret.nodes[n.ID] = n - n.Children = n.Children.Copy() // so we can do unsafe adds ret.mapChild(n.ID, n.ID) } diff --git a/report/id.go b/report/id.go index 37db28df1..436b02b9b 100644 --- a/report/id.go +++ b/report/id.go @@ -262,6 +262,19 @@ func split2(s, sep string) (s1, s2 string, ok bool) { return s[:pos], s[pos+1:], true } +// NodeIDType returns the type of a node ID - e.g. process, pod, endpoint +func NodeIDType(nodeID string) (string, bool) { + if _, tag, ok := ParseNodeID(nodeID); ok { + switch { + case len(tag) >= 2 && tag[0] == '<' && tag[len(tag)-1] == '>': + return tag[1 : len(tag)-1], true + case len(tag) >= 1 && tag[0] >= '0' && tag[0] <= '9': + return Endpoint, true + } + } + return "", false +} + // ParseNodeID produces the id and tag of a single-component node ID. func ParseNodeID(nodeID string) (id string, tag string, ok bool) { return split2(nodeID, ScopeDelim) diff --git a/report/id_test.go b/report/id_test.go index f6306e5c1..bbe0fb1ef 100644 --- a/report/id_test.go +++ b/report/id_test.go @@ -3,6 +3,7 @@ package report_test import ( "testing" + "github.com/stretchr/testify/assert" "github.com/weaveworks/scope/report" ) @@ -78,3 +79,18 @@ func TestECSServiceNodeIDCompat(t *testing.T) { t.Errorf("Backwards-compatible id %q parsed name to %q, expected %q", testID, name, testName) } } + +func TestNodeIDType(t *testing.T) { + ty, ok := report.NodeIDType("") + assert.False(t, ok) + ty, ok = report.NodeIDType(clientHostNodeID) + assert.True(t, ok) + assert.Equal(t, report.Host, ty) + ty, ok = report.NodeIDType(client54001EndpointNodeID) + assert.True(t, ok) + assert.Equal(t, report.Endpoint, ty) + rsetID := report.MakeReplicaSetNodeID("foo") + ty, ok = report.NodeIDType(rsetID) + assert.True(t, ok) + assert.Equal(t, report.ReplicaSet, ty) +} diff --git a/report/node.go b/report/node.go index 859398be0..5dd52d7b0 100644 --- a/report/node.go +++ b/report/node.go @@ -19,7 +19,7 @@ type Node struct { Latest StringLatestMap `json:"latest,omitempty"` Metrics Metrics `json:"metrics,omitempty" deepequal:"nil==empty"` Parents Sets `json:"parents,omitempty"` - Children NodeSet `json:"children,omitempty"` + ChildIDs IDList `json:"cs,omitempty"` } // MakeNode creates a new Node with no initial metadata. @@ -162,14 +162,16 @@ func (n Node) PruneParents() Node { } // WithChildren returns a fresh copy of n, with children merged in. -func (n Node) WithChildren(children NodeSet) Node { - n.Children = n.Children.Merge(children) +func (n Node) WithChildren(children IDList) Node { + for _, childID := range children { + n.ChildIDs = n.ChildIDs.Add(childID) + } return n } -// WithChild returns a fresh copy of n, with one child merged in. -func (n Node) WithChild(child Node) Node { - n.Children = n.Children.Merge(MakeNodeSet(child)) +// WithChildID returns a fresh copy of n, with one child merged in. +func (n Node) WithChildID(childID string) Node { + n.ChildIDs = n.ChildIDs.Add(childID) return n } @@ -194,7 +196,7 @@ func (n Node) Merge(other Node) Node { Latest: n.Latest.Merge(other.Latest), Metrics: n.Metrics.Merge(other.Metrics), Parents: n.Parents.Merge(other.Parents), - Children: n.Children.Merge(other.Children), + ChildIDs: n.ChildIDs.Merge(other.ChildIDs), } } diff --git a/test/utils/prune.go b/test/utils/prune.go index 69860e913..6be1dbdcd 100644 --- a/test/utils/prune.go +++ b/test/utils/prune.go @@ -21,15 +21,11 @@ func Prune(nodes report.Nodes) report.Nodes { // necessary for rendering nodes and edges stripped away. Specifically, that // means cutting out parts of the Node. func PruneNode(node report.Node) report.Node { - prunedChildren := report.MakeNodeSet() - node.Children.ForEach(func(child report.Node) { - prunedChildren = prunedChildren.Add(PruneNode(child)) - }) prunedNode := report.MakeNode( node.ID). WithTopology(node.Topology). WithAdjacent(node.Adjacency...). - WithChildren(prunedChildren) + WithChildren(node.ChildIDs) // Copy counters across, but with zero timestamp so they compare equal node.Latest.ForEach(func(k string, _ time.Time, v string) { if strings.HasPrefix(k, report.CounterPrefix) {