From d7da27a69209e0578c6c02dd3f0924f7ea1b8038 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 27 May 2015 11:22:12 +0200 Subject: [PATCH 1/4] Function has slowly become dead code; remove it --- report/topology.go | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/report/topology.go b/report/topology.go index d4215bef1e..f461ab2348 100644 --- a/report/topology.go +++ b/report/topology.go @@ -1,7 +1,6 @@ package report import ( - "net" "reflect" "strings" ) @@ -118,7 +117,7 @@ func (t Topology) RenderBy(f MapFunc, grouped bool) map[string]RenderableNode { var maj, min string if dstNodeAddress == TheInternet { dstRenderableID = dstNodeAddress - maj, min = formatLabel(dstNodeAddress) + maj, min = "the Internet", "" } else if grouped { dstRenderableID = localUnknown maj, min = "", "" @@ -190,26 +189,6 @@ func (t Topology) EdgeMetadata(f MapFunc, grouped bool, srcRenderableID, dstRend return metadata } -// formatLabel is an opportunistic helper to format any addressID into -// something we can show on screen. -func formatLabel(s string) (major, minor string) { - if s == TheInternet { - return "the Internet", "" - } - - // Format is either "scope;ip;port", "scope;ip", or some process id. - parts := strings.SplitN(s, ScopeDelim, 3) - if len(parts) < 2 { - return s, "" - } - - if len(parts) == 2 { - return parts[1], "" - } - - return net.JoinHostPort(parts[1], parts[2]), "" -} - // Diff is returned by TopoDiff. It represents the changes between two // RenderableNode maps. type Diff struct { From ea6d1282dc5b9175820ec061e34549ebe880a3ef Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 27 May 2015 08:31:11 +0000 Subject: [PATCH 2/4] Group pseudo nodes by the node the referred to and their address in grouped topologies. --- report/topology.go | 6 ++-- report/topology_test.go | 63 +++++++++++++++++++---------------------- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/report/topology.go b/report/topology.go index f461ab2348..48649fc36d 100644 --- a/report/topology.go +++ b/report/topology.go @@ -119,8 +119,10 @@ func (t Topology) RenderBy(f MapFunc, grouped bool) map[string]RenderableNode { dstRenderableID = dstNodeAddress maj, min = "the Internet", "" } else if grouped { - dstRenderableID = localUnknown - maj, min = "", "" + // When grouping, emit one pseudo node per (srcNodeAddress, dstNodeAddr) + dstNodeAddr, _ := trySplitAddr(dstNodeAddress) + dstRenderableID = strings.Join([]string{"pseudo:", dstNodeAddr, srcRenderableID}, ScopeDelim) + maj, min = dstNodeAddr, "" } else { // Rule for non-internet psuedo nodes; emit 1 new node for each // dstNodeAddr, srcNodeAddr, srcNodePort. diff --git a/report/topology_test.go b/report/topology_test.go index 260b8b46bc..27e872fe67 100644 --- a/report/topology_test.go +++ b/report/topology_test.go @@ -183,26 +183,16 @@ func TestRenderByProcessPID(t *testing.T) { }, }, "pseudo:;10.10.10.10;192.168.1.1;80": { - ID: "pseudo:;10.10.10.10;192.168.1.1;80", - LabelMajor: "10.10.10.10", - LabelMinor: "", - Rank: "", - Pseudo: true, - Adjacency: nil, - OriginHosts: nil, - OriginNodes: nil, - Metadata: AggregateMetadata{}, + ID: "pseudo:;10.10.10.10;192.168.1.1;80", + LabelMajor: "10.10.10.10", + Pseudo: true, + Metadata: AggregateMetadata{}, }, "pseudo:;10.10.10.11;192.168.1.1;80": { - ID: "pseudo:;10.10.10.11;192.168.1.1;80", - LabelMajor: "10.10.10.11", - LabelMinor: "", - Rank: "", - Pseudo: true, - Adjacency: nil, - OriginHosts: nil, - OriginNodes: nil, - Metadata: AggregateMetadata{}, + ID: "pseudo:;10.10.10.11;192.168.1.1;80", + LabelMajor: "10.10.10.11", + Pseudo: true, + Metadata: AggregateMetadata{}, }, } have := report.Process.RenderBy(ProcessPID, false) @@ -231,12 +221,16 @@ func TestRenderByProcessPIDGrouped(t *testing.T) { }, }, "apache": { - ID: "apache", - LabelMajor: "apache", - LabelMinor: "", - Rank: "215", - Pseudo: false, - Adjacency: NewIDList("curl", "localUnknown"), + ID: "apache", + LabelMajor: "apache", + LabelMinor: "", + Rank: "215", + Pseudo: false, + Adjacency: NewIDList( + "curl", + "pseudo:;10.10.10.10;apache", + "pseudo:;10.10.10.11;apache", + ), OriginHosts: NewIDList("server.hostname.com"), OriginNodes: NewIDList(";192.168.1.1;80"), Metadata: AggregateMetadata{ @@ -244,16 +238,17 @@ func TestRenderByProcessPIDGrouped(t *testing.T) { KeyBytesEgress: 1500, }, }, - "localUnknown": { - ID: "localUnknown", - LabelMajor: "", - LabelMinor: "", - Rank: "", - Pseudo: true, - Adjacency: nil, - OriginHosts: nil, - OriginNodes: nil, - Metadata: AggregateMetadata{}, + "pseudo:;10.10.10.10;apache": { + ID: "pseudo:;10.10.10.10;apache", + LabelMajor: "10.10.10.10", + Pseudo: true, + Metadata: AggregateMetadata{}, + }, + "pseudo:;10.10.10.11;apache": { + ID: "pseudo:;10.10.10.11;apache", + LabelMajor: "10.10.10.11", + Pseudo: true, + Metadata: AggregateMetadata{}, }, } have := report.Process.RenderBy(ProcessPID, true) From 0fd5c2eeae0887918996cc58bbf64ea87a00b457 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 27 May 2015 09:17:43 +0000 Subject: [PATCH 3/4] Factor out pseudo node creation to a per-topology function - no semantic change right now. --- app/api_topologies.go | 2 +- app/api_topology.go | 10 ++++---- app/router.go | 9 ++++--- report/mapping_functions.go | 45 ++++++++++++++++++++++++++++++++++ report/topology.go | 48 ++++++++++--------------------------- report/topology_test.go | 6 ++--- 6 files changed, 73 insertions(+), 47 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 5bceb6ec29..347dd1dba2 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -42,7 +42,7 @@ func makeTopologyList(rep Reporter) func(w http.ResponseWriter, r *http.Request) Name: def.human, URL: url, GroupedURL: groupedURL, - Stats: stats(def.topologySelecter(rpt).RenderBy(def.MapFunc, false)), + Stats: stats(def.topologySelecter(rpt).RenderBy(def.MapFunc, def.PseudoFunc, false)), }) } respondWith(w, http.StatusOK, a) diff --git a/app/api_topology.go b/app/api_topology.go index 653862de11..75ae6a7a1d 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -46,6 +46,7 @@ func makeTopologyHandlers( rep Reporter, topo topologySelecter, mapping report.MapFunc, + pseudo report.PseudoFunc, grouped bool, get *mux.Router, base string, @@ -53,7 +54,7 @@ func makeTopologyHandlers( // Full topology. get.HandleFunc(base, func(w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: topo(rep.Report()).RenderBy(mapping, grouped), + Nodes: topo(rep.Report()).RenderBy(mapping, pseudo, grouped), }) }) @@ -71,7 +72,7 @@ func makeTopologyHandlers( return } } - handleWebsocket(w, r, rep, topo, mapping, grouped, loop) + handleWebsocket(w, r, rep, topo, mapping, pseudo, grouped, loop) }) // Individual nodes. @@ -80,7 +81,7 @@ func makeTopologyHandlers( vars = mux.Vars(r) nodeID = vars["id"] rpt = rep.Report() - node, ok = topo(rpt).RenderBy(mapping, grouped)[nodeID] + node, ok = topo(rpt).RenderBy(mapping, pseudo, grouped)[nodeID] ) if !ok { http.NotFound(w, r) @@ -114,6 +115,7 @@ func handleWebsocket( rep Reporter, topo topologySelecter, mapping report.MapFunc, + psuedo report.PseudoFunc, grouped bool, loop time.Duration, ) { @@ -139,7 +141,7 @@ func handleWebsocket( tick = time.Tick(loop) ) for { - newTopo := topo(rep.Report()).RenderBy(mapping, grouped) + newTopo := topo(rep.Report()).RenderBy(mapping, psuedo, grouped) diff := report.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/router.go b/app/router.go index 0b9c51a2a5..030af850ed 100644 --- a/app/router.go +++ b/app/router.go @@ -19,6 +19,7 @@ func Router(c Reporter) *mux.Router { c, def.topologySelecter, def.MapFunc, + def.PseudoFunc, false, // not grouped get, "/api/topology/"+name, @@ -28,6 +29,7 @@ func Router(c Reporter) *mux.Router { c, def.topologySelecter, def.MapFunc, + def.PseudoFunc, true, // grouped get, "/api/topology/"+name+"grouped", @@ -44,9 +46,10 @@ var topologyRegistry = map[string]struct { human string topologySelecter report.MapFunc + report.PseudoFunc hasGrouped bool }{ - "applications": {"Applications", selectProcess, report.ProcessPID, true}, - "containers": {"Containers", selectProcess, report.ProcessContainer, true}, - "hosts": {"Hosts", selectNetwork, report.NetworkHostname, false}, + "applications": {"Applications", selectProcess, report.ProcessPID, report.GenericPseudoNode, true}, + "containers": {"Containers", selectProcess, report.ProcessContainer, report.GenericPseudoNode, true}, + "hosts": {"Hosts", selectNetwork, report.NetworkHostname, report.GenericPseudoNode, false}, } diff --git a/report/mapping_functions.go b/report/mapping_functions.go index 1b20108f5d..93825d7b5c 100644 --- a/report/mapping_functions.go +++ b/report/mapping_functions.go @@ -25,6 +25,12 @@ type MappedNode struct { // rendered topology. type MapFunc func(string, NodeMetadata, bool) (MappedNode, bool) +// PseudoFunc creates MappedNode representing pseudo nodes given the dstNodeID. +// The srcNode renderable node is essentially from MapFunc, representing one of +// the rendered nodes this pseudo node refers to. srcNodeID and dstNodeID are +// node IDs prior to mapping. +type PseudoFunc func(srcNodeID string, srcNode RenderableNode, dstNodeID string, grouped bool) (MappedNode, bool) + // ProcessPID takes a node NodeMetadata from a Process topology, and returns a // representation with the ID based on the process PID and the labels based // on the process name. @@ -100,3 +106,42 @@ func NetworkHostname(_ string, m NodeMetadata, _ bool) (MappedNode, bool) { Rank: parts[0], }, name != "" } + +// GenericPseudoNode contains heuristics for building sensible pseudo nodes. +// It should go away. +func GenericPseudoNode(src string, srcMapped RenderableNode, dst string, grouped bool) (MappedNode, bool) { + var maj, min, outputID string + + if dst == TheInternet { + outputID = dst + maj, min = "the Internet", "" + } else if grouped { + // When grouping, emit one pseudo node per (srcNodeAddress, dstNodeAddr) + dstNodeAddr, _ := trySplitAddr(dst) + + outputID = strings.Join([]string{"pseudo:", dstNodeAddr, srcMapped.ID}, ScopeDelim) + maj, min = dstNodeAddr, "" + } else { + // Rule for non-internet psuedo nodes; emit 1 new node for each + // dstNodeAddr, srcNodeAddr, srcNodePort. + srcNodeAddr, srcNodePort := trySplitAddr(src) + dstNodeAddr, _ := trySplitAddr(dst) + + outputID = strings.Join([]string{"pseudo:", dstNodeAddr, srcNodeAddr, srcNodePort}, ScopeDelim) + maj, min = dstNodeAddr, "" + } + + return MappedNode{ + ID: outputID, + Major: maj, + Minor: min, + }, true +} + +func trySplitAddr(addr string) (string, string) { + fields := strings.SplitN(addr, ScopeDelim, 3) + if len(fields) == 3 { + return fields[1], fields[2] + } + return fields[1], "" +} diff --git a/report/topology.go b/report/topology.go index 48649fc36d..2419306bb2 100644 --- a/report/topology.go +++ b/report/topology.go @@ -69,7 +69,7 @@ func NewTopology() Topology { // // RenderBy takes a a MapFunc, which defines how to group and label nodes. If // grouped is true, nodes that belong to the same "class" will be merged. -func (t Topology) RenderBy(f MapFunc, grouped bool) map[string]RenderableNode { +func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc, grouped bool) map[string]RenderableNode { nodes := map[string]RenderableNode{} // Build a set of RenderableNodes for all non-pseudo probes, and an @@ -77,7 +77,7 @@ func (t Topology) RenderBy(f MapFunc, grouped bool) map[string]RenderableNode { // RenderableNodes. address2mapped := map[string]string{} for addressID, metadata := range t.NodeMetadatas { - mapped, ok := f(addressID, metadata, grouped) + mapped, ok := mapFunc(addressID, metadata, grouped) if !ok { continue } @@ -112,31 +112,15 @@ func (t Topology) RenderBy(f MapFunc, grouped bool) map[string]RenderableNode { for _, dstNodeAddress := range dsts { dstRenderableID, ok := address2mapped[dstNodeAddress] if !ok { - // We don't have a node for this target address. So we'll make - // a pseudonode for it, instead. - var maj, min string - if dstNodeAddress == TheInternet { - dstRenderableID = dstNodeAddress - maj, min = "the Internet", "" - } else if grouped { - // When grouping, emit one pseudo node per (srcNodeAddress, dstNodeAddr) - dstNodeAddr, _ := trySplitAddr(dstNodeAddress) - dstRenderableID = strings.Join([]string{"pseudo:", dstNodeAddr, srcRenderableID}, ScopeDelim) - maj, min = dstNodeAddr, "" - } else { - // Rule for non-internet psuedo nodes; emit 1 new node for each - // dstNodeAddr, srcNodeAddr, srcNodePort. - srcNodeAddr, srcNodePort := trySplitAddr(srcNodeAddress) - dstNodeAddr, _ := trySplitAddr(dstNodeAddress) - - // We don't care about dst remote port, just src local. - dstRenderableID = strings.Join([]string{"pseudo:", dstNodeAddr, srcNodeAddr, srcNodePort}, ScopeDelim) - maj, min = dstNodeAddr, "" + pseudoNode, ok := pseudoFunc(srcNodeAddress, srcRenderableNode, dstNodeAddress, grouped) + if !ok { + continue } + dstRenderableID = pseudoNode.ID nodes[dstRenderableID] = RenderableNode{ - ID: dstRenderableID, - LabelMajor: maj, - LabelMinor: min, + ID: pseudoNode.ID, + LabelMajor: pseudoNode.Major, + LabelMinor: pseudoNode.Minor, Pseudo: true, Metadata: AggregateMetadata{}, // populated below - or not? } @@ -158,30 +142,22 @@ func (t Topology) RenderBy(f MapFunc, grouped bool) map[string]RenderableNode { return nodes } -func trySplitAddr(addr string) (string, string) { - fields := strings.SplitN(addr, ScopeDelim, 3) - if len(fields) == 3 { - return fields[1], fields[2] - } - return fields[1], "" -} - // EdgeMetadata gives the metadata of an edge from the perspective of the // srcRenderableID. Since an edgeID can have multiple edges on the address // level, it uses the supplied mapping function to translate address IDs to // renderable node (mapped) IDs. -func (t Topology) EdgeMetadata(f MapFunc, grouped bool, srcRenderableID, dstRenderableID string) EdgeMetadata { +func (t Topology) EdgeMetadata(mapFunc MapFunc, grouped bool, srcRenderableID, dstRenderableID string) EdgeMetadata { metadata := EdgeMetadata{} for edgeID, edgeMeta := range t.EdgeMetadatas { edgeParts := strings.SplitN(edgeID, IDDelim, 2) src := edgeParts[0] if src != TheInternet { - mapped, _ := f(src, t.NodeMetadatas[src], grouped) + mapped, _ := mapFunc(src, t.NodeMetadatas[src], grouped) src = mapped.ID } dst := edgeParts[1] if dst != TheInternet { - mapped, _ := f(dst, t.NodeMetadatas[dst], grouped) + mapped, _ := mapFunc(dst, t.NodeMetadatas[dst], grouped) dst = mapped.ID } if src == srcRenderableID && dst == dstRenderableID { diff --git a/report/topology_test.go b/report/topology_test.go index 27e872fe67..d0eb8bfb76 100644 --- a/report/topology_test.go +++ b/report/topology_test.go @@ -195,7 +195,7 @@ func TestRenderByProcessPID(t *testing.T) { Metadata: AggregateMetadata{}, }, } - have := report.Process.RenderBy(ProcessPID, false) + have := report.Process.RenderBy(ProcessPID, GenericPseudoNode, false) if !reflect.DeepEqual(want, have) { t.Error("\n" + diff(want, have)) } @@ -251,7 +251,7 @@ func TestRenderByProcessPIDGrouped(t *testing.T) { Metadata: AggregateMetadata{}, }, } - have := report.Process.RenderBy(ProcessPID, true) + have := report.Process.RenderBy(ProcessPID, GenericPseudoNode, true) if !reflect.DeepEqual(want, have) { t.Error("\n" + diff(want, have)) } @@ -310,7 +310,7 @@ func TestRenderByNetworkHostname(t *testing.T) { Metadata: AggregateMetadata{}, }, } - have := report.Network.RenderBy(NetworkHostname, false) + have := report.Network.RenderBy(NetworkHostname, GenericPseudoNode, false) if !reflect.DeepEqual(want, have) { t.Error("\n" + diff(want, have)) } From 0b786ae5a7547d2398fd5967d226f0f52b09120f Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Wed, 27 May 2015 09:32:32 +0000 Subject: [PATCH 4/4] Don't produce pseudo nodes for the container topologies --- app/router.go | 2 +- report/mapping_functions.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/router.go b/app/router.go index 030af850ed..1da3f9764b 100644 --- a/app/router.go +++ b/app/router.go @@ -50,6 +50,6 @@ var topologyRegistry = map[string]struct { hasGrouped bool }{ "applications": {"Applications", selectProcess, report.ProcessPID, report.GenericPseudoNode, true}, - "containers": {"Containers", selectProcess, report.ProcessContainer, report.GenericPseudoNode, true}, + "containers": {"Containers", selectProcess, report.ProcessContainer, report.NoPseudoNode, true}, "hosts": {"Hosts", selectNetwork, report.NetworkHostname, report.GenericPseudoNode, false}, } diff --git a/report/mapping_functions.go b/report/mapping_functions.go index 93825d7b5c..007c8bd891 100644 --- a/report/mapping_functions.go +++ b/report/mapping_functions.go @@ -138,6 +138,11 @@ func GenericPseudoNode(src string, srcMapped RenderableNode, dst string, grouped }, true } +// NoPseudoNode never creates a pseudo node. +func NoPseudoNode(string, RenderableNode, string, bool) (MappedNode, bool) { + return MappedNode{}, false +} + func trySplitAddr(addr string) (string, string) { fields := strings.SplitN(addr, ScopeDelim, 3) if len(fields) == 3 {