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.

Counters are not set in the probe, so we don't need any
backwards-compatibility in report decoding. Similarly they are not set
until after all nodes are merged, so we don't need that logic.
This commit is contained in:
Bryan Boreham
2019-10-20 10:16:36 +00:00
parent 1ce1ecad7c
commit c03aeb5d43
13 changed files with 95 additions and 363 deletions

View File

@@ -326,7 +326,7 @@ 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).
AddCounter(n.Topology, 1)
return node
}

View File

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

View File

@@ -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}).
AddCounter(report.Process, 2).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Client54001NodeID],
RenderedEndpoints[fixture.Client54002NodeID],
@@ -133,14 +133,14 @@ var (
fixture.ServerName: processNameNode(fixture.ServerName).
WithLatests(map[string]string{process.Name: fixture.ServerName}).
WithCounters(map[string]int{report.Process: 1}).
AddCounter(report.Process, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Server80NodeID],
RenderedProcesses[fixture.ServerProcessNodeID],
)),
fixture.NonContainerName: processNameNode(fixture.NonContainerName, render.OutgoingInternetID).
WithCounters(map[string]int{report.Process: 1}).
AddCounter(report.Process, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.NonContainerNodeID],
RenderedProcesses[fixture.NonContainerProcessNodeID],
@@ -191,9 +191,7 @@ var (
WithLatests(map[string]string{
docker.ContainerHostname: fixture.ClientContainerHostname,
}).
WithCounters(map[string]int{
report.Container: 1,
}).
AddCounter(report.Container, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Client54001NodeID],
RenderedEndpoints[fixture.Client54002NodeID],
@@ -206,7 +204,7 @@ var (
WithLatests(map[string]string{
docker.ContainerHostname: fixture.ServerContainerHostname,
}).
WithCounters(map[string]int{report.Container: 1}).
AddCounter(report.Container, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Server80NodeID],
RenderedProcesses[fixture.ServerProcessNodeID],
@@ -228,9 +226,7 @@ var (
docker.ImageID: fixture.ClientContainerImageID,
docker.ImageName: fixture.ClientContainerImageName,
}).
WithCounters(map[string]int{
report.Container: 1,
}).
AddCounter(report.Container, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Client54001NodeID],
RenderedEndpoints[fixture.Client54002NodeID],
@@ -240,7 +236,7 @@ var (
)),
ServerContainerImageNodeID: containerImage(ServerContainerImageNodeID).
WithCounters(map[string]int{report.Container: 1}).
AddCounter(report.Container, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Server80NodeID],
RenderedProcesses[fixture.ServerProcessNodeID],
@@ -258,11 +254,11 @@ var (
RenderedEndpoints[fixture.NonContainerNodeID],
RenderedProcesses[fixture.NonContainerProcessNodeID],
)).
WithCounters(map[string]int{render.Pseudo: 1})
AddCounter(render.Pseudo, 1)
RenderedPods = report.Nodes{
fixture.ClientPodNodeID: pod(fixture.ClientPodNodeID, fixture.ServerPodNodeID).
WithCounters(map[string]int{report.Container: 1}).
AddCounter(report.Container, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Client54001NodeID],
RenderedEndpoints[fixture.Client54002NodeID],
@@ -272,7 +268,7 @@ var (
)),
fixture.ServerPodNodeID: pod(fixture.ServerPodNodeID).
WithCounters(map[string]int{report.Container: 1}).
AddCounter(report.Container, 1).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Server80NodeID],
RenderedProcesses[fixture.ServerProcessNodeID],
@@ -329,7 +325,7 @@ var (
RenderedPodServices = report.Nodes{
fixture.ServiceNodeID: service(fixture.ServiceNodeID, fixture.ServiceNodeID).
WithCounters(map[string]int{report.Pod: 2}).
AddCounter(report.Pod, 2).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Client54001NodeID],
RenderedEndpoints[fixture.Client54002NodeID],
@@ -353,12 +349,10 @@ var (
WithLatests(map[string]string{
report.HostName: fixture.ClientHostName,
}).
WithCounters(map[string]int{
report.Container: 1,
report.ContainerImage: 1,
report.Pod: 1,
report.Process: 2,
}).
AddCounter(report.Container, 1).
AddCounter(report.ContainerImage, 1).
AddCounter(report.Pod, 1).
AddCounter(report.Process, 2).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Client54001NodeID],
RenderedEndpoints[fixture.Client54002NodeID],
@@ -370,12 +364,10 @@ var (
)),
fixture.ServerHostNodeID: hostNode(fixture.ServerHostNodeID, render.OutgoingInternetID).
WithCounters(map[string]int{
report.Container: 1,
report.ContainerImage: 1,
report.Pod: 1,
report.Process: 2,
}).
AddCounter(report.Container, 1).
AddCounter(report.ContainerImage, 1).
AddCounter(report.Pod, 1).
AddCounter(report.Process, 2).
WithChildren(report.MakeNodeSet(
RenderedEndpoints[fixture.Server80NodeID],
RenderedEndpoints[fixture.NonContainerNodeID],

View File

@@ -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.AddCounter(m.Topology, 1)
}
ret.nodes[id] = result
}

