From 42be036428ec9d01329e1cdbe5e9d0a8a0ff4d46 Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Tue, 25 Aug 2015 17:52:20 +0100 Subject: [PATCH] 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.