From 144a7986024a43af3148cdd32b39d0a4a5c768f7 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Tue, 1 Sep 2015 17:35:14 +0000 Subject: [PATCH] Review Feedback --- experimental/demoprobe/main.go | 4 +- experimental/genreport/generate.go | 4 +- probe/endpoint/reporter.go | 8 +-- probe/sniff/sniffer.go | 10 ++-- probe/sniff/sniffer_internal_test.go | 4 +- probe/sniff/sniffer_test.go | 4 +- render/render.go | 4 +- render/render_test.go | 4 +- report/topology.go | 90 +++++++++++++++++++--------- test/report_fixture.go | 14 ++--- 10 files changed, 91 insertions(+), 55 deletions(-) diff --git a/experimental/demoprobe/main.go b/experimental/demoprobe/main.go index 2e277edf5..f084d79c1 100644 --- a/experimental/demoprobe/main.go +++ b/experimental/demoprobe/main.go @@ -88,14 +88,14 @@ func demoReport(nodeCount int) report.Report { process.PID: "4000", "name": c.srcProc, "domain": "node-" + src, - }).WithEdgeMetadata(dstPortID, report.EdgeMetadata{ + }).WithEdge(dstPortID, report.EdgeMetadata{ MaxConnCountTCP: newu64(uint64(rand.Intn(100) + 10)), })) r.Endpoint = r.Endpoint.WithNode(dstPortID, report.MakeNodeMetadata().WithMetadata(map[string]string{ process.PID: "4000", "name": c.dstProc, "domain": "node-" + dst, - }).WithEdgeMetadata(srcPortID, report.EdgeMetadata{ + }).WithEdge(srcPortID, report.EdgeMetadata{ MaxConnCountTCP: newu64(uint64(rand.Intn(100) + 10)), })) diff --git a/experimental/genreport/generate.go b/experimental/genreport/generate.go index 0af8a084e..7df1ffd12 100644 --- a/experimental/genreport/generate.go +++ b/experimental/genreport/generate.go @@ -68,14 +68,14 @@ func DemoReport(nodeCount int) report.Report { "pid": "4000", "name": c.srcProc, "domain": "node-" + src, - }).WithEdgeMetadata(dstPortID, report.EdgeMetadata{ + }).WithEdge(dstPortID, report.EdgeMetadata{ MaxConnCountTCP: newu64(uint64(rand.Intn(100) + 10)), })) r.Endpoint = r.Endpoint.WithNode(dstPortID, report.MakeNodeMetadata().WithMetadata(map[string]string{ "pid": "4000", "name": c.dstProc, "domain": "node-" + dst, - }).WithEdgeMetadata(srcPortID, report.EdgeMetadata{ + }).WithEdge(srcPortID, report.EdgeMetadata{ MaxConnCountTCP: newu64(uint64(rand.Intn(100) + 10)), })) diff --git a/probe/endpoint/reporter.go b/probe/endpoint/reporter.go index 187f3879e..333c76566 100644 --- a/probe/endpoint/reporter.go +++ b/probe/endpoint/reporter.go @@ -147,11 +147,11 @@ func (r *Reporter) addConnection(rpt *report.Report, localAddr, remoteAddr strin if localIsClient { // New nodes are merged into the report so we don't need to do any counting here; the merge does it for us. - localNode = localNode.WithEdgeMetadata(remoteAddressNodeID, report.EdgeMetadata{ + localNode = localNode.WithEdge(remoteAddressNodeID, report.EdgeMetadata{ MaxConnCountTCP: newu64(1), }) } else { - remoteNode = localNode.WithEdgeMetadata(localAddressNodeID, report.EdgeMetadata{ + remoteNode = localNode.WithEdge(localAddressNodeID, report.EdgeMetadata{ MaxConnCountTCP: newu64(1), }) } @@ -179,11 +179,11 @@ func (r *Reporter) addConnection(rpt *report.Report, localAddr, remoteAddr strin if localIsClient { // New nodes are merged into the report so we don't need to do any counting here; the merge does it for us. - localNode = localNode.WithEdgeMetadata(remoteEndpointNodeID, report.EdgeMetadata{ + localNode = localNode.WithEdge(remoteEndpointNodeID, report.EdgeMetadata{ MaxConnCountTCP: newu64(1), }) } else { - remoteNode = remoteNode.WithEdgeMetadata(localEndpointNodeID, report.EdgeMetadata{ + remoteNode = remoteNode.WithEdge(localEndpointNodeID, report.EdgeMetadata{ MaxConnCountTCP: newu64(1), }) } diff --git a/probe/sniff/sniffer.go b/probe/sniff/sniffer.go index b105a9327..628c9c648 100644 --- a/probe/sniff/sniffer.go +++ b/probe/sniff/sniffer.go @@ -119,7 +119,7 @@ func interpolateCounts(r report.Report) { factor := 1.0 / rate for _, topology := range r.Topologies() { for _, nmd := range topology.NodeMetadatas { - for _, emd := range nmd.EdgeMetadatas { + for _, emd := range nmd.Edges { if emd.EgressPacketCount != nil { *emd.EgressPacketCount = uint64(float64(*emd.EgressPacketCount) * factor) } @@ -267,7 +267,7 @@ func (s *Sniffer) Merge(p Packet, rpt *report.Report) { rpt.Address = addAdjacency(rpt.Address, srcNodeID, dstNodeID) - emd := rpt.Address.NodeMetadatas[srcNodeID].EdgeMetadatas[dstNodeID] + emd := rpt.Address.NodeMetadatas[srcNodeID].Edges[dstNodeID] if egress { if emd.EgressPacketCount == nil { emd.EgressPacketCount = new(uint64) @@ -287,7 +287,7 @@ func (s *Sniffer) Merge(p Packet, rpt *report.Report) { } *emd.IngressByteCount += uint64(p.Network) } - rpt.Address.NodeMetadatas[srcNodeID].EdgeMetadatas[dstNodeID] = emd + rpt.Address.NodeMetadatas[srcNodeID].Edges[dstNodeID] = emd } // If we have ports, we can add to the endpoint topology, too. @@ -299,7 +299,7 @@ func (s *Sniffer) Merge(p Packet, rpt *report.Report) { rpt.Endpoint = addAdjacency(rpt.Endpoint, srcNodeID, dstNodeID) - emd := rpt.Endpoint.NodeMetadatas[srcNodeID].EdgeMetadatas[dstNodeID] + emd := rpt.Endpoint.NodeMetadatas[srcNodeID].Edges[dstNodeID] if egress { if emd.EgressPacketCount == nil { emd.EgressPacketCount = new(uint64) @@ -319,6 +319,6 @@ func (s *Sniffer) Merge(p Packet, rpt *report.Report) { } *emd.IngressByteCount += uint64(p.Transport) } - rpt.Endpoint.NodeMetadatas[srcNodeID].EdgeMetadatas[dstNodeID] = emd + rpt.Endpoint.NodeMetadatas[srcNodeID].Edges[dstNodeID] = emd } } diff --git a/probe/sniff/sniffer_internal_test.go b/probe/sniff/sniffer_internal_test.go index 1313f798b..512f5f0fc 100644 --- a/probe/sniff/sniffer_internal_test.go +++ b/probe/sniff/sniffer_internal_test.go @@ -20,7 +20,7 @@ func TestInterpolateCounts(t *testing.T) { r := report.MakeReport() r.Sampling.Count = samplingCount r.Sampling.Total = samplingTotal - r.Endpoint.NodeMetadatas[srcNodeID] = report.MakeNodeMetadata().WithEdgeMetadata(dstNodeID, report.EdgeMetadata{ + r.Endpoint.NodeMetadatas[srcNodeID] = report.MakeNodeMetadata().WithEdge(dstNodeID, report.EdgeMetadata{ EgressPacketCount: newu64(packetCount), IngressPacketCount: newu64(packetCount), EgressByteCount: newu64(byteCount), @@ -33,7 +33,7 @@ func TestInterpolateCounts(t *testing.T) { rate = float64(samplingCount) / float64(samplingTotal) factor = 1.0 / rate apply = func(v uint64) uint64 { return uint64(factor * float64(v)) } - emd = r.Endpoint.NodeMetadatas[srcNodeID].EdgeMetadatas[dstNodeID] + emd = r.Endpoint.NodeMetadatas[srcNodeID].Edges[dstNodeID] ) if want, have := apply(packetCount), (*emd.EgressPacketCount); want != have { t.Errorf("want %d packets, have %d", want, have) diff --git a/probe/sniff/sniffer_test.go b/probe/sniff/sniffer_test.go index e9ed3f7c2..3fb4d6bc2 100644 --- a/probe/sniff/sniffer_test.go +++ b/probe/sniff/sniffer_test.go @@ -66,7 +66,7 @@ func TestMerge(t *testing.T) { ) if want, have := (report.Topology{ NodeMetadatas: report.NodeMetadatas{ - srcEndpointNodeID: report.MakeNodeMetadata().WithEdgeMetadata(dstEndpointNodeID, report.EdgeMetadata{ + srcEndpointNodeID: report.MakeNodeMetadata().WithEdge(dstEndpointNodeID, report.EdgeMetadata{ EgressPacketCount: newu64(1), EgressByteCount: newu64(256), }), @@ -82,7 +82,7 @@ func TestMerge(t *testing.T) { ) if want, have := (report.Topology{ NodeMetadatas: report.NodeMetadatas{ - srcAddressNodeID: report.MakeNodeMetadata().WithEdgeMetadata(dstAddressNodeID, report.EdgeMetadata{ + srcAddressNodeID: report.MakeNodeMetadata().WithEdge(dstAddressNodeID, report.EdgeMetadata{ EgressPacketCount: newu64(1), EgressByteCount: newu64(512), }), diff --git a/render/render.go b/render/render.go index cb3775703..66cee9f11 100644 --- a/render/render.go +++ b/render/render.go @@ -175,7 +175,7 @@ func (m LeafMap) Render(rpt report.Report) RenderableNodes { for _, srcRenderableID := range source2mapped[srcNodeID] { srcRenderableNode := nodes[srcRenderableID] - for dstNodeID, emd := range nmd.EdgeMetadatas { + for dstNodeID, emd := range nmd.Edges { for _, dstRenderableID := range source2mapped[dstNodeID] { dstRenderableNode := nodes[dstRenderableID] @@ -227,7 +227,7 @@ func (m LeafMap) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableI metadata = report.EdgeMetadata{} ) for src, nmd := range t.NodeMetadatas { - for dst, edgeMeta := range nmd.EdgeMetadatas { + for dst, edgeMeta := range nmd.Edges { srcs, dsts := report.MakeIDList(src), report.MakeIDList(dst) if src != report.TheInternet { mapped := m.Mapper(t.NodeMetadatas[src], localNetworks) diff --git a/render/render_test.go b/render/render_test.go index dbb2bee01..22e84d013 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -115,14 +115,14 @@ func TestMapEdge(t *testing.T) { NodeMetadatas: report.NodeMetadatas{ "foo": report.MakeNodeMetadata().WithMetadata(map[string]string{ "id": "foo", - }).WithEdgeMetadata("bar", report.EdgeMetadata{ + }).WithEdge("bar", report.EdgeMetadata{ EgressPacketCount: newu64(1), EgressByteCount: newu64(2), }), "bar": report.MakeNodeMetadata().WithMetadata(map[string]string{ "id": "bar", - }).WithEdgeMetadata("foo", report.EdgeMetadata{ + }).WithEdge("foo", report.EdgeMetadata{ EgressPacketCount: newu64(3), EgressByteCount: newu64(4), }), diff --git a/report/topology.go b/report/topology.go index 52a0c048b..c30fd49d0 100644 --- a/report/topology.go +++ b/report/topology.go @@ -74,19 +74,19 @@ func (n NodeMetadatas) Merge(other NodeMetadatas) NodeMetadatas { // NodeMetadata describes a superset of the metadata that probes can collect // about a given node in a given topology. type NodeMetadata struct { - Metadata map[string]string - Counters map[string]int + Metadata + Counters Adjacency IDList - EdgeMetadatas + Edges EdgeMetadatas } // MakeNodeMetadata creates a new NodeMetadata with no initial metadata. func MakeNodeMetadata() NodeMetadata { return NodeMetadata{ - Metadata: map[string]string{}, - Counters: map[string]int{}, - Adjacency: MakeIDList(), - EdgeMetadatas: EdgeMetadatas{}, + Metadata: Metadata{}, + Counters: Counters{}, + Adjacency: MakeIDList(), + Edges: EdgeMetadatas{}, } } @@ -123,42 +123,78 @@ func (n NodeMetadata) WithAdjacent(a string) NodeMetadata { return result } -// WithEdgeMetadata returns a fresh copy of n, with 'dst' added to Adjacency and md added to EdgeMetadata -func (n NodeMetadata) WithEdgeMetadata(dst string, md EdgeMetadata) NodeMetadata { +// WithEdge returns a fresh copy of n, with 'dst' added to Adjacency and md added to EdgeMetadata +func (n NodeMetadata) WithEdge(dst string, md EdgeMetadata) NodeMetadata { result := n.Copy() result.Adjacency = result.Adjacency.Add(dst) - result.EdgeMetadatas[dst] = md + result.Edges[dst] = md return result } // Copy returns a value copy of the NodeMetadata. func (n NodeMetadata) Copy() NodeMetadata { cp := MakeNodeMetadata() - for k, v := range n.Metadata { - cp.Metadata[k] = v - } - for k, v := range n.Counters { - cp.Counters[k] = v - } + cp.Metadata = n.Metadata.Copy() + cp.Counters = n.Counters.Copy() cp.Adjacency = n.Adjacency.Copy() - cp.EdgeMetadatas = n.EdgeMetadatas.Copy() + cp.Edges = n.Edges.Copy() return cp } +// Merge mergses the individual components of a node and returns a +// fresh node. +func (n NodeMetadata) Merge(other NodeMetadata) NodeMetadata { + cp := n.Copy() + cp.Metadata = cp.Metadata.Merge(other.Metadata) + cp.Counters = cp.Counters.Merge(other.Counters) + cp.Adjacency = cp.Adjacency.Merge(other.Adjacency) + cp.Edges = cp.Edges.Merge(n.Edges) + return cp +} + +// Metadata is a string->string map +type Metadata map[string]string + // Merge merges two node metadata maps together. In case of conflict, the // other (right-hand) side wins. Always reassign the result of merge to the // destination. Merge does not modify the receiver. -func (n NodeMetadata) Merge(other NodeMetadata) NodeMetadata { - cp := n.Copy() - for k, v := range other.Metadata { - cp.Metadata[k] = v // other takes precedence +func (m Metadata) Merge(other Metadata) Metadata { + result := m.Copy() + for k, v := range other { + result[k] = v // other takes precedence } - for k, v := range other.Counters { - cp.Counters[k] = n.Counters[k] + v + return result +} + +// Copy creates a deep copy of the Metadata +func (m Metadata) Copy() Metadata { + result := Metadata{} + for k, v := range m { + result[k] = v } - cp.Adjacency = cp.Adjacency.Merge(other.Adjacency) - cp.EdgeMetadatas = cp.EdgeMetadatas.Merge(n.EdgeMetadatas) - return cp + return result +} + +// Counters is a string->int map +type Counters map[string]int + +// Merge merges two sets of counters into a fresh set of counters, +// summing values where appropriate +func (c Counters) Merge(other Counters) Counters { + result := c.Copy() + for k, v := range other { + result[k] = result[k] + v + } + return result +} + +// Copy creates a deep copy of the Counters +func (c Counters) Copy() Counters { + result := Counters{} + for k, v := range c { + result[k] = v + } + return result } // EdgeMetadatas collect metadata about each edge in a topology. Keys are @@ -263,7 +299,7 @@ func (t Topology) Validate() error { } // Check all the edge metadatas have entries in adjacencies - for dstNodeID := range nmd.EdgeMetadatas { + for dstNodeID := range nmd.Edges { if _, ok := t.NodeMetadatas[dstNodeID]; !ok { errs = append(errs, fmt.Sprintf("node %s metadatas missing for edge %q", dstNodeID, nodeID)) } diff --git a/test/report_fixture.go b/test/report_fixture.go index 346d86226..78a52c762 100644 --- a/test/report_fixture.go +++ b/test/report_fixture.go @@ -95,7 +95,7 @@ var ( endpoint.Port: ClientPort54001, process.PID: Client1PID, report.HostNodeID: ClientHostNodeID, - }).WithEdgeMetadata(Server80NodeID, report.EdgeMetadata{ + }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(10), EgressByteCount: newu64(100), }), @@ -105,7 +105,7 @@ var ( endpoint.Port: ClientPort54002, process.PID: Client2PID, report.HostNodeID: ClientHostNodeID, - }).WithEdgeMetadata(Server80NodeID, report.EdgeMetadata{ + }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(20), EgressByteCount: newu64(200), }), @@ -128,7 +128,7 @@ var ( UnknownClient1NodeID: report.MakeNodeMetadata().WithMetadata(map[string]string{ endpoint.Addr: UnknownClient1IP, endpoint.Port: UnknownClient1Port, - }).WithEdgeMetadata(Server80NodeID, report.EdgeMetadata{ + }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(30), EgressByteCount: newu64(300), }), @@ -136,7 +136,7 @@ var ( UnknownClient2NodeID: report.MakeNodeMetadata().WithMetadata(map[string]string{ endpoint.Addr: UnknownClient2IP, endpoint.Port: UnknownClient2Port, - }).WithEdgeMetadata(Server80NodeID, report.EdgeMetadata{ + }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(40), EgressByteCount: newu64(400), }), @@ -144,7 +144,7 @@ var ( UnknownClient3NodeID: report.MakeNodeMetadata().WithMetadata(map[string]string{ endpoint.Addr: UnknownClient3IP, endpoint.Port: UnknownClient3Port, - }).WithEdgeMetadata(Server80NodeID, report.EdgeMetadata{ + }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(50), EgressByteCount: newu64(500), }), @@ -152,7 +152,7 @@ var ( RandomClientNodeID: report.MakeNodeMetadata().WithMetadata(map[string]string{ endpoint.Addr: RandomClientIP, endpoint.Port: RandomClientPort, - }).WithEdgeMetadata(Server80NodeID, report.EdgeMetadata{ + }).WithEdge(Server80NodeID, report.EdgeMetadata{ EgressPacketCount: newu64(60), EgressByteCount: newu64(600), }), @@ -229,7 +229,7 @@ var ( ClientAddressNodeID: report.MakeNodeMetadata().WithMetadata(map[string]string{ endpoint.Addr: ClientIP, report.HostNodeID: ClientHostNodeID, - }).WithEdgeMetadata(ServerAddressNodeID, report.EdgeMetadata{ + }).WithEdge(ServerAddressNodeID, report.EdgeMetadata{ MaxConnCountTCP: newu64(3), }),