View File

@@ -13,6 +13,7 @@ import (
type bcNode struct {
Node
LatestControls map[string]nodeControlDataLatestEntry `json:"latestControls,omitempty"`
Counters map[string]int `json:"counters,omitempty"`
}
type nodeControlDataLatestEntry struct {
@@ -44,6 +45,10 @@ func (n *Node) CodecDecodeSelf(decoder *codec.Decoder) {
}
n.Latest = n.Latest.Set(NodeActiveControls, ts, 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.AddCounter(k, v)
}
}
type _Node Node // just so we don't recurse inside CodecEncodeSelf

View File

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

View File

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

View File

@@ -4,6 +4,7 @@ package report
const (
// Node
NodeActiveControls = "active_controls"
CounterPrefix = "count_"
// probe/endpoint
ReverseDNSNames = "reverse_dns_names"
SnoopedDNSNames = "snooped_dns_names"

View File

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

View File

@@ -1,6 +1,7 @@
package report
import (
"strconv"
"strings"
"time"
@@ -13,7 +14,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:"latest,omitempty"`
@@ -26,7 +26,6 @@ type Node struct {
func MakeNode(id string) Node {
return Node{
ID: id,
Counters: MakeCounters(),
Sets: MakeSets(),
Adjacency: MakeIDList(),
Latest: MakeStringLatestMap(),
@@ -80,10 +79,25 @@ func (n Node) WithLatest(k string, ts time.Time, 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
}
// AddCounter 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) AddCounter(k string, value int) Node {
name := CounterPrefix + k
if prevValue, found := n.LookupCounter(k); found {
value += prevValue
}
return n.WithLatest(name, mtime.Now(), strconv.Itoa(value))
}
// WithSet returns a fresh copy of n, with set merged in at key.
@@ -175,7 +189,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),

View File

@@ -146,33 +146,37 @@ 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,
}),
},
},
// Note we previously tested that counters merged by adding,
// but that was a bug: merges must be idempotent.
// Counters merge like other 'latest' values now.
} {
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) {
mtime.NowForce(time.Now())
defer mtime.NowReset()
a := report.MakeNode("1").
AddCounter("a", 13).
AddCounter("b", 57).
AddCounter("c", 89)
b := a.
AddCounter("a", 78).
AddCounter("b", 3).
AddCounter("d", 47)
want := report.MakeNode("1").
AddCounter("a", 91).
AddCounter("b", 60).
AddCounter("c", 89).
AddCounter("d", 47)
if have := b; !reflect.DeepEqual(want, have) {
t.Errorf("Counters: %s", test.Diff(want, have))
}
}

View File

@@ -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").AddCounter("foo", 1)
if value, _ := node.LookupCounter("foo"); value != 1 {
t.Errorf("want foo, have %d", value)
}
}

View File

@@ -1,6 +1,9 @@
package utils
import (
"strings"
"time"
"github.com/weaveworks/scope/report"
)
@@ -27,6 +30,11 @@ func PruneNode(node report.Node) report.Node {
WithTopology(node.Topology).
WithAdjacent(node.Adjacency...).
WithChildren(prunedChildren)
prunedNode.Counters = node.Counters
// Copy counters across, but with zero timestamp so they compare equal
node.Latest.ForEach(func(k string, _ time.Time, v string) {
if strings.HasPrefix(k, report.CounterPrefix) {
prunedNode.Latest = prunedNode.Latest.Set(k, time.Time{}, v)
}
})
return prunedNode
}