From cb9d558665d13999c901385d15ff5041a3e89639 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Mon, 18 Jan 2016 11:51:06 +0000 Subject: [PATCH] Review feedback refactoring --- render/detailed/node.go | 39 ++++---- render/detailed/node_test.go | 172 ++++++++++++++++++----------------- 2 files changed, 105 insertions(+), 106 deletions(-) diff --git a/render/detailed/node.go b/render/detailed/node.go index c57a66977..5d9836ef1 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -14,13 +14,10 @@ import ( // Node is the data type that's yielded to the JavaScript layer when // we want deep information about an individual node. type Node struct { - ID string `json:"id"` - Label string `json:"label"` + NodeSummary Rank string `json:"rank,omitempty"` Pseudo bool `json:"pseudo,omitempty"` Controls []ControlInstance `json:"controls"` - Metadata []MetadataRow `json:"metadata,omitempty"` - Metrics []MetricRow `json:"metrics,omitempty"` Children []NodeSummaryGroup `json:"children,omitempty"` Parents []Parent `json:"parents,omitempty"` } @@ -43,16 +40,16 @@ type ControlInstance struct { // MakeNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. func MakeNode(r report.Report, n render.RenderableNode) Node { + summary, _ := MakeNodeSummary(n.Node) + summary.ID = n.ID + summary.Label = n.LabelMajor return Node{ - ID: n.ID, - Label: n.LabelMajor, - Rank: n.Rank, - Pseudo: n.Pseudo, - Controls: controls(r, n), - Metadata: NodeMetadata(n.Node), - Metrics: NodeMetrics(n.Node), - Children: children(n), - Parents: parents(r, n), + NodeSummary: summary, + Rank: n.Rank, + Pseudo: n.Pseudo, + Controls: controls(r, n), + Children: children(n), + Parents: parents(r, n), } } @@ -125,17 +122,9 @@ func children(n render.RenderableNode) []NodeSummaryGroup { return nodeSummaryGroups } -// parents is a total a hack to find the parents of a node (which is -// ill-defined). +// parents renders the parents of this report.Node, which have been aggregated +// from the probe reports. func parents(r report.Report, n render.RenderableNode) (result []Parent) { - defer func() { - for i, parent := range result { - if parent.ID == n.ID { - result = append(result[:i], result[i+1:]...) - } - } - }() - topologies := map[string]struct { report.Topology render func(report.Node) Parent @@ -154,6 +143,10 @@ func parents(r report.Report, n render.RenderableNode) (result []Parent) { for _, topologyID := range topologyIDs { t := topologies[topologyID] for _, id := range n.Node.Parents[topologyID] { + if topologyID == n.Node.Topology && id == n.ID { + continue + } + parent, ok := t.Nodes[id] if !ok { continue diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 2cd49bf68..127817fcb 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -25,65 +25,68 @@ func TestMakeDetailedHostNode(t *testing.T) { process2NodeSummary, _ := detailed.MakeNodeSummary(fixture.Report.Process.Nodes[fixture.ClientProcess2NodeID]) process2NodeSummary.Linkable = true want := detailed.Node{ - ID: render.MakeHostID(fixture.ClientHostID), - Label: "client", + NodeSummary: detailed.NodeSummary{ + ID: render.MakeHostID(fixture.ClientHostID), + Label: "client", + Linkable: true, + Metadata: []detailed.MetadataRow{ + { + ID: "host_name", + Label: "Hostname", + Value: "client.hostname.com", + }, + { + ID: "os", + Label: "Operating system", + Value: "Linux", + }, + { + ID: "local_networks", + Label: "Local Networks", + Value: "10.10.10.0/24", + }, + }, + Metrics: []detailed.MetricRow{ + { + ID: host.CPUUsage, + Format: "percent", + Label: "CPU", + Value: 0.01, + Metric: &fixture.CPUMetric, + }, + { + ID: host.MemUsage, + Format: "filesize", + Label: "Memory", + Value: 0.01, + Metric: &fixture.MemoryMetric, + }, + { + ID: host.Load1, + Group: "load", + Label: "Load (1m)", + Value: 0.01, + Metric: &fixture.LoadMetric, + }, + { + ID: host.Load5, + Group: "load", + Label: "Load (5m)", + Value: 0.01, + Metric: &fixture.LoadMetric, + }, + { + ID: host.Load15, + Label: "Load (15m)", + Group: "load", + Value: 0.01, + Metric: &fixture.LoadMetric, + }, + }, + }, Rank: "hostname.com", Pseudo: false, Controls: []detailed.ControlInstance{}, - Metadata: []detailed.MetadataRow{ - { - ID: "host_name", - Label: "Hostname", - Value: "client.hostname.com", - }, - { - ID: "os", - Label: "Operating system", - Value: "Linux", - }, - { - ID: "local_networks", - Label: "Local Networks", - Value: "10.10.10.0/24", - }, - }, - Metrics: []detailed.MetricRow{ - { - ID: host.CPUUsage, - Format: "percent", - Label: "CPU", - Value: 0.01, - Metric: &fixture.CPUMetric, - }, - { - ID: host.MemUsage, - Format: "filesize", - Label: "Memory", - Value: 0.01, - Metric: &fixture.MemoryMetric, - }, - { - ID: host.Load1, - Group: "load", - Label: "Load (1m)", - Value: 0.01, - Metric: &fixture.LoadMetric, - }, - { - ID: host.Load5, - Group: "load", - Label: "Load (5m)", - Value: 0.01, - Metric: &fixture.LoadMetric, - }, - { - ID: host.Load15, - Label: "Load (15m)", - Group: "load", - Value: 0.01, - Metric: &fixture.LoadMetric, - }, - }, Children: []detailed.NodeSummaryGroup{ { Label: "Container Images", @@ -118,36 +121,39 @@ func TestMakeDetailedContainerNode(t *testing.T) { } have := detailed.MakeNode(fixture.Report, renderableNode) want := detailed.Node{ - ID: id, - Label: "server", + NodeSummary: detailed.NodeSummary{ + ID: id, + Label: "server", + Linkable: true, + Metadata: []detailed.MetadataRow{ + {ID: "docker_container_id", Label: "ID", Value: fixture.ServerContainerID}, + {ID: "docker_image_id", Label: "Image ID", Value: fixture.ServerContainerImageID}, + {ID: "docker_container_state", Label: "State", Value: "running"}, + {ID: "label_" + render.AmazonECSContainerNameLabel, Label: fmt.Sprintf(`Label %q`, render.AmazonECSContainerNameLabel), Value: `server`}, + {ID: "label_foo1", Label: `Label "foo1"`, Value: `bar1`}, + {ID: "label_foo2", Label: `Label "foo2"`, Value: `bar2`}, + {ID: "label_io.kubernetes.pod.name", Label: `Label "io.kubernetes.pod.name"`, Value: "ping/pong-b"}, + }, + Metrics: []detailed.MetricRow{ + { + ID: docker.CPUTotalUsage, + Format: "percent", + Label: "CPU", + Value: 0.01, + Metric: &fixture.CPUMetric, + }, + { + ID: docker.MemoryUsage, + Format: "filesize", + Label: "Memory", + Value: 0.01, + Metric: &fixture.MemoryMetric, + }, + }, + }, Rank: "imageid456", Pseudo: false, Controls: []detailed.ControlInstance{}, - Metadata: []detailed.MetadataRow{ - {ID: "docker_container_id", Label: "ID", Value: fixture.ServerContainerID}, - {ID: "docker_image_id", Label: "Image ID", Value: fixture.ServerContainerImageID}, - {ID: "docker_container_state", Label: "State", Value: "running"}, - {ID: "label_" + render.AmazonECSContainerNameLabel, Label: fmt.Sprintf(`Label %q`, render.AmazonECSContainerNameLabel), Value: `server`}, - {ID: "label_foo1", Label: `Label "foo1"`, Value: `bar1`}, - {ID: "label_foo2", Label: `Label "foo2"`, Value: `bar2`}, - {ID: "label_io.kubernetes.pod.name", Label: `Label "io.kubernetes.pod.name"`, Value: "ping/pong-b"}, - }, - Metrics: []detailed.MetricRow{ - { - ID: docker.CPUTotalUsage, - Format: "percent", - Label: "CPU", - Value: 0.01, - Metric: &fixture.CPUMetric, - }, - { - ID: docker.MemoryUsage, - Format: "filesize", - Label: "Memory", - Value: 0.01, - Metric: &fixture.MemoryMetric, - }, - }, Children: []detailed.NodeSummaryGroup{ { Label: "Applications",