Move Counters into Latest

The only place Counters are used is in rendering, for the number of
nodes under a topology, so the overhead of holding a unique data
structure in every Node is unwarranted.

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
 }
 

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
 	}

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],

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
 }

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

report/counters.go

@@ -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")
-}

report/counters_internal_test.go

@@ -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)
-		}
-	}
-}

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"

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
 }
 

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),

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))
+	}
+}

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)
 		}
 	}