From b138dda3b381eb0b7ff2f096b7a6ceebb8ea38f4 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Tue, 25 Aug 2015 13:31:25 +0100 Subject: [PATCH 1/7] Move merge functions with their types --- report/merge.go | 116 --------------------------------------------- report/report.go | 19 ++++++++ report/topology.go | 94 +++++++++++++++++++++++++++++++++++- 3 files changed, 111 insertions(+), 118 deletions(-) delete mode 100644 report/merge.go diff --git a/report/merge.go b/report/merge.go deleted file mode 100644 index 223ffa433..000000000 --- a/report/merge.go +++ /dev/null @@ -1,116 +0,0 @@ -package report - -// Merge functions for all topology datatypes. The general semantics are that -// the receiver is modified, and what's merged in isn't. - -// Merge merges another Report into the receiver. Pass addWindows true if the -// reports represent distinct (non-overlapping) periods of time. -func (r *Report) Merge(other Report) { - r.Endpoint.Merge(other.Endpoint) - r.Address.Merge(other.Address) - r.Process.Merge(other.Process) - r.Container.Merge(other.Container) - r.ContainerImage.Merge(other.ContainerImage) - r.Host.Merge(other.Host) - r.Overlay.Merge(other.Overlay) - r.Sampling.Merge(other.Sampling) - r.Window += other.Window -} - -// Merge merges another Topology into the receiver. -func (t *Topology) Merge(other Topology) { - t.Adjacency.Merge(other.Adjacency) - t.EdgeMetadatas.Merge(other.EdgeMetadatas) - t.NodeMetadatas.Merge(other.NodeMetadatas) -} - -// Merge merges another Adjacency list into the receiver. -func (a *Adjacency) Merge(other Adjacency) { - for addr, adj := range other { - (*a)[addr] = (*a)[addr].Merge(adj) - } -} - -// Merge merges another NodeMetadatas into the receiver. -func (m *NodeMetadatas) Merge(other NodeMetadatas) { - for id, meta := range other { - if _, ok := (*m)[id]; !ok { - (*m)[id] = meta // not a copy - } - } -} - -// 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 is defined on the value-type, but node metadata map is -// itself a reference type, so if you want to maintain immutability, use copy. -func (nm NodeMetadata) Merge(other NodeMetadata) NodeMetadata { - for k, v := range other.Metadata { - nm.Metadata[k] = v // other takes precedence - } - for k, v := range other.Counters { - nm.Counters[k] = nm.Counters[k] + v - } - return nm -} - -// Merge merges another EdgeMetadatas into the receiver. If other is from -// another probe this is the union of both metadatas. Keys present in both are -// summed. -func (e *EdgeMetadatas) Merge(other EdgeMetadatas) { - for id, edgemeta := range other { - local := (*e)[id] - local.Merge(edgemeta) - (*e)[id] = local - } -} - -// Merge merges another EdgeMetadata into the receiver. The two edge metadatas -// should represent the same edge on different times. -func (m *EdgeMetadata) Merge(other EdgeMetadata) { - m.EgressPacketCount = merge(m.EgressPacketCount, other.EgressPacketCount, sum) - m.IngressPacketCount = merge(m.IngressPacketCount, other.IngressPacketCount, sum) - m.EgressByteCount = merge(m.EgressByteCount, other.EgressByteCount, sum) - m.IngressByteCount = merge(m.IngressByteCount, other.IngressByteCount, sum) - m.MaxConnCountTCP = merge(m.MaxConnCountTCP, other.MaxConnCountTCP, max) -} - -// Flatten sums two EdgeMetadatas. Their windows should be the same duration; -// they should represent different edges at the same time. -func (m *EdgeMetadata) Flatten(other EdgeMetadata) { - m.EgressPacketCount = merge(m.EgressPacketCount, other.EgressPacketCount, sum) - m.IngressPacketCount = merge(m.IngressPacketCount, other.IngressPacketCount, sum) - m.EgressByteCount = merge(m.EgressByteCount, other.EgressByteCount, sum) - m.IngressByteCount = merge(m.IngressByteCount, other.IngressByteCount, sum) - // Note that summing of two maximums doesn't always give us the true - // maximum. But it's a best effort. - m.MaxConnCountTCP = merge(m.MaxConnCountTCP, other.MaxConnCountTCP, sum) -} - -// Merge combines two sampling structures via simple addition. -func (s *Sampling) Merge(other Sampling) { - s.Count += other.Count - s.Total += other.Total -} - -func merge(dst, src *uint64, op func(uint64, uint64) uint64) *uint64 { - if src == nil { - return dst - } - if dst == nil { - dst = new(uint64) - } - (*dst) = op(*dst, *src) - return dst -} - -func sum(dst, src uint64) uint64 { - return dst + src -} - -func max(dst, src uint64) uint64 { - if dst > src { - return dst - } - return src -} diff --git a/report/report.go b/report/report.go index 990d5bab0..05df26599 100644 --- a/report/report.go +++ b/report/report.go @@ -70,6 +70,19 @@ func MakeReport() Report { } } +// Merge merges another Report into the receiver. +func (r *Report) Merge(other Report) { + r.Endpoint.Merge(other.Endpoint) + r.Address.Merge(other.Address) + r.Process.Merge(other.Process) + r.Container.Merge(other.Container) + r.ContainerImage.Merge(other.ContainerImage) + r.Host.Merge(other.Host) + r.Overlay.Merge(other.Overlay) + r.Sampling.Merge(other.Sampling) + r.Window += other.Window +} + // Topologies returns a slice of Topologies in this report func (r Report) Topologies() []Topology { return []Topology{ @@ -118,6 +131,12 @@ func (s Sampling) Rate() float64 { return float64(s.Count) / float64(s.Total) } +// Merge combines two sampling structures via simple addition. +func (s *Sampling) Merge(other Sampling) { + s.Count += other.Count + s.Total += other.Total +} + const ( // HostNodeID is a metadata foreign key, linking a node in any topology to // a node in the host topology. That host node is the origin host, where diff --git a/report/topology.go b/report/topology.go index 6c4760ea3..857c55e0c 100644 --- a/report/topology.go +++ b/report/topology.go @@ -5,8 +5,6 @@ import ( "strings" ) -const localUnknown = "localUnknown" - // Topology describes a specific view of a network. It consists of nodes and // edges, represented by Adjacency, and metadata about those nodes and edges, // represented by EdgeMetadatas and NodeMetadatas respectively. @@ -16,18 +14,52 @@ type Topology struct { NodeMetadatas } +// Merge merges another Topology into the receiver. +func (t *Topology) Merge(other Topology) { + t.Adjacency.Merge(other.Adjacency) + t.EdgeMetadatas.Merge(other.EdgeMetadatas) + t.NodeMetadatas.Merge(other.NodeMetadatas) +} + // Adjacency is an adjacency-list encoding of the topology. Keys are node IDs, // as produced by the relevant MappingFunc for the topology. type Adjacency map[string]IDList +// Merge merges another Adjacency list into the receiver. +func (a *Adjacency) Merge(other Adjacency) { + for addr, adj := range other { + (*a)[addr] = (*a)[addr].Merge(adj) + } +} + // EdgeMetadatas collect metadata about each edge in a topology. Keys are a // concatenation of node IDs. type EdgeMetadatas map[string]EdgeMetadata +// Merge merges another EdgeMetadatas into the receiver. If other is from +// another probe this is the union of both metadatas. Keys present in both are +// summed. +func (e *EdgeMetadatas) Merge(other EdgeMetadatas) { + for id, edgemeta := range other { + local := (*e)[id] + local.Merge(edgemeta) + (*e)[id] = local + } +} + // NodeMetadatas collect metadata about each node in a topology. Keys are node // IDs. type NodeMetadatas map[string]NodeMetadata +// Merge merges another NodeMetadatas into the receiver. +func (m *NodeMetadatas) Merge(other NodeMetadatas) { + for id, meta := range other { + if _, ok := (*m)[id]; !ok { + (*m)[id] = meta // not a copy + } + } +} + // EdgeMetadata describes a superset of the metadata that probes can possibly // collect about a directed edge between two nodes in any topology. type EdgeMetadata struct { @@ -38,6 +70,28 @@ type EdgeMetadata struct { MaxConnCountTCP *uint64 `json:"max_conn_count_tcp,omitempty"` } +// Merge merges another EdgeMetadata into the receiver. The two edge metadatas +// should represent the same edge on different times. +func (m *EdgeMetadata) Merge(other EdgeMetadata) { + m.EgressPacketCount = merge(m.EgressPacketCount, other.EgressPacketCount, sum) + m.IngressPacketCount = merge(m.IngressPacketCount, other.IngressPacketCount, sum) + m.EgressByteCount = merge(m.EgressByteCount, other.EgressByteCount, sum) + m.IngressByteCount = merge(m.IngressByteCount, other.IngressByteCount, sum) + m.MaxConnCountTCP = merge(m.MaxConnCountTCP, other.MaxConnCountTCP, max) +} + +// Flatten sums two EdgeMetadatas. Their windows should be the same duration; +// they should represent different edges at the same time. +func (m *EdgeMetadata) Flatten(other EdgeMetadata) { + m.EgressPacketCount = merge(m.EgressPacketCount, other.EgressPacketCount, sum) + m.IngressPacketCount = merge(m.IngressPacketCount, other.IngressPacketCount, sum) + m.EgressByteCount = merge(m.EgressByteCount, other.EgressByteCount, sum) + m.IngressByteCount = merge(m.IngressByteCount, other.IngressByteCount, sum) + // Note that summing of two maximums doesn't always give us the true + // maximum. But it's a best effort. + m.MaxConnCountTCP = merge(m.MaxConnCountTCP, other.MaxConnCountTCP, sum) +} + // NodeMetadata describes a superset of the metadata that probes can collect // about a given node in a given topology. type NodeMetadata struct { @@ -58,6 +112,20 @@ func MakeNodeMetadataWith(m map[string]string) NodeMetadata { } } +// 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 is defined on the value-type, but node metadata map is +// itself a reference type, so if you want to maintain immutability, use copy. +func (nm NodeMetadata) Merge(other NodeMetadata) NodeMetadata { + for k, v := range other.Metadata { + nm.Metadata[k] = v // other takes precedence + } + for k, v := range other.Counters { + nm.Counters[k] = nm.Counters[k] + v + } + return nm +} + // Copy returns a value copy, useful for tests. func (nm NodeMetadata) Copy() NodeMetadata { cp := MakeNodeMetadata() @@ -140,3 +208,25 @@ func (t Topology) Validate() error { return nil } + +func merge(dst, src *uint64, op func(uint64, uint64) uint64) *uint64 { + if src == nil { + return dst + } + if dst == nil { + dst = new(uint64) + } + (*dst) = op(*dst, *src) + return dst +} + +func sum(dst, src uint64) uint64 { + return dst + src +} + +func max(dst, src uint64) uint64 { + if dst > src { + return dst + } + return src +} From 42be036428ec9d01329e1cdbe5e9d0a8a0ff4d46 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Tue, 25 Aug 2015 17:52:20 +0100 Subject: [PATCH 2/7] report: always copy --- report/id_list.go | 12 +++ report/merge_test.go | 14 +-- report/report.go | 65 +++++++++----- report/report_test.go | 2 +- report/topology.go | 197 ++++++++++++++++++++++++++++-------------- 5 files changed, 192 insertions(+), 98 deletions(-) diff --git a/report/id_list.go b/report/id_list.go index c98ce509b..c77dc86de 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -7,6 +7,9 @@ type IDList []string // MakeIDList makes a new IDList. func MakeIDList(ids ...string) IDList { + if len(ids) <= 0 { + return IDList{} + } sort.Strings(ids) for i := 1; i < len(ids); { // shuffle down any duplicates if ids[i-1] == ids[i] { @@ -18,6 +21,15 @@ func MakeIDList(ids ...string) IDList { return IDList(ids) } +// Copy returns a copy of the IDList. +func (a IDList) Copy() IDList { + cp := make(IDList, len(a)) + for i, s := range a { + cp[i] = s + } + return cp +} + // Add is the only correct way to add ids to an IDList. func (a IDList) Add(ids ...string) IDList { for _, s := range ids { diff --git a/report/merge_test.go b/report/merge_test.go index 1426ea17f..1dab3a92f 100644 --- a/report/merge_test.go +++ b/report/merge_test.go @@ -96,9 +96,7 @@ func TestMergeAdjacency(t *testing.T) { }, }, } { - have := c.a - have.Merge(c.b) - if !reflect.DeepEqual(c.want, have) { + if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { t.Errorf("%s: want\n\t%#v\nhave\n\t%#v", name, c.want, have) } } @@ -192,10 +190,7 @@ func TestMergeEdgeMetadatas(t *testing.T) { }, }, } { - have := c.a - have.Merge(c.b) - - if !reflect.DeepEqual(c.want, have) { + if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { t.Errorf("%s: want\n\t%#v, have\n\t%#v", name, c.want, have) } } @@ -291,10 +286,7 @@ func TestMergeNodeMetadatas(t *testing.T) { }, }, } { - have := c.a - have.Merge(c.b) - - if !reflect.DeepEqual(c.want, have) { + if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { t.Errorf("%s: want\n\t%#v, have\n\t%#v", name, c.want, have) } } diff --git a/report/report.go b/report/report.go index 05df26599..99716a634 100644 --- a/report/report.go +++ b/report/report.go @@ -58,29 +58,47 @@ type Report struct { // MakeReport makes a clean report, ready to Merge() other reports into. func MakeReport() Report { return Report{ - Endpoint: NewTopology(), - Address: NewTopology(), - Process: NewTopology(), - Container: NewTopology(), - ContainerImage: NewTopology(), - Host: NewTopology(), - Overlay: NewTopology(), + Endpoint: MakeTopology(), + Address: MakeTopology(), + Process: MakeTopology(), + Container: MakeTopology(), + ContainerImage: MakeTopology(), + Host: MakeTopology(), + Overlay: MakeTopology(), Sampling: Sampling{}, Window: 0, } } -// Merge merges another Report into the receiver. -func (r *Report) Merge(other Report) { - r.Endpoint.Merge(other.Endpoint) - r.Address.Merge(other.Address) - r.Process.Merge(other.Process) - r.Container.Merge(other.Container) - r.ContainerImage.Merge(other.ContainerImage) - r.Host.Merge(other.Host) - r.Overlay.Merge(other.Overlay) - r.Sampling.Merge(other.Sampling) - r.Window += other.Window +// Copy returns a value copy of the report. +func (r Report) Copy() Report { + return Report{ + Endpoint: r.Endpoint.Copy(), + Address: r.Address.Copy(), + Process: r.Process.Copy(), + Container: r.Container.Copy(), + ContainerImage: r.ContainerImage.Copy(), + Host: r.Host.Copy(), + Overlay: r.Overlay.Copy(), + Sampling: r.Sampling, + Window: r.Window, + } +} + +// Merge merges another Report into the receiver and returns the result. The +// original is not modified. +func (r Report) Merge(other Report) Report { + cp := r.Copy() + cp.Endpoint = r.Endpoint.Merge(other.Endpoint) + cp.Address = r.Address.Merge(other.Address) + cp.Process = r.Process.Merge(other.Process) + cp.Container = r.Container.Merge(other.Container) + cp.ContainerImage = r.ContainerImage.Merge(other.ContainerImage) + cp.Host = r.Host.Merge(other.Host) + cp.Overlay = r.Overlay.Merge(other.Overlay) + cp.Sampling = r.Sampling.Merge(other.Sampling) + cp.Window += other.Window + return cp } // Topologies returns a slice of Topologies in this report @@ -131,10 +149,13 @@ func (s Sampling) Rate() float64 { return float64(s.Count) / float64(s.Total) } -// Merge combines two sampling structures via simple addition. -func (s *Sampling) Merge(other Sampling) { - s.Count += other.Count - s.Total += other.Total +// Merge combines two sampling structures via simple addition and returns the +// result. The original is not modified. +func (s Sampling) Merge(other Sampling) Sampling { + return Sampling{ + Count: s.Count + other.Count, + Total: s.Total + other.Total, + } } const ( diff --git a/report/report_test.go b/report/report_test.go index aa330ce1f..1e83b96b6 100644 --- a/report/report_test.go +++ b/report/report_test.go @@ -11,7 +11,7 @@ import ( func TestReportTopologies(t *testing.T) { var ( reportType = reflect.TypeOf(report.MakeReport()) - topologyType = reflect.TypeOf(report.NewTopology()) + topologyType = reflect.TypeOf(report.MakeTopology()) ) var want int diff --git a/report/topology.go b/report/topology.go index 857c55e0c..64efb5500 100644 --- a/report/topology.go +++ b/report/topology.go @@ -14,50 +14,103 @@ type Topology struct { NodeMetadatas } -// Merge merges another Topology into the receiver. -func (t *Topology) Merge(other Topology) { - t.Adjacency.Merge(other.Adjacency) - t.EdgeMetadatas.Merge(other.EdgeMetadatas) - t.NodeMetadatas.Merge(other.NodeMetadatas) +// MakeTopology gives you a Topology. +func MakeTopology() Topology { + return Topology{ + Adjacency: map[string]IDList{}, + EdgeMetadatas: map[string]EdgeMetadata{}, + NodeMetadatas: map[string]NodeMetadata{}, + } } -// Adjacency is an adjacency-list encoding of the topology. Keys are node IDs, -// as produced by the relevant MappingFunc for the topology. +// Copy returns a value copy of the Topology. +func (t Topology) Copy() Topology { + return Topology{ + Adjacency: t.Adjacency.Copy(), + EdgeMetadatas: t.EdgeMetadatas.Copy(), + NodeMetadatas: t.NodeMetadatas.Copy(), + } +} + +// Merge merges the other object into this one, and returns the result object. +// The original is not modified. +func (t Topology) Merge(other Topology) Topology { + return Topology{ + Adjacency: t.Adjacency.Merge(other.Adjacency), + EdgeMetadatas: t.EdgeMetadatas.Merge(other.EdgeMetadatas), + NodeMetadatas: t.NodeMetadatas.Merge(other.NodeMetadatas), + } +} + +// Adjacency is an adjacency-list encoding of the topology. Keys are adjacency +// IDs, values are lists of node IDs. type Adjacency map[string]IDList -// Merge merges another Adjacency list into the receiver. -func (a *Adjacency) Merge(other Adjacency) { - for addr, adj := range other { - (*a)[addr] = (*a)[addr].Merge(adj) +// Copy returns a value copy of the adjacency. +func (a Adjacency) Copy() Adjacency { + cp := make(Adjacency, len(a)) + for k, v := range a { + cp[k] = v.Copy() } + return cp +} + +// Merge merges the other object into this one, and returns the result object. +// The original is not modified. +func (a Adjacency) Merge(other Adjacency) Adjacency { + cp := a.Copy() + for k, v := range other { + cp[k] = cp[k].Merge(v) + } + return cp } // EdgeMetadatas collect metadata about each edge in a topology. Keys are a // concatenation of node IDs. type EdgeMetadatas map[string]EdgeMetadata -// Merge merges another EdgeMetadatas into the receiver. If other is from -// another probe this is the union of both metadatas. Keys present in both are -// summed. -func (e *EdgeMetadatas) Merge(other EdgeMetadatas) { - for id, edgemeta := range other { - local := (*e)[id] - local.Merge(edgemeta) - (*e)[id] = local +// Copy returns a value copy of the EdgeMetadatas. +func (e EdgeMetadatas) Copy() EdgeMetadatas { + cp := make(EdgeMetadatas, len(e)) + for k, v := range e { + cp[k] = v.Copy() } + return cp +} + +// Merge merges the other object into this one, and returns the result object. +// The original is not modified. +func (e EdgeMetadatas) Merge(other EdgeMetadatas) EdgeMetadatas { + cp := e.Copy() + for k, v := range other { + cp[k] = cp[k].Merge(v) + } + return cp } // NodeMetadatas collect metadata about each node in a topology. Keys are node // IDs. type NodeMetadatas map[string]NodeMetadata -// Merge merges another NodeMetadatas into the receiver. -func (m *NodeMetadatas) Merge(other NodeMetadatas) { - for id, meta := range other { - if _, ok := (*m)[id]; !ok { - (*m)[id] = meta // not a copy +// Copy returns a value copy of the NodeMetadatas. +func (n NodeMetadatas) Copy() NodeMetadatas { + cp := make(NodeMetadatas, len(n)) + for k, v := range n { + cp[k] = v.Copy() + } + return cp +} + +// Merge merges the other object into this one, and returns the result object. +// The original is not modified. +func (n NodeMetadatas) Merge(other NodeMetadatas) NodeMetadatas { + cp := n.Copy() + for k, v := range other { + if _, ok := cp[k]; !ok { // don't overwrite + cp[k] = v.Copy() } } + return cp } // EdgeMetadata describes a superset of the metadata that probes can possibly @@ -70,26 +123,51 @@ type EdgeMetadata struct { MaxConnCountTCP *uint64 `json:"max_conn_count_tcp,omitempty"` } -// Merge merges another EdgeMetadata into the receiver. The two edge metadatas -// should represent the same edge on different times. -func (m *EdgeMetadata) Merge(other EdgeMetadata) { - m.EgressPacketCount = merge(m.EgressPacketCount, other.EgressPacketCount, sum) - m.IngressPacketCount = merge(m.IngressPacketCount, other.IngressPacketCount, sum) - m.EgressByteCount = merge(m.EgressByteCount, other.EgressByteCount, sum) - m.IngressByteCount = merge(m.IngressByteCount, other.IngressByteCount, sum) - m.MaxConnCountTCP = merge(m.MaxConnCountTCP, other.MaxConnCountTCP, max) +// Copy returns a value copy of the EdgeMetadata. +func (e EdgeMetadata) Copy() EdgeMetadata { + return EdgeMetadata{ + EgressPacketCount: cpu64ptr(e.EgressPacketCount), + IngressPacketCount: cpu64ptr(e.IngressPacketCount), + EgressByteCount: cpu64ptr(e.EgressByteCount), + IngressByteCount: cpu64ptr(e.IngressByteCount), + MaxConnCountTCP: cpu64ptr(e.MaxConnCountTCP), + } } -// Flatten sums two EdgeMetadatas. Their windows should be the same duration; -// they should represent different edges at the same time. -func (m *EdgeMetadata) Flatten(other EdgeMetadata) { - m.EgressPacketCount = merge(m.EgressPacketCount, other.EgressPacketCount, sum) - m.IngressPacketCount = merge(m.IngressPacketCount, other.IngressPacketCount, sum) - m.EgressByteCount = merge(m.EgressByteCount, other.EgressByteCount, sum) - m.IngressByteCount = merge(m.IngressByteCount, other.IngressByteCount, sum) +func cpu64ptr(u *uint64) *uint64 { + if u == nil { + return nil + } + value := *u // oh man + return &value // this sucks +} + +// Merge merges another EdgeMetadata into the receiver and returns the result. +// The receiver is not modified. The two edge metadatas should represent the +// same edge on different times. +func (e EdgeMetadata) Merge(other EdgeMetadata) EdgeMetadata { + cp := e.Copy() + cp.EgressPacketCount = merge(cp.EgressPacketCount, other.EgressPacketCount, sum) + cp.IngressPacketCount = merge(cp.IngressPacketCount, other.IngressPacketCount, sum) + cp.EgressByteCount = merge(cp.EgressByteCount, other.EgressByteCount, sum) + cp.IngressByteCount = merge(cp.IngressByteCount, other.IngressByteCount, sum) + cp.MaxConnCountTCP = merge(cp.MaxConnCountTCP, other.MaxConnCountTCP, max) + return cp +} + +// Flatten sums two EdgeMetadatas and returns the result. The receiver is not +// modified. The two edge metadata windows should be the same duration; they +// should represent different edges at the same time. +func (e EdgeMetadata) Flatten(other EdgeMetadata) EdgeMetadata { + cp := e.Copy() + cp.EgressPacketCount = merge(cp.EgressPacketCount, other.EgressPacketCount, sum) + cp.IngressPacketCount = merge(cp.IngressPacketCount, other.IngressPacketCount, sum) + cp.EgressByteCount = merge(cp.EgressByteCount, other.EgressByteCount, sum) + cp.IngressByteCount = merge(cp.IngressByteCount, other.IngressByteCount, sum) // Note that summing of two maximums doesn't always give us the true // maximum. But it's a best effort. - m.MaxConnCountTCP = merge(m.MaxConnCountTCP, other.MaxConnCountTCP, sum) + cp.MaxConnCountTCP = merge(cp.MaxConnCountTCP, other.MaxConnCountTCP, sum) + return cp } // NodeMetadata describes a superset of the metadata that probes can collect @@ -112,39 +190,30 @@ func MakeNodeMetadataWith(m map[string]string) NodeMetadata { } } -// 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 is defined on the value-type, but node metadata map is -// itself a reference type, so if you want to maintain immutability, use copy. -func (nm NodeMetadata) Merge(other NodeMetadata) NodeMetadata { - for k, v := range other.Metadata { - nm.Metadata[k] = v // other takes precedence - } - for k, v := range other.Counters { - nm.Counters[k] = nm.Counters[k] + v - } - return nm -} - -// Copy returns a value copy, useful for tests. -func (nm NodeMetadata) Copy() NodeMetadata { +// Copy returns a value copy of the NodeMetadata. +func (n NodeMetadata) Copy() NodeMetadata { cp := MakeNodeMetadata() - for k, v := range nm.Metadata { + for k, v := range n.Metadata { cp.Metadata[k] = v } - for k, v := range nm.Counters { + for k, v := range n.Counters { cp.Counters[k] = v } return cp } -// NewTopology gives you a Topology. -func NewTopology() Topology { - return Topology{ - Adjacency: map[string]IDList{}, - EdgeMetadatas: map[string]EdgeMetadata{}, - NodeMetadatas: map[string]NodeMetadata{}, +// 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 } + for k, v := range other.Counters { + cp.Counters[k] = n.Counters[k] + v + } + return cp } // Validate checks the topology for various inconsistencies. From d5ab8177f6502c97e647bf69d4b08e5618303fbd Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Tue, 25 Aug 2015 17:52:59 +0100 Subject: [PATCH 3/7] probe: fixes for new merge semantics --- probe/docker/reporter.go | 8 ++++---- probe/docker/tagger.go | 2 +- probe/host/tagger.go | 6 +++--- probe/process/reporter.go | 4 ++-- probe/tag_report.go | 6 +++--- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index 587fcff28..734fb3c3f 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -29,13 +29,13 @@ func NewReporter(registry Registry, scope string) *Reporter { // Report generates a Report containing Container and ContainerImage topologies func (r *Reporter) Report() (report.Report, error) { result := report.MakeReport() - result.Container.Merge(r.containerTopology()) - result.ContainerImage.Merge(r.containerImageTopology()) + result.Container = result.Container.Merge(r.containerTopology()) + result.ContainerImage = result.ContainerImage.Merge(r.containerImageTopology()) return result, nil } func (r *Reporter) containerTopology() report.Topology { - result := report.NewTopology() + result := report.MakeTopology() r.registry.WalkContainers(func(c Container) { nodeID := report.MakeContainerNodeID(r.scope, c.ID()) @@ -46,7 +46,7 @@ func (r *Reporter) containerTopology() report.Topology { } func (r *Reporter) containerImageTopology() report.Topology { - result := report.NewTopology() + result := report.MakeTopology() r.registry.WalkImages(func(image *docker_client.APIImages) { nmd := report.MakeNodeMetadataWith(map[string]string{ diff --git a/probe/docker/tagger.go b/probe/docker/tagger.go index 3badea8a0..201b317c8 100644 --- a/probe/docker/tagger.go +++ b/probe/docker/tagger.go @@ -83,6 +83,6 @@ func (t *Tagger) tag(tree process.Tree, topology *report.Topology) { ContainerID: c.ID(), }) - topology.NodeMetadatas[nodeID].Merge(md) + topology.NodeMetadatas[nodeID] = nodeMetadata.Merge(md) } } diff --git a/probe/host/tagger.go b/probe/host/tagger.go index 0776dc869..ac381012e 100644 --- a/probe/host/tagger.go +++ b/probe/host/tagger.go @@ -17,10 +17,10 @@ func NewTagger(hostID string) Tagger { // Tag implements Tagger. func (t Tagger) Tag(r report.Report) (report.Report, error) { - md := report.MakeNodeMetadataWith(map[string]string{report.HostNodeID: t.hostNodeID}) + other := report.MakeNodeMetadataWith(map[string]string{report.HostNodeID: t.hostNodeID}) for _, topology := range r.Topologies() { - for nodeID := range topology.NodeMetadatas { - topology.NodeMetadatas[nodeID].Merge(md) + for id, md := range topology.NodeMetadatas { + topology.NodeMetadatas[id] = md.Merge(other) } } return r, nil diff --git a/probe/process/reporter.go b/probe/process/reporter.go index dd595cbc2..28c9c99af 100644 --- a/probe/process/reporter.go +++ b/probe/process/reporter.go @@ -36,12 +36,12 @@ func (r *Reporter) Report() (report.Report, error) { if err != nil { return result, err } - result.Process.Merge(processes) + result.Process = result.Process.Merge(processes) return result, nil } func (r *Reporter) processTopology() (report.Topology, error) { - t := report.NewTopology() + t := report.MakeTopology() err := r.walker.Walk(func(p Process) { pidstr := strconv.Itoa(p.PID) nodeID := report.MakeProcessNodeID(r.scope, pidstr) diff --git a/probe/tag_report.go b/probe/tag_report.go index 92d298259..cf1f240a1 100644 --- a/probe/tag_report.go +++ b/probe/tag_report.go @@ -50,9 +50,9 @@ func (topologyTagger) Tag(r report.Report) (report.Report, error) { "host": &(r.Host), "overlay": &(r.Overlay), } { - md := report.MakeNodeMetadataWith(map[string]string{Topology: val}) - for nodeID := range topology.NodeMetadatas { - (*topology).NodeMetadatas[nodeID].Merge(md) + other := report.MakeNodeMetadataWith(map[string]string{Topology: val}) + for id, md := range topology.NodeMetadatas { + topology.NodeMetadatas[id] = md.Merge(other) } } return r, nil From 8f55c74b27037986447355818db51c437bb9f018 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 26 Aug 2015 09:45:42 +0100 Subject: [PATCH 4/7] xfer: fixes for new merge semantics --- xfer/collector.go | 6 +++--- xfer/collector_test.go | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/xfer/collector.go b/xfer/collector.go index a19d684ac..5155f4c96 100644 --- a/xfer/collector.go +++ b/xfer/collector.go @@ -52,11 +52,11 @@ func (c *Collector) Report() report.Report { c.reports = clean(c.reports, c.window) - report := report.MakeReport() + rpt := report.MakeReport() for _, tr := range c.reports { - report.Merge(tr.report) + rpt = rpt.Merge(tr.report) } - return report + return rpt } type timestampReport struct { diff --git a/xfer/collector_test.go b/xfer/collector_test.go index f8fc98508..1025b09d8 100644 --- a/xfer/collector_test.go +++ b/xfer/collector_test.go @@ -32,9 +32,10 @@ func TestCollector(t *testing.T) { } c.Add(r2) + merged := report.MakeReport() - merged.Merge(r1) - merged.Merge(r2) + merged = merged.Merge(r1) + merged = merged.Merge(r2) if want, have := merged, c.Report(); !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } From 775f0ede90c947282490b59ced2159fedf56fb9d Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 26 Aug 2015 09:52:59 +0100 Subject: [PATCH 5/7] render: fixes for new merge semantics --- render/detailed_node.go | 8 +++-- render/expected/expected.go | 3 ++ render/render.go | 50 ++++++++++++++-------------- render/render_test.go | 13 ++++---- render/renderable_node.go | 16 ++++----- render/renderable_node_test.go | 61 ++++++++++++++++++++++++---------- render/theinternet_test.go | 15 ++++++--- render/topologies_test.go | 50 +++++++++++----------------- report/merge_test.go | 22 +++++++++++- 9 files changed, 143 insertions(+), 95 deletions(-) diff --git a/render/detailed_node.go b/render/detailed_node.go index 66e42d135..1dac743ed 100644 --- a/render/detailed_node.go +++ b/render/detailed_node.go @@ -109,9 +109,11 @@ 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{}) +func getRenderingContext(r report.Report, n RenderableNode) (multiContainer, multiHost bool) { + var ( + originHosts = map[string]struct{}{} + originContainers = map[string]struct{}{} + ) for _, id := range n.Origins { for _, topology := range r.Topologies() { if nmd, ok := topology.NodeMetadatas[id]; ok { diff --git a/render/expected/expected.go b/render/expected/expected.go index a3f0248fa..7349fdb7a 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -186,6 +186,7 @@ var ( LabelMinor: "1 process", Rank: "bash", Pseudo: false, + Adjacency: report.MakeIDList(), Origins: report.MakeIDList( test.NonContainerProcessNodeID, test.ServerHostNodeID, @@ -245,6 +246,7 @@ var ( LabelMinor: test.ServerHostName, Rank: "", Pseudo: true, + Adjacency: report.MakeIDList(), Origins: report.MakeIDList( test.NonContainerProcessNodeID, test.ServerHostNodeID, @@ -303,6 +305,7 @@ var ( LabelMinor: test.ServerHostName, Rank: "", Pseudo: true, + Adjacency: report.MakeIDList(), Origins: report.MakeIDList( test.NonContainerProcessNodeID, test.ServerHostNodeID, diff --git a/render/render.go b/render/render.go index 205d68b75..5990885d8 100644 --- a/render/render.go +++ b/render/render.go @@ -16,26 +16,6 @@ type Renderer interface { // other renderers. type Reduce []Renderer -// Map is a Renderer which produces a set of RenderableNodes from the set of -// RenderableNodes produced by another Renderer. -type Map struct { - MapFunc - Renderer -} - -// LeafMap is a Renderer which produces a set of RenderableNodes from a report.Topology -// by using a map function and topology selector. -type LeafMap struct { - Selector report.TopologySelector - Mapper LeafMapFunc - Pseudo PseudoFunc -} - -// FilterUnconnected is a Renderer which filters out unconnected nodes. -type FilterUnconnected struct { - Renderer -} - // MakeReduce is the only sane way to produce a Reduce Renderer. func MakeReduce(renderers ...Renderer) Renderer { return Reduce(renderers) @@ -54,11 +34,18 @@ func (r Reduce) Render(rpt report.Report) RenderableNodes { func (r Reduce) EdgeMetadata(rpt report.Report, localID, remoteID string) report.EdgeMetadata { metadata := report.EdgeMetadata{} for _, renderer := range r { - metadata.Merge(renderer.EdgeMetadata(rpt, localID, remoteID)) + metadata = metadata.Merge(renderer.EdgeMetadata(rpt, localID, remoteID)) } return metadata } +// Map is a Renderer which produces a set of RenderableNodes from the set of +// RenderableNodes produced by another Renderer. +type Map struct { + MapFunc + Renderer +} + // Render transforms a set of RenderableNodes produces by another Renderer. // using a map function func (m Map) Render(rpt report.Report) RenderableNodes { @@ -133,11 +120,19 @@ func (m Map) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableID st output := report.EdgeMetadata{} for _, edge := range oldEdges { metadata := m.Renderer.EdgeMetadata(rpt, edge.src, edge.dst) - output.Merge(metadata) + output = output.Merge(metadata) } return output } +// LeafMap is a Renderer which produces a set of RenderableNodes from a report.Topology +// by using a map function and topology selector. +type LeafMap struct { + Selector report.TopologySelector + Mapper LeafMapFunc + Pseudo PseudoFunc +} + // Render transforms a given Report into a set of RenderableNodes, which // the UI will render collectively as a graph. Note that a RenderableNode will // always be rendered with other nodes, and therefore contains limited detail. @@ -234,8 +229,8 @@ func (m LeafMap) Render(rpt report.Report) RenderableNodes { // We propagate edge metadata to nodes on both ends of the edges. // TODO we should 'reverse' one end of the edge meta data - ingress -> egress etc. if md, ok := t.EdgeMetadatas[report.MakeEdgeID(srcNodeID, dstNodeID)]; ok { - srcRenderableNode.EdgeMetadata.Merge(md) - dstRenderableNode.EdgeMetadata.Merge(md) + srcRenderableNode.EdgeMetadata = srcRenderableNode.EdgeMetadata.Merge(md) + dstRenderableNode.EdgeMetadata = dstRenderableNode.EdgeMetadata.Merge(md) nodes[dstRenderableID] = dstRenderableNode } } @@ -268,12 +263,17 @@ func (m LeafMap) EdgeMetadata(rpt report.Report, srcRenderableID, dstRenderableI dst = mapped.ID } if src == srcRenderableID && dst == dstRenderableID { - metadata.Flatten(edgeMeta) + metadata = metadata.Flatten(edgeMeta) } } return metadata } +// FilterUnconnected is a Renderer which filters out unconnected nodes. +type FilterUnconnected struct { + Renderer +} + // Render produces a set of RenderableNodes given a Report func (f FilterUnconnected) Render(rpt report.Report) RenderableNodes { return OnlyConnected(f.Renderer.Render(rpt)) diff --git a/render/render_test.go b/render/render_test.go index 03a9ae232..8371edaa6 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -6,6 +6,7 @@ import ( "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" ) type mockRenderer struct { @@ -76,12 +77,12 @@ func TestMapRender2(t *testing.T) { "baz": {ID: "baz"}, }}, } - want := render.RenderableNodes{ + want := sterilize(render.RenderableNodes{ "bar": render.RenderableNode{ID: "bar"}, - } + }, false) have := mapper.Render(report.MakeReport()) if !reflect.DeepEqual(want, have) { - t.Errorf("want %+v, have %+v", want, have) + t.Error(test.Diff(want, have)) } } @@ -129,8 +130,8 @@ func TestMapEdge(t *testing.T) { } mapper := render.Map{ - MapFunc: func(nodes render.RenderableNode) (render.RenderableNode, bool) { - return render.RenderableNode{ID: "_" + nodes.ID}, true + MapFunc: func(n render.RenderableNode) (render.RenderableNode, bool) { + return render.RenderableNode{ID: "_" + n.ID}, true }, Renderer: render.LeafMap{ Selector: selector, @@ -143,7 +144,7 @@ func TestMapEdge(t *testing.T) { EgressPacketCount: newu64(1), EgressByteCount: newu64(2), }), mapper.EdgeMetadata(report.MakeReport(), "_foo", "_bar"); !reflect.DeepEqual(want, have) { - t.Errorf("want %+v, have %+v", want, have) + t.Error(test.Diff(want, have)) } } diff --git a/render/renderable_node.go b/render/renderable_node.go index 074ca3774..48168edc0 100644 --- a/render/renderable_node.go +++ b/render/renderable_node.go @@ -16,8 +16,8 @@ type RenderableNode struct { Adjacency report.IDList `json:"adjacency,omitempty"` // Node IDs (in the same topology domain) Origins report.IDList `json:"origins,omitempty"` // Core node IDs that contributed information - report.EdgeMetadata `json:"metadata"` // Numeric sums - report.NodeMetadata `json:"-"` // merged NodeMetadata of the nodes used to build this + report.EdgeMetadata `json:"metadata"` // Numeric sums + report.NodeMetadata `json:"XXXNODEMETADATA"` // merged NodeMetadata of the nodes used to build this // TODO ### } // RenderableNodes is a set of RenderableNodes @@ -56,8 +56,8 @@ func (rn *RenderableNode) Merge(other RenderableNode) { rn.Adjacency = rn.Adjacency.Merge(other.Adjacency) rn.Origins = rn.Origins.Merge(other.Origins) - rn.EdgeMetadata.Merge(other.EdgeMetadata) - rn.NodeMetadata.Merge(other.NodeMetadata) + rn.EdgeMetadata = rn.EdgeMetadata.Merge(other.EdgeMetadata) + rn.NodeMetadata = rn.NodeMetadata.Merge(other.NodeMetadata) } // NewRenderableNode makes a new RenderableNode @@ -80,8 +80,8 @@ func newDerivedNode(id string, node RenderableNode) RenderableNode { LabelMinor: "", Rank: "", Pseudo: node.Pseudo, - EdgeMetadata: node.EdgeMetadata, - Origins: node.Origins, + Origins: node.Origins.Copy(), + EdgeMetadata: node.EdgeMetadata.Copy(), NodeMetadata: report.MakeNodeMetadata(), } } @@ -105,8 +105,8 @@ func newDerivedPseudoNode(id, major string, node RenderableNode) RenderableNode LabelMinor: "", Rank: "", Pseudo: true, - EdgeMetadata: node.EdgeMetadata, - Origins: node.Origins, + Origins: node.Origins.Copy(), + EdgeMetadata: node.EdgeMetadata.Copy(), NodeMetadata: report.MakeNodeMetadata(), } } diff --git a/render/renderable_node_test.go b/render/renderable_node_test.go index efa66b545..f54362296 100644 --- a/render/renderable_node_test.go +++ b/render/renderable_node_test.go @@ -6,6 +6,7 @@ import ( "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" ) func TestMergeRenderableNodes(t *testing.T) { @@ -17,19 +18,45 @@ func TestMergeRenderableNodes(t *testing.T) { "bar": render.RenderableNode{ID: "bar"}, "baz": render.RenderableNode{ID: "baz"}, } - - want := render.RenderableNodes{ + want := sterilize(render.RenderableNodes{ "foo": render.RenderableNode{ID: "foo"}, "bar": render.RenderableNode{ID: "bar"}, "baz": render.RenderableNode{ID: "baz"}, - } + }, false) nodes1.Merge(nodes2) - - if !reflect.DeepEqual(want, nodes1) { - t.Errorf("want %+v, have %+v", want, nodes1) + if have := sterilize(nodes1, false); !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } +func sterilize(r render.RenderableNodes, destructive bool) render.RenderableNodes { + // Since introducing new map fields to the report.NodeMetadata type, its + // zero value is •not valid• -- every time you need one, you need to use + // the report.MakeNodeMetadata constructor. (Similarly, but not exactly + // the same, is that a zero-value Adjacency is not the same as a created + // but empty Adjacency.) + // + // But we're not doing this in tests. So this function sterilizes invalid + // RenderableNodes by fixing all nil Metadata fields. The proper fix + // involves lots of annoying changes to instantiation. + // + // The extra destructive parameter is to support a historical test use + // case where we explicitly don't compare node metadata. + for id, n := range r { + if n.Adjacency == nil { + n.Adjacency = report.IDList{} + } + if destructive || n.NodeMetadata.Metadata == nil { + n.NodeMetadata.Metadata = map[string]string{} + } + if destructive || n.NodeMetadata.Counters == nil { + n.NodeMetadata.Counters = map[string]int{} + } + r[id] = n + } + return r +} + func TestMergeRenderableNode(t *testing.T) { node1 := render.RenderableNode{ ID: "foo", @@ -49,19 +76,19 @@ func TestMergeRenderableNode(t *testing.T) { Adjacency: report.MakeIDList("a2"), Origins: report.MakeIDList("o2"), } - want := render.RenderableNode{ - ID: "foo", - LabelMajor: "major", - LabelMinor: "minor", - Rank: "rank", - Pseudo: false, - Adjacency: report.MakeIDList("a1", "a2"), - Origins: report.MakeIDList("o1", "o2"), + ID: "foo", + LabelMajor: "major", + LabelMinor: "minor", + Rank: "rank", + Pseudo: false, + Adjacency: report.MakeIDList("a1", "a2"), + Origins: report.MakeIDList("o1", "o2"), + NodeMetadata: report.MakeNodeMetadata(), + EdgeMetadata: report.EdgeMetadata{}, } node1.Merge(node2) - - if !reflect.DeepEqual(want, node1) { - t.Errorf("want %+v, have %+v", want, node1) + if have := node1; !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } diff --git a/render/theinternet_test.go b/render/theinternet_test.go index 283e3a963..fad04b5c3 100644 --- a/render/theinternet_test.go +++ b/render/theinternet_test.go @@ -12,11 +12,16 @@ import ( ) func TestReportLocalNetworks(t *testing.T) { - r := report.MakeReport() - r.Merge(report.Report{Host: report.Topology{NodeMetadatas: report.NodeMetadatas{ - "nonets": report.MakeNodeMetadata(), - "foo": report.MakeNodeMetadataWith(map[string]string{host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33"}), - }}}) + r := report.MakeReport().Merge(report.Report{ + Host: report.Topology{ + NodeMetadatas: report.NodeMetadatas{ + "nonets": report.MakeNodeMetadata(), + "foo": report.MakeNodeMetadataWith(map[string]string{ + host.LocalNetworks: "10.0.0.1/8 192.168.1.1/24 10.0.0.1/8 badnet/33", + }), + }, + }, + }) want := report.Networks([]*net.IPNet{ mustParseCIDR("10.0.0.1/8"), mustParseCIDR("192.168.1.1/24"), diff --git a/render/topologies_test.go b/render/topologies_test.go index f8f0402f8..f46ec0061 100644 --- a/render/topologies_test.go +++ b/render/topologies_test.go @@ -6,55 +6,45 @@ import ( "github.com/weaveworks/scope/render" "github.com/weaveworks/scope/render/expected" - "github.com/weaveworks/scope/report" "github.com/weaveworks/scope/test" ) -func trimNodeMetadata(rns render.RenderableNodes) render.RenderableNodes { - result := render.RenderableNodes{} - for id, rn := range rns { - rn.NodeMetadata = report.MakeNodeMetadata() - result[id] = rn - } - return result -} - func TestProcessRenderer(t *testing.T) { - have := render.ProcessRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedProcesses, have) { - t.Error(test.Diff(expected.RenderedProcesses, have)) + have := sterilize(render.ProcessRenderer.Render(test.Report), true) + want := expected.RenderedProcesses + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestProcessNameRenderer(t *testing.T) { - have := render.ProcessNameRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedProcessNames, have) { - t.Error(test.Diff(expected.RenderedProcessNames, have)) + have := sterilize(render.ProcessNameRenderer.Render(test.Report), true) + want := expected.RenderedProcessNames + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestContainerRenderer(t *testing.T) { - have := render.ContainerRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedContainers, have) { - t.Error(test.Diff(expected.RenderedContainers, have)) + have := sterilize(render.ContainerRenderer.Render(test.Report), true) + want := expected.RenderedContainers + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestContainerImageRenderer(t *testing.T) { - have := render.ContainerImageRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedContainerImages, have) { - t.Error(test.Diff(expected.RenderedContainerImages, have)) + have := sterilize(render.ContainerImageRenderer.Render(test.Report), true) + want := expected.RenderedContainerImages + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } func TestHostRenderer(t *testing.T) { - have := render.HostRenderer.Render(test.Report) - have = trimNodeMetadata(have) - if !reflect.DeepEqual(expected.RenderedHosts, have) { - t.Error(test.Diff(expected.RenderedHosts, have)) + have := sterilize(render.HostRenderer.Render(test.Report), true) + want := expected.RenderedHosts + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) } } diff --git a/report/merge_test.go b/report/merge_test.go index 1dab3a92f..11674bd7c 100644 --- a/report/merge_test.go +++ b/report/merge_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test" ) const ( @@ -191,11 +192,30 @@ func TestMergeEdgeMetadatas(t *testing.T) { }, } { if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { - t.Errorf("%s: want\n\t%#v, have\n\t%#v", name, c.want, have) + t.Errorf("%s:\n%s", name, test.Diff(c.want, have)) } } } +func TestFlattenEdgeMetadata(t *testing.T) { + have := (report.EdgeMetadata{ + EgressPacketCount: newu64(1), + MaxConnCountTCP: newu64(2), + }).Flatten(report.EdgeMetadata{ + EgressPacketCount: newu64(4), + EgressByteCount: newu64(8), + MaxConnCountTCP: newu64(16), + }) + want := report.EdgeMetadata{ + EgressPacketCount: newu64(1 + 4), + EgressByteCount: newu64(8), + MaxConnCountTCP: newu64(2 + 16), // flatten should sum MaxConnCountTCP + } + if !reflect.DeepEqual(want, have) { + t.Error(test.Diff(want, have)) + } +} + func TestMergeNodeMetadatas(t *testing.T) { for name, c := range map[string]struct { a, b, want report.NodeMetadatas From 6d2252f60418f9d62aa4da4c6ad8ef656a97ec70 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Wed, 26 Aug 2015 11:55:21 +0100 Subject: [PATCH 6/7] app: fixes for new merge semantics --- app/api_topology_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/app/api_topology_test.go b/app/api_topology_test.go index 0a8df90c2..56fea5b61 100644 --- a/app/api_topology_test.go +++ b/app/api_topology_test.go @@ -16,18 +16,17 @@ import ( "github.com/weaveworks/scope/test" ) -func fixNodeMetadatas(nodes render.RenderableNodes) render.RenderableNodes { - result := make(render.RenderableNodes, len(nodes)) - for id, node := range nodes { - if node.NodeMetadata.Metadata == nil { - node.NodeMetadata.Metadata = map[string]string{} +// A copy of sanitize from the render package. +func sanitize(nodes render.RenderableNodes) render.RenderableNodes { + for id, n := range nodes { + if n.Adjacency == nil { + n.Adjacency = report.IDList{} } - if node.NodeMetadata.Counters == nil { - node.NodeMetadata.Counters = map[string]int{} - } - result[id] = node + n.NodeMetadata.Metadata = map[string]string{} + n.NodeMetadata.Counters = map[string]int{} + nodes[id] = n } - return result + return nodes } func TestAll(t *testing.T) { @@ -74,7 +73,7 @@ func TestAPITopologyContainers(t *testing.T) { t.Fatal(err) } - if want, have := expected.RenderedContainers, fixNodeMetadatas(topo.Nodes); !reflect.DeepEqual(want, have) { + if want, have := expected.RenderedContainers, sanitize(topo.Nodes); !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } } @@ -122,7 +121,7 @@ func TestAPITopologyHosts(t *testing.T) { t.Fatal(err) } - if want, have := expected.RenderedHosts, fixNodeMetadatas(topo.Nodes); !reflect.DeepEqual(want, have) { + if want, have := expected.RenderedHosts, sanitize(topo.Nodes); !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) } } From fcecb922069cf8109eba99849788e3ff4568d578 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Thu, 27 Aug 2015 16:54:06 +0200 Subject: [PATCH 7/7] Ugh. --- probe/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probe/main.go b/probe/main.go index d636b386a..9a27d699e 100644 --- a/probe/main.go +++ b/probe/main.go @@ -183,7 +183,7 @@ func main() { if err != nil { log.Printf("error generating report: %v", err) } - r.Merge(newReport) + r = r.Merge(newReport) } r = Apply(r, taggers)