diff --git a/render/container.go b/render/container.go index 5e78fddfa..533f30c93 100644 --- a/render/container.go +++ b/render/container.go @@ -304,8 +304,8 @@ func MapContainer2ContainerImage(n report.Node) report.Node { // Add container id key to the counters, which will later be // counted to produce the minor label id := report.MakeContainerImageNodeID(imageID) - result := NewDerivedNode(id, n).WithTopology(report.ContainerImage) - result.Counters = result.Counters.Add(n.Topology, 1) + result := NewDerivedNode(id, n).WithTopology(report.ContainerImage). + WithCounter(n.Topology, 1) return result } @@ -354,8 +354,8 @@ func MapContainer2Hostname(n report.Node) report.Node { return report.Node{} } - node := NewDerivedNode(id, n).WithTopology(containerHostnameTopology) - node.Counters = node.Counters.Add(n.Topology, 1) + node := NewDerivedNode(id, n).WithTopology(containerHostnameTopology). + WithCounter(n.Topology, 1) return node } diff --git a/render/detailed/summary.go b/render/detailed/summary.go index 6f42f79f8..e18cefd5a 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -164,7 +164,7 @@ func MakeNodeSummary(rc RenderContext, n report.Node) (NodeSummary, bool) { Adjacency: n.Adjacency, } // Only include metadata, metrics, tables when it's not a group node - if _, ok := n.Counters.Lookup(n.Topology); !ok { + if _, ok := n.LookupCounter(n.Topology); !ok { if topology, ok := rc.Topology(n.Topology); ok { summary.Metadata = topology.MetadataTemplates.MetadataRows(n) summary.Metrics = topology.MetricTemplates.MetricRows(n) @@ -292,7 +292,7 @@ func containerImageNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSu // heuristic regexp match we cannot tell the difference. base.Label, _ = report.ParseContainerImageNodeID(n.ID) } - base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers") + base.LabelMinor = pluralize(n, report.Container, "container", "containers") base.Rank = base.Label base.Stack = true return base @@ -314,7 +314,7 @@ func addKubernetesLabelAndRank(base BasicNodeSummary, n report.Node) BasicNodeSu func podNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary { base = addKubernetesLabelAndRank(base, n) - base.LabelMinor = pluralize(n.Counters, report.Container, "container", "containers") + base.LabelMinor = pluralize(n, report.Container, "container", "containers") return base } @@ -331,7 +331,7 @@ func podGroupNodeSummary(base BasicNodeSummary, n report.Node) BasicNodeSummary base.Stack = true // NB: pods are the highest aggregation level for which we display // counts. - count := pluralize(n.Counters, report.Pod, "pod", "pods") + count := pluralize(n, report.Pod, "pod", "pods") if typeName, ok := podGroupNodeTypeName[n.Topology]; ok { base.LabelMinor = fmt.Sprintf("%s of %s", typeName, count) } else { @@ -423,7 +423,7 @@ func groupNodeSummary(base BasicNodeSummary, r report.Report, n report.Node) Bas base.Shape = t.GetShape() base.Tag = t.Tag if t.Label != "" { - base.LabelMinor = pluralize(n.Counters, topology, t.Label, t.LabelPlural) + base.LabelMinor = pluralize(n, topology, t.Label, t.LabelPlural) } } } @@ -431,8 +431,8 @@ func groupNodeSummary(base BasicNodeSummary, r report.Report, n report.Node) Bas return base } -func pluralize(counters report.Counters, key, singular, plural string) string { - c, ok := counters.Lookup(key) +func pluralize(n report.Node, key, singular, plural string) string { + c, ok := n.LookupCounter(key) if !ok { c = 0 } diff --git a/render/expected/expected.go b/render/expected/expected.go index f5f478de0..2ebd0f5fc 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -123,7 +123,7 @@ var ( RenderedProcessNames = report.Nodes{ fixture.Client1Name: processNameNode(fixture.Client1Name, fixture.ServerName). WithLatests(map[string]string{process.Name: fixture.Client1Name}). - WithCounters(map[string]int{report.Process: 2}). + WithCounter(report.Process, 2). WithChildren(report.MakeNodeSet( RenderedEndpoints[fixture.Client54001NodeID], RenderedEndpoints[fixture.Client54002NodeID], @@ -133,7 +133,7 @@ var ( fixture.ServerName: processNameNode(fixture.ServerName). WithLatests(map[string]string{process.Name: fixture.ServerName}). - WithCounters(map[string]int{report.Process: 1}). + WithCounter(report.Process, 1). WithChildren(report.MakeNodeSet( RenderedEndpoints[fixture.Server80NodeID], RenderedProcesses[fixture.ServerProcessNodeID], @@ -190,9 +190,7 @@ var ( WithLatests(map[string]string{ docker.ContainerHostname: fixture.ClientContainerHostname, }). - WithCounters(map[string]int{ - report.Container: 1, - }). + WithCounter(report.Container, 1). WithChildren(report.MakeNodeSet( RenderedEndpoints[fixture.Client54001NodeID], RenderedEndpoints[fixture.Client54002NodeID], @@ -226,9 +224,7 @@ var ( docker.ImageID: fixture.ClientContainerImageID, docker.ImageName: fixture.ClientContainerImageName, }). - WithCounters(map[string]int{ - report.Container: 1, - }). + WithCounter(report.Container, 1). WithChildren(report.MakeNodeSet( RenderedEndpoints[fixture.Client54001NodeID], RenderedEndpoints[fixture.Client54002NodeID], diff --git a/render/render.go b/render/render.go index a3c3d1b49..74c2b826f 100644 --- a/render/render.go +++ b/render/render.go @@ -195,7 +195,7 @@ func (ret *joinResults) addUnmappedChild(m report.Node, id string, topology stri } result.Children.UnsafeAdd(m) if m.Topology != report.Endpoint { // optimisation: we never look at endpoint counts - result.Counters = result.Counters.Add(m.Topology, 1) + result = result.WithCounter(m.Topology, 1) } ret.nodes[id] = result } diff --git a/report/backcompat.go b/report/backcompat.go index 57387febf..d5bb2c1e1 100644 --- a/report/backcompat.go +++ b/report/backcompat.go @@ -15,6 +15,7 @@ type bcNode struct { Node LatestControls map[string]nodeControlDataLatestEntry `json:"latestControls,omitempty"` OldStringMap map[string]oldStringEntry `json:"latest,omitempty"` + Counters map[string]int `json:"counters,omitempty"` } type nodeControlDataLatestEntry struct { @@ -53,6 +54,10 @@ func (n *Node) CodecDecodeSelf(decoder *codec.Decoder) { } n.Latest = n.Latest.Set(NodeActiveControls, strings.Join(cs, ScopeDelim)) } + // Counters were not generated in the Scope probe, but decode them here in case a plugin used them. + for k, v := range in.Counters { + *n = n.WithCounter(k, v) + } } type _Node Node // just so we don't recurse inside CodecEncodeSelf diff --git a/report/counters.go b/report/counters.go deleted file mode 100644 index 8bb494dcd..000000000 --- a/report/counters.go +++ /dev/null @@ -1,137 +0,0 @@ -package report - -import ( - "bytes" - "fmt" - "reflect" - - "github.com/ugorji/go/codec" - "github.com/weaveworks/ps" -) - -// Counters is a string->int map. -type Counters struct { - psMap ps.Map -} - -var emptyCounters = Counters{ps.NewMap()} - -// MakeCounters returns EmptyCounters -func MakeCounters() Counters { - return emptyCounters -} - -// Add value to the counter 'key' -func (c Counters) Add(key string, value int) Counters { - if c.psMap == nil { - c = emptyCounters - } - if existingValue, ok := c.psMap.Lookup(key); ok { - value += existingValue.(int) - } - return Counters{ - c.psMap.Set(key, value), - } -} - -// Lookup the counter 'key' -func (c Counters) Lookup(key string) (int, bool) { - if c.psMap != nil { - existingValue, ok := c.psMap.Lookup(key) - if ok { - return existingValue.(int), true - } - } - return 0, false -} - -// Size returns the number of counters -func (c Counters) Size() int { - if c.psMap == nil { - return 0 - } - return c.psMap.Size() -} - -// Merge produces a fresh Counters, container the keys from both inputs. When -// both inputs container the same key, the latter value is used. -func (c Counters) Merge(other Counters) Counters { - var ( - cSize = c.Size() - otherSize = other.Size() - output = c.psMap - iter = other.psMap - ) - switch { - case cSize == 0: - return other - case otherSize == 0: - return c - case cSize < otherSize: - output, iter = iter, output - } - iter.ForEach(func(key string, otherVal interface{}) { - if val, ok := output.Lookup(key); ok { - output = output.Set(key, otherVal.(int)+val.(int)) - } else { - output = output.Set(key, otherVal) - } - }) - return Counters{output} -} - -// String serializes Counters into a string. -func (c Counters) String() string { - buf := bytes.NewBufferString("{") - prefix := "" - for _, key := range mapKeys(c.psMap) { - val, _ := c.psMap.Lookup(key) - fmt.Fprintf(buf, "%s%s: %d", prefix, key, val.(int)) - prefix = ", " - } - fmt.Fprintf(buf, "}") - return buf.String() -} - -// DeepEqual tests equality with other Counters -func (c Counters) DeepEqual(d Counters) bool { - return mapEqual(c.psMap, d.psMap, reflect.DeepEqual) -} - -func (c Counters) fromIntermediate(in map[string]int) Counters { - out := ps.NewMap() - for k, v := range in { - out = out.Set(k, v) - } - return Counters{out} -} - -// CodecEncodeSelf implements codec.Selfer -func (c *Counters) CodecEncodeSelf(encoder *codec.Encoder) { - mapWrite(c.psMap, encoder, func(encoder *codec.Encoder, val interface{}) { - i := val.(int) - encoder.Encode(i) - }) -} - -// CodecDecodeSelf implements codec.Selfer -func (c *Counters) CodecDecodeSelf(decoder *codec.Decoder) { - out := mapRead(decoder, func(isNil bool) interface{} { - var value int - if !isNil { - decoder.Decode(&value) - } - return value - }) - *c = Counters{out} -} - -// MarshalJSON shouldn't be used, use CodecEncodeSelf instead -func (Counters) MarshalJSON() ([]byte, error) { - panic("MarshalJSON shouldn't be used, use CodecEncodeSelf instead") -} - -// UnmarshalJSON shouldn't be used, use CodecDecodeSelf instead -func (*Counters) UnmarshalJSON(b []byte) error { - panic("UnmarshalJSON shouldn't be used, use CodecDecodeSelf instead") -} diff --git a/report/counters_internal_test.go b/report/counters_internal_test.go deleted file mode 100644 index 36fe42a1b..000000000 --- a/report/counters_internal_test.go +++ /dev/null @@ -1,152 +0,0 @@ -package report - -import ( - "bytes" - "testing" - - "github.com/ugorji/go/codec" - - "github.com/weaveworks/common/test" - "github.com/weaveworks/scope/test/reflect" -) - -func TestCountersAdd(t *testing.T) { - have := MakeCounters(). - Add("foo", 1). - Add("foo", 2) - if v, ok := have.Lookup("foo"); !ok || v != 3 { - t.Errorf("foo != 3") - } - if v, ok := have.Lookup("bar"); ok || v != 0 { - t.Errorf("bar != nil") - } -} - -func TestCountersDeepEquals(t *testing.T) { - want := MakeCounters(). - Add("foo", 3) - have := MakeCounters(). - Add("foo", 3) - if !reflect.DeepEqual(want, have) { - t.Errorf(test.Diff(want, have)) - } - notequal := MakeCounters(). - Add("foo", 4) - if reflect.DeepEqual(want, notequal) { - t.Errorf(test.Diff(want, have)) - } -} - -func TestCountersNil(t *testing.T) { - want := Counters{} - if want.Size() != 0 { - t.Errorf("nil.Size != 0") - } - if v, ok := want.Lookup("foo"); ok || v != 0 { - t.Errorf("nil.Lookup != false") - } - have := want.Add("foo", 1) - if v, ok := have.Lookup("foo"); !ok || v != 1 { - t.Errorf("nil.Add failed") - } - if have2 := want.Merge(have); !reflect.DeepEqual(have, have2) { - t.Errorf(test.Diff(have, have2)) - } -} - -func TestCountersMerge(t *testing.T) { - for name, c := range map[string]struct { - a, b, want Counters - }{ - "Empty a": { - a: MakeCounters(), - b: MakeCounters(). - Add("foo", 1), - want: MakeCounters(). - Add("foo", 1), - }, - "Empty b": { - a: MakeCounters(). - Add("foo", 1), - b: MakeCounters(), - want: MakeCounters(). - Add("foo", 1), - }, - "Disjoin a & b": { - a: MakeCounters(). - Add("foo", 1), - b: MakeCounters(). - Add("bar", 2), - want: MakeCounters(). - Add("foo", 1). - Add("bar", 2), - }, - "Overlapping a & b": { - a: MakeCounters(). - Add("foo", 1), - b: MakeCounters(). - Add("foo", 2), - want: MakeCounters(). - Add("foo", 3), - }, - } { - if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { - t.Errorf("%s:\n%s", name, test.Diff(c.want, have)) - } - } -} - -func TestCountersEncoding(t *testing.T) { - want := MakeCounters(). - Add("foo", 1). - Add("bar", 2) - - { - - for _, h := range []codec.Handle{ - codec.Handle(&codec.MsgpackHandle{}), - codec.Handle(&codec.JsonHandle{}), - } { - buf := &bytes.Buffer{} - encoder := codec.NewEncoder(buf, h) - want.CodecEncodeSelf(encoder) - decoder := codec.NewDecoder(buf, h) - have := MakeCounters() - have.CodecDecodeSelf(decoder) - if !reflect.DeepEqual(want, have) { - t.Error(test.Diff(want, have)) - } - } - } - -} - -func TestCountersString(t *testing.T) { - { - var c Counters - have := c.String() - want := `{}` - if want != have { - t.Errorf("Expected: %s, Got %s", want, have) - } - } - - { - have := MakeCounters().String() - want := `{}` - if want != have { - t.Errorf("Expected: %s, Got %s", want, have) - } - } - - { - have := MakeCounters(). - Add("foo", 1). - Add("bar", 2).String() - - want := `{bar: 2, foo: 1}` - if want != have { - t.Errorf("Expected: %s, Got %s", want, have) - } - } -} diff --git a/report/map_keys.go b/report/map_keys.go index e8bfac3f8..cb5dfcc12 100644 --- a/report/map_keys.go +++ b/report/map_keys.go @@ -4,6 +4,7 @@ package report const ( // Node NodeActiveControls = "active_controls" + CounterPrefix = "count_" // probe/endpoint ReverseDNSNames = "reverse_dns_names" SnoopedDNSNames = "snooped_dns_names" diff --git a/report/metadata_template.go b/report/metadata_template.go index a6a446463..40e9d6182 100644 --- a/report/metadata_template.go +++ b/report/metadata_template.go @@ -71,7 +71,7 @@ func fromSets(n Node, key string) (string, bool) { } func fromCounters(n Node, key string) (string, bool) { - val, ok := n.Counters.Lookup(key) + val, ok := n.LookupCounter(key) return strconv.Itoa(val), ok } diff --git a/report/node.go b/report/node.go index 1c8ef9638..da2ce5ad9 100644 --- a/report/node.go +++ b/report/node.go @@ -1,6 +1,7 @@ package report import ( + "strconv" "strings" ) @@ -10,7 +11,6 @@ import ( type Node struct { ID string `json:"id,omitempty"` Topology string `json:"topology,omitempty"` - Counters Counters `json:"counters,omitempty"` Sets Sets `json:"sets,omitempty"` Adjacency IDList `json:"adjacency,omitempty"` Latest StringLatestMap `json:"strs,omitempty"` @@ -23,7 +23,6 @@ type Node struct { func MakeNode(id string) Node { return Node{ ID: id, - Counters: MakeCounters(), Sets: MakeSets(), Adjacency: MakeIDList(), Latest: MakeStringLatestMap(), @@ -77,10 +76,26 @@ func (n Node) WithLatest(k string, v string) Node { return n } -// WithCounters returns a fresh copy of n, with Counters c merged in. -func (n Node) WithCounters(c map[string]int) Node { - n.Counters = n.Counters.Merge(Counters{}.fromIntermediate(c)) - return n +// LookupCounter returns the value of a counter +// (counters are stored as strings, to keep the data structure simple) +func (n Node) LookupCounter(k string) (value int, found bool) { + name := CounterPrefix + k + var str string + if str, found = n.Latest.Lookup(name); found { + value, _ = strconv.Atoi(str) + } + return value, found +} + +// WithCounter returns a fresh copy of n, with Counter c added in. +// (counters are stored as strings, to keep the data structure simple) +func (n Node) WithCounter(k string, value int) Node { + name := CounterPrefix + k + if prevStr, found := n.Latest.Lookup(name); found { + prevValue, _ := strconv.Atoi(prevStr) + value += prevValue + } + return n.WithLatest(name, strconv.Itoa(value)) } // WithSet returns a fresh copy of n, with set merged in at key. @@ -173,7 +188,6 @@ func (n Node) Merge(other Node) Node { return Node{ ID: id, Topology: topology, - Counters: n.Counters.Merge(other.Counters), Sets: n.Sets.Merge(other.Sets), Adjacency: n.Adjacency.Merge(other.Adjacency), Latest: n.Latest.Merge(other.Latest), diff --git a/report/node_test.go b/report/node_test.go index 53f22ac22..322896c8a 100644 --- a/report/node_test.go +++ b/report/node_test.go @@ -130,33 +130,31 @@ func TestMergeNodes(t *testing.T) { }), }, }, - "Counters": { - a: report.Nodes{ - "1": report.MakeNode("1").WithCounters(map[string]int{ - "a": 13, - "b": 57, - "c": 89, - }), - }, - b: report.Nodes{ - "1": report.MakeNode("1").WithCounters(map[string]int{ - "a": 78, - "b": 3, - "d": 47, - }), - }, - want: report.Nodes{ - "1": report.MakeNode("1").WithCounters(map[string]int{ - "a": 91, - "b": 60, - "c": 89, - "d": 47, - }), - }, - }, } { if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { t.Errorf("%s: %s", name, test.Diff(c.want, have)) } } } + +func TestCounters(t *testing.T) { + a := report.MakeNode("1"). + WithCounter("a", 13). + WithCounter("b", 57). + WithCounter("c", 89) + + b := a. + WithCounter("a", 78). + WithCounter("b", 3). + WithCounter("d", 47) + + want := report.MakeNode("1"). + WithCounter("a", 91). + WithCounter("b", 60). + WithCounter("c", 89). + WithCounter("d", 47) + + if have := b; !reflect.DeepEqual(want, have) { + t.Errorf("Counters: %s", test.Diff(want, have)) + } +} diff --git a/report/report_test.go b/report/report_test.go index 80717e7f6..c361cdada 100644 --- a/report/report_test.go +++ b/report/report_test.go @@ -57,10 +57,8 @@ func TestNode(t *testing.T) { } } { - node := report.MakeNode("foo").WithCounters( - map[string]int{"foo": 1}, - ) - if value, _ := node.Counters.Lookup("foo"); value != 1 { + node := report.MakeNode("foo").WithCounter("foo", 1) + if value, _ := node.LookupCounter("foo"); value != 1 { t.Errorf("want foo, have %d", value) } }