From 10199737e449ea95002cab0ca8222f1c66fe3964 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 21 Aug 2015 15:19:26 +0000 Subject: [PATCH 1/7] Add host context in the details pane In situations when the details can refer to entities from multiple hosts (e.g. container image) make explicit to what host we are referring to. --- app/api_topology.go | 2 +- app/router.go | 21 ++++++++++-------- render/detailed_node.go | 22 ++++++++++-------- render/detailed_node_test.go | 43 ++++++++++++++++++++++++++++++++---- 4 files changed, 65 insertions(+), 23 deletions(-) diff --git a/app/api_topology.go b/app/api_topology.go index 204ddaa65..198d1b162 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -68,7 +68,7 @@ func handleNode(rep xfer.Reporter, t topologyView, w http.ResponseWriter, r *htt http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: render.MakeDetailedNode(rpt, node)}) + respondWith(w, http.StatusOK, APINode{Node: render.MakeDetailedNode(rpt, node, t.isMultiHost)}) } // Individual edges. diff --git a/app/router.go b/app/router.go index 570f223f6..7eabfc8f7 100644 --- a/app/router.go +++ b/app/router.go @@ -107,9 +107,10 @@ var topologyRegistry = map[string]topologyView{ renderer: render.FilterUnconnected{Renderer: render.ProcessWithContainerNameRenderer{}}, }, "applications-by-name": { - human: "by name", - parent: "applications", - renderer: render.FilterUnconnected{Renderer: render.ProcessNameRenderer}, + human: "by name", + parent: "applications", + renderer: render.FilterUnconnected{Renderer: render.ProcessNameRenderer}, + isMultiHost: true, }, "containers": { human: "Containers", @@ -117,9 +118,10 @@ var topologyRegistry = map[string]topologyView{ renderer: render.ContainerRenderer, }, "containers-by-image": { - human: "by image", - parent: "containers", - renderer: render.ContainerImageRenderer, + human: "by image", + parent: "containers", + renderer: render.ContainerImageRenderer, + isMultiHost: true, }, "hosts": { human: "Hosts", @@ -129,7 +131,8 @@ var topologyRegistry = map[string]topologyView{ } type topologyView struct { - human string - parent string - renderer render.Renderer + human string + parent string + renderer render.Renderer + isMultiHost bool // does the view involve information from multiple hosts? } diff --git a/render/detailed_node.go b/render/detailed_node.go index 2d7e9f34b..abb097c24 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -72,7 +72,7 @@ func (t tables) Less(i, j int) bool { return t[i].Rank > t[j].Rank } // MakeDetailedNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. -func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { +func MakeDetailedNode(r report.Report, n RenderableNode, addHostTags bool) DetailedNode { tables := tables{} // RenderableNode may be the result of merge operation(s), and so may have // multiple origins. The ultimate goal here is to generate tables to view @@ -80,7 +80,7 @@ func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { // add them later. connections := []Row{} for _, id := range n.Origins { - if table, ok := OriginTable(r, id); ok { + if table, ok := OriginTable(r, id, addHostTags); ok { tables = append(tables, table) } else if _, ok := r.Endpoint.NodeMetadatas[id]; ok { connections = append(connections, connectionDetailsRows(r.Endpoint, id)...) @@ -155,12 +155,12 @@ func addConnectionsTable(tables *tables, connections []Row, r report.Report, n R // OriginTable produces a table (to be consumed directly by the UI) based on // an origin ID, which is (optimistically) a node ID in one of our topologies. -func OriginTable(r report.Report, originID string) (Table, bool) { +func OriginTable(r report.Report, originID string, addHostTags bool) (Table, bool) { if nmd, ok := r.Process.NodeMetadatas[originID]; ok { - return processOriginTable(nmd) + return processOriginTable(nmd, addHostTags) } if nmd, ok := r.Container.NodeMetadatas[originID]; ok { - return containerOriginTable(nmd) + return containerOriginTable(nmd, addHostTags) } if nmd, ok := r.ContainerImage.NodeMetadatas[originID]; ok { return containerImageOriginTable(nmd) @@ -224,7 +224,7 @@ func connectionDetailsRows(topology report.Topology, originID string) []Row { return rows } -func processOriginTable(nmd report.NodeMetadata) (Table, bool) { +func processOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ {process.Comm, "Name"}, @@ -237,7 +237,9 @@ func processOriginTable(nmd report.NodeMetadata) (Table, bool) { rows = append(rows, Row{Key: tuple.human, ValueMajor: val, ValueMinor: ""}) } } - + if addHostTag { + rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) + } return Table{ Title: "Origin Process", Numeric: false, @@ -246,7 +248,7 @@ func processOriginTable(nmd report.NodeMetadata) (Table, bool) { }, len(rows) > 0 } -func containerOriginTable(nmd report.NodeMetadata) (Table, bool) { +func containerOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ {docker.ContainerID, "ID"}, @@ -268,7 +270,9 @@ func containerOriginTable(nmd report.NodeMetadata) (Table, bool) { rows = append(rows, Row{Key: "Memory Usage (MB):", ValueMajor: memoryStr, ValueMinor: ""}) } } - + if addHostTag { + rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) + } return Table{ Title: "Origin Container", Numeric: false, diff --git a/render/detailed_node_test.go b/render/detailed_node_test.go index 11742d62e..4cba8359a 100644 --- a/render/detailed_node_test.go +++ b/render/detailed_node_test.go @@ -10,7 +10,7 @@ import ( ) func TestOriginTable(t *testing.T) { - if _, ok := render.OriginTable(test.Report, "not-found"); ok { + if _, ok := render.OriginTable(test.Report, "not-found", false); ok { t.Errorf("unknown origin ID gave unexpected success") } for originID, want := range map[string]render.Table{ @@ -34,7 +34,7 @@ func TestOriginTable(t *testing.T) { }, }, } { - have, ok := render.OriginTable(test.Report, originID) + have, ok := render.OriginTable(test.Report, originID, false) if !ok { t.Errorf("%q: not OK", originID) continue @@ -43,11 +43,46 @@ func TestOriginTable(t *testing.T) { t.Errorf("%q: %s", originID, test.Diff(want, have)) } } + + // Test host tags + for originID, want := range map[string]render.Table{ + test.ServerProcessNodeID: { + Title: "Origin Process", + Numeric: false, + Rank: 2, + Rows: []render.Row{ + {"Host", test.ServerHostID, "", false}, + {"Name", "apache", "", false}, + {"PID", test.ServerPID, "", false}, + }, + }, + test.ServerContainerNodeID: { + Title: "Origin Container", + Numeric: false, + Rank: 3, + Rows: []render.Row{ + {"Host", test.ServerHostID, "", false}, + {"ID", test.ServerContainerID, "", false}, + {"Name", "server", "", false}, + {"Image ID", test.ServerContainerImageID, "", false}, + }, + }, + } { + have, ok := render.OriginTable(test.Report, originID, true) + if !ok { + t.Errorf("%q: not OK", originID) + continue + } + if !reflect.DeepEqual(want, have) { + t.Errorf("%q: %s", originID, test.Diff(want, have)) + } + } + } func TestMakeDetailedHostNode(t *testing.T) { renderableNode := render.HostRenderer.Render(test.Report)[render.MakeHostID(test.ClientHostID)] - have := render.MakeDetailedNode(test.Report, renderableNode) + have := render.MakeDetailedNode(test.Report, renderableNode, false) want := render.DetailedNode{ ID: render.MakeHostID(test.ClientHostID), LabelMajor: "client", @@ -109,7 +144,7 @@ func TestMakeDetailedHostNode(t *testing.T) { func TestMakeDetailedContainerNode(t *testing.T) { renderableNode := render.ContainerRenderer.Render(test.Report)[test.ServerContainerID] - have := render.MakeDetailedNode(test.Report, renderableNode) + have := render.MakeDetailedNode(test.Report, renderableNode, false) want := render.DetailedNode{ ID: test.ServerContainerID, LabelMajor: "server", From a01a7069b6c297e9425edf3972585a562bd44074 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 21 Aug 2015 15:30:23 +0000 Subject: [PATCH 2/7] Make tables and addContainerTable consistent with the rest of the code --- render/detailed_node.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/render/detailed_node.go b/render/detailed_node.go index abb097c24..81e518c22 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -64,16 +64,16 @@ func (r sortableRows) Less(i, j int) bool { } } -type tables []Table +type sortableTables []Table -func (t tables) Len() int { return len(t) } -func (t tables) Swap(i, j int) { t[i], t[j] = t[j], t[i] } -func (t tables) Less(i, j int) bool { return t[i].Rank > t[j].Rank } +func (t sortableTables) Len() int { return len(t) } +func (t sortableTables) Swap(i, j int) { t[i], t[j] = t[j], t[i] } +func (t sortableTables) Less(i, j int) bool { return t[i].Rank > t[j].Rank } // MakeDetailedNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. func MakeDetailedNode(r report.Report, n RenderableNode, addHostTags bool) DetailedNode { - tables := tables{} + tables := sortableTables{} // RenderableNode may be the result of merge operation(s), and so may have // multiple origins. The ultimate goal here is to generate tables to view // in the UI, so we skip the intermediate representations, but we could @@ -89,7 +89,9 @@ func MakeDetailedNode(r report.Report, n RenderableNode, addHostTags bool) Detai } } - addConnectionsTable(&tables, connections, r, n) + if table, ok := connectionsTable(connections, r, n); ok { + tables = append(tables, table) + } // Sort tables by rank sort.Sort(tables) @@ -103,7 +105,7 @@ func MakeDetailedNode(r report.Report, n RenderableNode, addHostTags bool) Detai } } -func addConnectionsTable(tables *tables, connections []Row, r report.Report, n RenderableNode) { +func connectionsTable(connections []Row, r report.Report, n RenderableNode) (Table, bool) { sec := r.Window.Seconds() rate := func(u *uint64) (float64, bool) { if u == nil { @@ -149,8 +151,9 @@ func addConnectionsTable(tables *tables, connections []Row, r report.Report, n R rows = append(rows, connections...) } if len(rows) > 0 { - *tables = append(*tables, Table{"Connections", true, connectionsRank, rows}) + return Table{"Connections", true, connectionsRank, rows}, true } + return Table{}, false } // OriginTable produces a table (to be consumed directly by the UI) based on From 75595a5519c4bc2aa0d7085f992d88bf5f3b80be Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Fri, 21 Aug 2015 16:03:45 +0000 Subject: [PATCH 3/7] Add useful information to table titles in details pane Title modifications: * "Origin Process" -> "Process \"name\" (pid)" * "Origin Container" -> "Container \"name\" (pid)" * "Origin Host" -> "Host \"name\"" * "Origin Container Image" -> "Container Image \"name\"" --- render/detailed_node.go | 59 ++++++++++++++++++++++++++++-------- render/detailed_node_test.go | 46 +++++++++------------------- 2 files changed, 60 insertions(+), 45 deletions(-) diff --git a/render/detailed_node.go b/render/detailed_node.go index 81e518c22..8b6d6aa5b 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -230,8 +230,6 @@ func connectionDetailsRows(topology report.Topology, originID string) []Row { func processOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ - {process.Comm, "Name"}, - {process.PID, "PID"}, {process.PPID, "Parent PID"}, {process.Cmdline, "Command"}, {process.Threads, "# Threads"}, @@ -243,19 +241,30 @@ func processOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) if addHostTag { rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) } + + title := "Process" + var ( + commFound, pidFound bool + name, pid string + ) + if name, commFound = nmd.Metadata[process.Comm]; commFound { + title += ` "` + name + `"` + } + if pid, pidFound = nmd.Metadata[process.PID]; pidFound { + title += " (" + pid + ")" + } return Table{ - Title: "Origin Process", + Title: title, Numeric: false, Rows: rows, Rank: processRank, - }, len(rows) > 0 + }, len(rows) > 0 || commFound || pidFound } func containerOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ {docker.ContainerID, "ID"}, - {docker.ContainerName, "Name"}, {docker.ImageID, "Image ID"}, {docker.ContainerPorts, "Ports"}, {docker.ContainerCreated, "Created"}, @@ -276,36 +285,52 @@ func containerOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool if addHostTag { rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) } + + title := "Container" + var ( + name string + nameFound bool + ) + if name, nameFound = nmd.Metadata[docker.ContainerName]; nameFound { + title += ` "` + name + `"` + } + return Table{ - Title: "Origin Container", + Title: title, Numeric: false, Rows: rows, Rank: containerRank, - }, len(rows) > 0 + }, len(rows) > 0 || nameFound } func containerImageOriginTable(nmd report.NodeMetadata) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ {docker.ImageID, "Image ID"}, - {docker.ImageName, "Image name"}, } { if val, ok := nmd.Metadata[tuple.key]; ok { rows = append(rows, Row{Key: tuple.human, ValueMajor: val, ValueMinor: ""}) } } + title := "Container Image" + var ( + nameFound bool + name string + ) + if name, nameFound = nmd.Metadata[docker.ImageName]; nameFound { + title += ` "` + name + `"` + } return Table{ - Title: "Origin Container Image", + Title: title, Numeric: false, Rows: rows, Rank: containerImageRank, - }, len(rows) > 0 + }, len(rows) > 0 || nameFound } func hostOriginTable(nmd report.NodeMetadata) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ - {host.HostName, "Host name"}, {host.Load, "Load"}, {host.OS, "Operating system"}, {host.KernelVersion, "Kernel version"}, @@ -316,10 +341,18 @@ func hostOriginTable(nmd report.NodeMetadata) (Table, bool) { } } + title := "Host" + var ( + name string + foundName bool + ) + if name, foundName = nmd.Metadata[host.HostName]; foundName { + title += ` "` + name + `"` + } return Table{ - Title: "Origin Host", + Title: title, Numeric: false, Rows: rows, Rank: hostRank, - }, len(rows) > 0 + }, len(rows) > 0 || foundName } diff --git a/render/detailed_node_test.go b/render/detailed_node_test.go index 4cba8359a..1f4e6de02 100644 --- a/render/detailed_node_test.go +++ b/render/detailed_node_test.go @@ -13,22 +13,17 @@ func TestOriginTable(t *testing.T) { if _, ok := render.OriginTable(test.Report, "not-found", false); ok { t.Errorf("unknown origin ID gave unexpected success") } - for originID, want := range map[string]render.Table{ - test.ServerProcessNodeID: { - Title: "Origin Process", - Numeric: false, - Rank: 2, - Rows: []render.Row{ - {"Name", "apache", "", false}, - {"PID", test.ServerPID, "", false}, - }, - }, + for originID, want := range map[string]render.Table{test.ServerProcessNodeID: { + Title: fmt.Sprintf(`Process "apache" (%s)`, test.ServerPID), + Numeric: false, + Rank: 2, + Rows: []render.Row{}, + }, test.ServerHostNodeID: { - Title: "Origin Host", + Title: fmt.Sprintf("Host %q", test.ServerHostName), Numeric: false, Rank: 1, Rows: []render.Row{ - {"Host name", test.ServerHostName, "", false}, {"Load", "0.01 0.01 0.01", "", false}, {"Operating system", "Linux", "", false}, }, @@ -47,23 +42,20 @@ func TestOriginTable(t *testing.T) { // Test host tags for originID, want := range map[string]render.Table{ test.ServerProcessNodeID: { - Title: "Origin Process", + Title: fmt.Sprintf(`Process "apache" (%s)`, test.ServerPID), Numeric: false, Rank: 2, Rows: []render.Row{ {"Host", test.ServerHostID, "", false}, - {"Name", "apache", "", false}, - {"PID", test.ServerPID, "", false}, }, }, test.ServerContainerNodeID: { - Title: "Origin Container", + Title: `Container "server"`, Numeric: false, Rank: 3, Rows: []render.Row{ {"Host", test.ServerHostID, "", false}, {"ID", test.ServerContainerID, "", false}, - {"Name", "server", "", false}, {"Image ID", test.ServerContainerImageID, "", false}, }, }, @@ -90,15 +82,10 @@ func TestMakeDetailedHostNode(t *testing.T) { Pseudo: false, Tables: []render.Table{ { - Title: "Origin Host", + Title: fmt.Sprintf("Host %q", test.ClientHostName), Numeric: false, Rank: 1, Rows: []render.Row{ - { - Key: "Host name", - ValueMajor: "client.hostname.com", - ValueMinor: "", - }, { Key: "Load", ValueMajor: "0.01 0.01 0.01", @@ -152,30 +139,25 @@ func TestMakeDetailedContainerNode(t *testing.T) { Pseudo: false, Tables: []render.Table{ { - Title: "Origin Container", + Title: `Container "server"`, Numeric: false, Rank: 3, Rows: []render.Row{ {"ID", test.ServerContainerID, "", false}, - {"Name", "server", "", false}, {"Image ID", test.ServerContainerImageID, "", false}, }, }, { - Title: "Origin Process", + Title: fmt.Sprintf(`Process "apache" (%s)`, test.ServerPID), Numeric: false, Rank: 2, - Rows: []render.Row{ - {"Name", "apache", "", false}, - {"PID", test.ServerPID, "", false}, - }, + Rows: []render.Row{}, }, { - Title: "Origin Host", + Title: fmt.Sprintf("Host %q", test.ServerHostName), Numeric: false, Rank: 1, Rows: []render.Row{ - {"Host name", test.ServerHostName, "", false}, {"Load", "0.01 0.01 0.01", "", false}, {"Operating system", "Linux", "", false}, }, From 807b30bfe3bbe3489f28f02bf80773e985212fc1 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Mon, 24 Aug 2015 18:40:20 +0000 Subject: [PATCH 4/7] Figure out whether to show host tags in details pane from context --- app/api_topology.go | 2 +- app/router.go | 21 +++++++++------------ render/detailed_node.go | 17 +++++++++++++++-- render/detailed_node_test.go | 4 ++-- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/app/api_topology.go b/app/api_topology.go index 198d1b162..204ddaa65 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -68,7 +68,7 @@ func handleNode(rep xfer.Reporter, t topologyView, w http.ResponseWriter, r *htt http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: render.MakeDetailedNode(rpt, node, t.isMultiHost)}) + respondWith(w, http.StatusOK, APINode{Node: render.MakeDetailedNode(rpt, node)}) } // Individual edges. diff --git a/app/router.go b/app/router.go index 7eabfc8f7..570f223f6 100644 --- a/app/router.go +++ b/app/router.go @@ -107,10 +107,9 @@ var topologyRegistry = map[string]topologyView{ renderer: render.FilterUnconnected{Renderer: render.ProcessWithContainerNameRenderer{}}, }, "applications-by-name": { - human: "by name", - parent: "applications", - renderer: render.FilterUnconnected{Renderer: render.ProcessNameRenderer}, - isMultiHost: true, + human: "by name", + parent: "applications", + renderer: render.FilterUnconnected{Renderer: render.ProcessNameRenderer}, }, "containers": { human: "Containers", @@ -118,10 +117,9 @@ var topologyRegistry = map[string]topologyView{ renderer: render.ContainerRenderer, }, "containers-by-image": { - human: "by image", - parent: "containers", - renderer: render.ContainerImageRenderer, - isMultiHost: true, + human: "by image", + parent: "containers", + renderer: render.ContainerImageRenderer, }, "hosts": { human: "Hosts", @@ -131,8 +129,7 @@ var topologyRegistry = map[string]topologyView{ } type topologyView struct { - human string - parent string - renderer render.Renderer - isMultiHost bool // does the view involve information from multiple hosts? + human string + parent string + renderer render.Renderer } diff --git a/render/detailed_node.go b/render/detailed_node.go index 8b6d6aa5b..3fd23ed94 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -72,15 +72,28 @@ func (t sortableTables) Less(i, j int) bool { return t[i].Rank > t[j].Rank } // MakeDetailedNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. -func MakeDetailedNode(r report.Report, n RenderableNode, addHostTags bool) DetailedNode { +func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { tables := sortableTables{} + + // Figure out if multiple hosts are referenced by the renderableNode + originHosts := make(map[string]struct{}) + for _, id := range n.Origins { + for _, topology := range r.Topologies() { + if nmd, ok := topology.NodeMetadatas[id]; ok { + originHosts[report.ExtractHostID(nmd)] = struct{}{} + break + } + } + } + multiHost := len(originHosts) > 1 + // RenderableNode may be the result of merge operation(s), and so may have // multiple origins. The ultimate goal here is to generate tables to view // in the UI, so we skip the intermediate representations, but we could // add them later. connections := []Row{} for _, id := range n.Origins { - if table, ok := OriginTable(r, id, addHostTags); ok { + if table, ok := OriginTable(r, id, multiHost); ok { tables = append(tables, table) } else if _, ok := r.Endpoint.NodeMetadatas[id]; ok { connections = append(connections, connectionDetailsRows(r.Endpoint, id)...) diff --git a/render/detailed_node_test.go b/render/detailed_node_test.go index 1f4e6de02..796a997d3 100644 --- a/render/detailed_node_test.go +++ b/render/detailed_node_test.go @@ -74,7 +74,7 @@ func TestOriginTable(t *testing.T) { func TestMakeDetailedHostNode(t *testing.T) { renderableNode := render.HostRenderer.Render(test.Report)[render.MakeHostID(test.ClientHostID)] - have := render.MakeDetailedNode(test.Report, renderableNode, false) + have := render.MakeDetailedNode(test.Report, renderableNode) want := render.DetailedNode{ ID: render.MakeHostID(test.ClientHostID), LabelMajor: "client", @@ -131,7 +131,7 @@ func TestMakeDetailedHostNode(t *testing.T) { func TestMakeDetailedContainerNode(t *testing.T) { renderableNode := render.ContainerRenderer.Render(test.Report)[test.ServerContainerID] - have := render.MakeDetailedNode(test.Report, renderableNode, false) + have := render.MakeDetailedNode(test.Report, renderableNode) want := render.DetailedNode{ ID: test.ServerContainerID, LabelMajor: "server", From 117b0e190b4d9146d52450ab3b02a662cb4706b4 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Mon, 24 Aug 2015 18:51:12 +0000 Subject: [PATCH 5/7] Address PR comment --- render/detailed_node.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/render/detailed_node.go b/render/detailed_node.go index 3fd23ed94..1cd02c06d 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -255,15 +255,15 @@ func processOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) } - title := "Process" var ( - commFound, pidFound bool - name, pid string + title = "Process" + name, commFound = nmd.Metadata[process.Comm] + pid, pidFound = nmd.Metadata[process.PID] ) - if name, commFound = nmd.Metadata[process.Comm]; commFound { + if commFound { title += ` "` + name + `"` } - if pid, pidFound = nmd.Metadata[process.PID]; pidFound { + if pidFound { title += " (" + pid + ")" } return Table{ @@ -299,12 +299,11 @@ func containerOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) } - title := "Container" var ( - name string - nameFound bool + title = "Container" + name, nameFound = nmd.Metadata[docker.ContainerName] ) - if name, nameFound = nmd.Metadata[docker.ContainerName]; nameFound { + if nameFound { title += ` "` + name + `"` } From d1126f911b5e7e0bcd246269b3595371f0a9282c Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Mon, 24 Aug 2015 21:04:28 +0000 Subject: [PATCH 6/7] Desambiguate Container associated to processes in details pane --- render/detailed_node.go | 21 +++++++++++++++------ render/detailed_node_test.go | 9 +++++---- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/render/detailed_node.go b/render/detailed_node.go index 1cd02c06d..6c08a7575 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -75,17 +75,21 @@ func (t sortableTables) Less(i, j int) bool { return t[i].Rank > t[j].Rank } func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { tables := sortableTables{} - // Figure out if multiple hosts are referenced by the renderableNode + // Figure out if multiple hosts/containers are referenced by the renderableNode originHosts := make(map[string]struct{}) + originContainers := make(map[string]struct{}) for _, id := range n.Origins { for _, topology := range r.Topologies() { if nmd, ok := topology.NodeMetadatas[id]; ok { originHosts[report.ExtractHostID(nmd)] = struct{}{} - break + if id, ok := nmd.Metadata[docker.ContainerID]; ok { + originContainers[id] = struct{}{} + } } } } multiHost := len(originHosts) > 1 + multiContainer := len(originContainers) > 1 // RenderableNode may be the result of merge operation(s), and so may have // multiple origins. The ultimate goal here is to generate tables to view @@ -93,7 +97,7 @@ func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { // add them later. connections := []Row{} for _, id := range n.Origins { - if table, ok := OriginTable(r, id, multiHost); ok { + if table, ok := OriginTable(r, id, multiHost, multiContainer); ok { tables = append(tables, table) } else if _, ok := r.Endpoint.NodeMetadatas[id]; ok { connections = append(connections, connectionDetailsRows(r.Endpoint, id)...) @@ -171,9 +175,9 @@ func connectionsTable(connections []Row, r report.Report, n RenderableNode) (Tab // OriginTable produces a table (to be consumed directly by the UI) based on // an origin ID, which is (optimistically) a node ID in one of our topologies. -func OriginTable(r report.Report, originID string, addHostTags bool) (Table, bool) { +func OriginTable(r report.Report, originID string, addHostTags bool, addContainerTags bool) (Table, bool) { if nmd, ok := r.Process.NodeMetadatas[originID]; ok { - return processOriginTable(nmd, addHostTags) + return processOriginTable(nmd, addHostTags, addContainerTags) } if nmd, ok := r.Container.NodeMetadatas[originID]; ok { return containerOriginTable(nmd, addHostTags) @@ -240,7 +244,7 @@ func connectionDetailsRows(topology report.Topology, originID string) []Row { return rows } -func processOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) { +func processOriginTable(nmd report.NodeMetadata, addHostTag bool, addContainerTag bool) (Table, bool) { rows := []Row{} for _, tuple := range []struct{ key, human string }{ {process.PPID, "Parent PID"}, @@ -251,6 +255,11 @@ func processOriginTable(nmd report.NodeMetadata, addHostTag bool) (Table, bool) rows = append(rows, Row{Key: tuple.human, ValueMajor: val, ValueMinor: ""}) } } + + if containerID, ok := nmd.Metadata[docker.ContainerID]; ok && addContainerTag { + rows = append([]Row{{Key: "Container ID", ValueMajor: containerID}}, rows...) + } + if addHostTag { rows = append([]Row{{Key: "Host", ValueMajor: report.ExtractHostID(nmd)}}, rows...) } diff --git a/render/detailed_node_test.go b/render/detailed_node_test.go index 796a997d3..af34a06b7 100644 --- a/render/detailed_node_test.go +++ b/render/detailed_node_test.go @@ -10,7 +10,7 @@ import ( ) func TestOriginTable(t *testing.T) { - if _, ok := render.OriginTable(test.Report, "not-found", false); ok { + if _, ok := render.OriginTable(test.Report, "not-found", false, false); ok { t.Errorf("unknown origin ID gave unexpected success") } for originID, want := range map[string]render.Table{test.ServerProcessNodeID: { @@ -29,7 +29,7 @@ func TestOriginTable(t *testing.T) { }, }, } { - have, ok := render.OriginTable(test.Report, originID, false) + have, ok := render.OriginTable(test.Report, originID, false, false) if !ok { t.Errorf("%q: not OK", originID) continue @@ -39,7 +39,7 @@ func TestOriginTable(t *testing.T) { } } - // Test host tags + // Test host/container tags for originID, want := range map[string]render.Table{ test.ServerProcessNodeID: { Title: fmt.Sprintf(`Process "apache" (%s)`, test.ServerPID), @@ -47,6 +47,7 @@ func TestOriginTable(t *testing.T) { Rank: 2, Rows: []render.Row{ {"Host", test.ServerHostID, "", false}, + {"Container ID", test.ServerContainerID, "", false}, }, }, test.ServerContainerNodeID: { @@ -60,7 +61,7 @@ func TestOriginTable(t *testing.T) { }, }, } { - have, ok := render.OriginTable(test.Report, originID, true) + have, ok := render.OriginTable(test.Report, originID, true, true) if !ok { t.Errorf("%q: not OK", originID) continue From 11e5135d195e4c8448c91e358d99edcebe27eca7 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 25 Aug 2015 10:31:13 +0000 Subject: [PATCH 7/7] Address PR comment --- render/detailed_node.go | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/render/detailed_node.go b/render/detailed_node.go index 6c08a7575..66e42d135 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -76,20 +76,7 @@ func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { tables := sortableTables{} // Figure out if multiple hosts/containers are referenced by the renderableNode - originHosts := make(map[string]struct{}) - originContainers := make(map[string]struct{}) - for _, id := range n.Origins { - for _, topology := range r.Topologies() { - if nmd, ok := topology.NodeMetadatas[id]; ok { - originHosts[report.ExtractHostID(nmd)] = struct{}{} - if id, ok := nmd.Metadata[docker.ContainerID]; ok { - originContainers[id] = struct{}{} - } - } - } - } - multiHost := len(originHosts) > 1 - multiContainer := len(originContainers) > 1 + multiContainer, multiHost := getRenderingContext(r, n) // RenderableNode may be the result of merge operation(s), and so may have // multiple origins. The ultimate goal here is to generate tables to view @@ -122,6 +109,28 @@ func MakeDetailedNode(r report.Report, n RenderableNode) DetailedNode { } } +func getRenderingContext(r report.Report, n RenderableNode) (multiContainer bool, multiHost bool) { + originHosts := make(map[string]struct{}) + originContainers := make(map[string]struct{}) + for _, id := range n.Origins { + for _, topology := range r.Topologies() { + if nmd, ok := topology.NodeMetadatas[id]; ok { + originHosts[report.ExtractHostID(nmd)] = struct{}{} + if id, ok := nmd.Metadata[docker.ContainerID]; ok { + originContainers[id] = struct{}{} + } + } + // Return early if possible + multiHost = len(originHosts) > 1 + multiContainer = len(originContainers) > 1 + if multiHost && multiContainer { + return + } + } + } + return +} + func connectionsTable(connections []Row, r report.Report, n RenderableNode) (Table, bool) { sec := r.Window.Seconds() rate := func(u *uint64) (float64, bool) {