From 07287c6ac02c495f96695c87086eb7c4e5ad9599 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 16 Oct 2019 07:52:45 +0000 Subject: [PATCH] Move timestamp up from 'Latest' string map to Report level We never merge multiple string values in a single node from different sources, so it's sufficient to take the value from the latest report. --- app/api_report.go | 5 +- probe/docker/container_test.go | 10 +- probe/plugins/registry_internal_test.go | 2 + probe/probe.go | 2 + render/detailed/connections.go | 4 +- render/process.go | 2 +- report/backcompat.go | 21 +++-- report/censor.go | 4 +- report/latest_map_generated.go | 85 ++++------------- report/latest_map_internal_test.go | 117 ++++++++++-------------- report/map_helpers.go | 34 ++----- report/node.go | 14 +-- report/node_test.go | 34 ++----- report/report.go | 16 +++- report/table.go | 5 +- 15 files changed, 138 insertions(+), 217 deletions(-) diff --git a/app/api_report.go b/app/api_report.go index 82852a18e..b24741e85 100644 --- a/app/api_report.go +++ b/app/api_report.go @@ -52,12 +52,13 @@ func makeProbeHandler(rep Reporter) CtxHandlerFunc { for _, n := range rpt.Host.Nodes { id, _ := n.Latest.Lookup(report.ControlProbeID) hostname, _ := n.Latest.Lookup(report.HostName) - version, dt, _ := n.Latest.LookupEntry(report.ScopeVersion) + version, _ := n.Latest.Lookup(report.ScopeVersion) result = append(result, probeDesc{ ID: id, Hostname: hostname, Version: version, - LastSeen: dt, + /* TODO: Figure out how to get a per-probe timestamp + LastSeen: dt, */ }) } respondWith(w, http.StatusOK, result) diff --git a/probe/docker/container_test.go b/probe/docker/container_test.go index 9b35dc216..ea0038e45 100644 --- a/probe/docker/container_test.go +++ b/probe/docker/container_test.go @@ -91,9 +91,9 @@ func TestContainer(t *testing.T) { test.Poll(t, 100*time.Millisecond, want, func() interface{} { node := c.GetNode() latest := report.MakeStringLatestMap() - node.Latest.ForEach(func(k string, t time.Time, v string) { + node.Latest.ForEach(func(k string, v string) { if v != "0" && v != "" { - latest = latest.Set(k, t, v) + latest = latest.Set(k, v) } }) node.Latest = latest @@ -132,7 +132,7 @@ func TestContainerHidingArgs(t *testing.T) { const hostID = "scope" c := docker.NewContainer(container1, hostID, true, false) node := c.GetNode() - node.Latest.ForEach(func(k string, _ time.Time, v string) { + node.Latest.ForEach(func(k string, v string) { if strings.Contains(v, "foo.bar.local") { t.Errorf("Found command line argument in node") } @@ -143,7 +143,7 @@ func TestContainerHidingEnv(t *testing.T) { const hostID = "scope" c := docker.NewContainer(container1, hostID, false, true) node := c.GetNode() - node.Latest.ForEach(func(k string, _ time.Time, v string) { + node.Latest.ForEach(func(k string, v string) { if strings.Contains(v, "secret-bar") { t.Errorf("Found environment variable in node") } @@ -154,7 +154,7 @@ func TestContainerHidingBoth(t *testing.T) { const hostID = "scope" c := docker.NewContainer(container1, hostID, true, true) node := c.GetNode() - node.Latest.ForEach(func(k string, _ time.Time, v string) { + node.Latest.ForEach(func(k string, v string) { if strings.Contains(v, "foo.bar.local") { t.Errorf("Found command line argument in node") } diff --git a/probe/plugins/registry_internal_test.go b/probe/plugins/registry_internal_test.go index 1259bf6ef..7d52df11c 100644 --- a/probe/plugins/registry_internal_test.go +++ b/probe/plugins/registry_internal_test.go @@ -17,6 +17,7 @@ import ( "github.com/paypal/ionet" "github.com/ugorji/go/codec" + "github.com/weaveworks/common/mtime" fs_hook "github.com/weaveworks/common/fs" "github.com/weaveworks/common/test" @@ -695,6 +696,7 @@ func pluginSpec(ID string, interfaces ...string) xfer.PluginSpec { func testReport(topology report.Topology, spec xfer.PluginSpec) report.Report { rpt := report.MakeReport() + rpt.TS = mtime.Now() set := false f := func(t *report.Topology) { if t.Label != topology.Label { diff --git a/probe/probe.go b/probe/probe.go index fb0146943..e6d329b45 100644 --- a/probe/probe.go +++ b/probe/probe.go @@ -7,6 +7,7 @@ import ( "github.com/armon/go-metrics" log "github.com/sirupsen/logrus" + "github.com/weaveworks/common/mtime" "golang.org/x/time/rate" "github.com/weaveworks/scope/report" @@ -185,6 +186,7 @@ func (p *Probe) report() report.Report { } result := report.MakeReport() + result.TS = mtime.Now() for i := 0; i < cap(reports); i++ { result.UnsafeMerge(<-reports) } diff --git a/render/detailed/connections.go b/render/detailed/connections.go index e7ac88fb5..d75eb423f 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -81,7 +81,7 @@ func (c *connectionCounters) add(dns report.DNSRecords, outgoing bool, localNode srcEndpoint, dstEndpoint = localEndpoint, remoteEndpoint } connectionID := srcEndpoint.ID - if copySrcEndpointID, _, ok := srcEndpoint.Latest.LookupEntry(endpoint.CopyOf); ok { + if copySrcEndpointID, ok := srcEndpoint.Latest.Lookup(endpoint.CopyOf); ok { connectionID = copySrcEndpointID } if _, ok := c.counted[connectionID]; ok { @@ -237,7 +237,7 @@ func endpointChildIDsAndCopyMapOf(n report.Node) (report.IDList, map[string]stri n.Children.ForEach(func(child report.Node) { if child.Topology == report.Endpoint { ids = ids.Add(child.ID) - if copyID, _, ok := child.Latest.LookupEntry(endpoint.CopyOf); ok { + if copyID, ok := child.Latest.Lookup(endpoint.CopyOf); ok { copies[child.ID] = copyID } } diff --git a/render/process.go b/render/process.go index 2cce7dc0d..6e46f18f4 100644 --- a/render/process.go +++ b/render/process.go @@ -79,7 +79,7 @@ func hasMoreThanOneConnection(n report.Node, endpoints report.Nodes) bool { firstRealEndpointID := "" for _, endpointID := range n.Adjacency { if ep, ok := endpoints[endpointID]; ok { - if copyID, _, ok := ep.Latest.LookupEntry(report.CopyOf); ok { + if copyID, ok := ep.Latest.Lookup(report.CopyOf); ok { endpointID = copyID } } diff --git a/report/backcompat.go b/report/backcompat.go index 0d7c8391e..57387febf 100644 --- a/report/backcompat.go +++ b/report/backcompat.go @@ -3,6 +3,7 @@ package report // Backwards-compatibility: code to read older reports and convert import ( + "sort" "strings" "time" @@ -13,6 +14,7 @@ import ( type bcNode struct { Node LatestControls map[string]nodeControlDataLatestEntry `json:"latestControls,omitempty"` + OldStringMap map[string]oldStringEntry `json:"latest,omitempty"` } type nodeControlDataLatestEntry struct { @@ -24,25 +26,32 @@ type nodeControlData struct { Dead bool `json:"dead"` } +type oldStringEntry struct { + // Timestamp time.Time `json:"timestamp"` // we don't look at the individual timestamps + Value string `json:"value"` +} + // CodecDecodeSelf implements codec.Selfer func (n *Node) CodecDecodeSelf(decoder *codec.Decoder) { var in bcNode decoder.Decode(&in) *n = in.Node + if len(in.OldStringMap) > 0 { + n.Latest = make(StringLatestMap, 0, len(in.OldStringMap)) + for key, data := range in.OldStringMap { + n.Latest = append(n.Latest, stringLatestEntry{key: key, value: data.Value}) + } + sort.Sort(n.Latest) + } if len(in.LatestControls) > 0 { // Convert the map into a delimited string cs := make([]string, 0, len(in.LatestControls)) - var ts time.Time for name, v := range in.LatestControls { if !v.Value.Dead { cs = append(cs, name) - // Pull out the newest timestamp to use for the whole set - if ts.Before(v.Timestamp) { - ts = v.Timestamp - } } } - n.Latest = n.Latest.Set(NodeActiveControls, ts, strings.Join(cs, ScopeDelim)) + n.Latest = n.Latest.Set(NodeActiveControls, strings.Join(cs, ScopeDelim)) } } diff --git a/report/censor.go b/report/censor.go index 819ceb186..490ac5ab0 100644 --- a/report/censor.go +++ b/report/censor.go @@ -46,7 +46,7 @@ func CensorRawReport(rawReport Report, cfg CensorConfig) Report { censoredReport.WalkTopologies(func(t *Topology) { for nodeID, node := range t.Nodes { if node.Latest != nil { - latest := make(StringLatestMap, 0, cap(node.Latest)) + latest := make([]stringLatestEntry, 0, len(node.Latest)) for _, entry := range node.Latest { // If environment variables are to be hidden, omit passing them to the final report. if cfg.HideEnvironmentVariables && IsEnvironmentVarsEntry(entry.key) { @@ -54,7 +54,7 @@ func CensorRawReport(rawReport Report, cfg CensorConfig) Report { } // If command line arguments are to be hidden, strip them away. if cfg.HideCommandLineArguments && IsCommandEntry(entry.key) { - entry.Value = StripCommandArgs(entry.Value) + entry.value = StripCommandArgs(entry.value) } // Pass the latest entry to the final report. latest = append(latest, entry) diff --git a/report/latest_map_generated.go b/report/latest_map_generated.go index 36e40ea16..22bd69030 100644 --- a/report/latest_map_generated.go +++ b/report/latest_map_generated.go @@ -7,26 +7,13 @@ import ( "bytes" "fmt" "sort" - "time" "github.com/ugorji/go/codec" ) type stringLatestEntry struct { - key string - Timestamp time.Time `json:"timestamp"` - Value string `json:"value"` - dummySelfer -} - -// String returns the StringLatestEntry's string representation. -func (e *stringLatestEntry) String() string { - return fmt.Sprintf("%v (%s)", e.Value, e.Timestamp.Format(time.RFC3339)) -} - -// Equal returns true if the supplied StringLatestEntry is equal to this one. -func (e *stringLatestEntry) Equal(e2 *stringLatestEntry) bool { - return e.Timestamp.Equal(e2.Timestamp) && e.Value == e2.Value + key string + value string } // StringLatestMap holds latest string instances, as a slice sorted by key. @@ -43,6 +30,7 @@ func (m StringLatestMap) Size() int { } // Merge produces a StringLatestMap containing the keys from both inputs. +// m must be at least as new as n // When both inputs contain the same key, the newer value is used. // Tries to return one of its inputs, if that already holds the correct result. func (m StringLatestMap) Merge(n StringLatestMap) StringLatestMap { @@ -52,13 +40,6 @@ func (m StringLatestMap) Merge(n StringLatestMap) StringLatestMap { case len(n) == 0: return m } - if len(n) > len(m) { - m, n = n, m //swap so m is always at least as long as n - } else if len(n) == len(m) && m[0].Timestamp.Before(n[0].Timestamp) { - // Optimise common case where we merge two nodes with the same contents - // sampled at different times. - m, n = n, m // swap equal-length arrays so first element of m is newer - } i, j := 0, 0 loop: @@ -67,9 +48,6 @@ loop: case j >= len(n): return m case m[i].key == n[j].key: - if m[i].Timestamp.Before(n[j].Timestamp) { - break loop - } i++ j++ case m[i].key < n[j].key: @@ -91,11 +69,7 @@ loop: out = append(out, m[i:]...) return out case m[i].key == n[j].key: - if m[i].Timestamp.Before(n[j].Timestamp) { - out = append(out, n[j]) - } else { - out = append(out, m[i]) - } + out = append(out, m[i]) i++ j++ case m[i].key < n[j].key: @@ -112,24 +86,19 @@ loop: // Lookup the value for the given key. func (m StringLatestMap) Lookup(key string) (string, bool) { - v, _, ok := m.LookupEntry(key) - if !ok { - var zero string - return zero, false - } - return v, true + return m.LookupEntry(key) } // LookupEntry returns the raw entry for the given key. -func (m StringLatestMap) LookupEntry(key string) (string, time.Time, bool) { +// name exists for backwards-compatibility +func (m StringLatestMap) LookupEntry(key string) (string, bool) { i := sort.Search(len(m), func(i int) bool { return m[i].key >= key }) if i < len(m) && m[i].key == key { - return m[i].Value, m[i].Timestamp, true + return m[i].value, true } - var zero string - return zero, time.Time{}, false + return "", false } // locate the position where key should go, and make room for it if not there already @@ -147,7 +116,7 @@ func (m *StringLatestMap) locate(key string) int { } // Set the value for the given key. -func (m StringLatestMap) Set(key string, timestamp time.Time, value string) StringLatestMap { +func (m StringLatestMap) Set(key string, value string) StringLatestMap { i := sort.Search(len(m), func(i int) bool { return m[i].key >= key }) @@ -164,14 +133,14 @@ func (m StringLatestMap) Set(key string, timestamp time.Time, value string) Stri copy(m, oldEntries[:i]) copy(m[i+1:], oldEntries[i:]) } - m[i] = stringLatestEntry{key: key, Timestamp: timestamp, Value: value} + m[i] = stringLatestEntry{key: key, value: value} return m } // ForEach executes fn on each key value pair in the map. -func (m StringLatestMap) ForEach(fn func(k string, timestamp time.Time, v string)) { +func (m StringLatestMap) ForEach(fn func(k string, v string)) { for _, value := range m { - fn(value.key, value.Timestamp, value.Value) + fn(value.key, value.value) } } @@ -179,7 +148,7 @@ func (m StringLatestMap) ForEach(fn func(k string, timestamp time.Time, v string func (m StringLatestMap) String() string { buf := bytes.NewBufferString("{") for _, val := range m { - fmt.Fprintf(buf, "%s: %s,\n", val.key, val.String()) + fmt.Fprintf(buf, "%s: %s,\n", val.key, val.value) } fmt.Fprintf(buf, "}") return buf.String() @@ -191,20 +160,7 @@ func (m StringLatestMap) DeepEqual(n StringLatestMap) bool { return false } for i := range m { - if m[i].key != n[i].key || !m[i].Equal(&n[i]) { - return false - } - } - return true -} - -// EqualIgnoringTimestamps returns true if all keys and values are the same. -func (m StringLatestMap) EqualIgnoringTimestamps(n StringLatestMap) bool { - if m.Size() != n.Size() { - return false - } - for i := range m { - if m[i].key != n[i].key || m[i].Value != n[i].Value { + if m[i] != n[i] { return false } } @@ -228,7 +184,7 @@ func (m StringLatestMap) CodecEncodeSelf(encoder *codec.Encoder) { z.EncSendContainerState(containerMapKey) r.EncodeString(cUTF8, val.key) z.EncSendContainerState(containerMapValue) - val.CodecEncodeSelf(encoder) + r.EncodeString(cUTF8, val.value) } z.EncSendContainerState(containerMapEnd) } @@ -253,16 +209,11 @@ func (m *StringLatestMap) CodecDecodeSelf(decoder *codec.Decoder) { break } z.DecSendContainerState(containerMapKey) - var key string - if !r.TryDecodeAsNil() { - key = lookupCommonKey(r.DecodeStringAsBytes()) - } + key := lookupCommonKey(r.DecodeStringAsBytes()) i := m.locate(key) (*m)[i].key = key z.DecSendContainerState(containerMapValue) - if !r.TryDecodeAsNil() { - (*m)[i].CodecDecodeSelf(decoder) - } + (*m)[i].value = r.DecodeString() } z.DecSendContainerState(containerMapEnd) } diff --git a/report/latest_map_internal_test.go b/report/latest_map_internal_test.go index 70ca27a7d..cbc0575db 100644 --- a/report/latest_map_internal_test.go +++ b/report/latest_map_internal_test.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "testing" - "time" "github.com/ugorji/go/codec" @@ -13,17 +12,16 @@ import ( ) func TestLatestMapAdd(t *testing.T) { - now := time.Now() have := MakeStringLatestMap(). - Set("foo", now.Add(-1), "Baz"). - Set("foo", now, "Bar") + Set("foo", "Baz"). + Set("foo", "Bar") if v, ok := have.Lookup("foo"); !ok || v != "Bar" { t.Errorf("v != Bar") } if v, ok := have.Lookup("bar"); ok || v != "" { t.Errorf("v != nil") } - have.ForEach(func(k string, _ time.Time, v string) { + have.ForEach(func(k string, v string) { if k != "foo" || v != "Bar" { t.Errorf("v != Bar") } @@ -31,40 +29,33 @@ func TestLatestMapAdd(t *testing.T) { } func TestLatestMapLookupEntry(t *testing.T) { - now := time.Now() - type LatestEntry struct { - Timestamp time.Time - Value interface{} + value := "Bar" + have := MakeStringLatestMap().Set("foo", value) + if got, ok := have.Lookup("foo"); !ok || got != value { + t.Errorf("got: %#v != expected %#v", got, value) } - entry := LatestEntry{Timestamp: now, Value: "Bar"} - have := MakeStringLatestMap().Set("foo", entry.Timestamp, entry.Value.(string)) - if got, timestamp, ok := have.LookupEntry("foo"); !ok || got != entry.Value || !timestamp.Equal(entry.Timestamp) { - t.Errorf("got: %#v %v != expected %#v", got, timestamp, entry) - } - if got, timestamp, ok := have.LookupEntry("not found"); ok { - t.Errorf("found unexpected entry for %q: %#v %v", "not found", got, timestamp) + if got, ok := have.Lookup("not found"); ok { + t.Errorf("found unexpected entry for %q: %#v", "not found", got) } } func TestLatestMapAddNil(t *testing.T) { - now := time.Now() - have := StringLatestMap{}.Set("foo", now, "Bar") + have := StringLatestMap{}.Set("foo", "Bar") if v, ok := have.Lookup("foo"); !ok || v != "Bar" { t.Errorf("v != Bar") } } func TestLatestMapDeepEquals(t *testing.T) { - now := time.Now() want := MakeStringLatestMap(). - Set("foo", now, "Bar") + Set("foo", "Bar") have := MakeStringLatestMap(). - Set("foo", now, "Bar") + Set("foo", "Bar") if !reflect.DeepEqual(want, have) { t.Errorf(test.Diff(want, have)) } notequal := MakeStringLatestMap(). - Set("foo", now, "Baz") + Set("foo", "Baz") if reflect.DeepEqual(want, notequal) { t.Errorf(test.Diff(want, have)) } @@ -75,9 +66,6 @@ func nilStringLatestMap() StringLatestMap { } func TestLatestMapMerge(t *testing.T) { - now := time.Now() - then := now.Add(-1) - for name, c := range map[string]struct { a, b, want StringLatestMap }{ @@ -89,62 +77,59 @@ func TestLatestMapMerge(t *testing.T) { "Empty a": { a: MakeStringLatestMap(), b: MakeStringLatestMap(). - Set("foo", now, "bar"), + Set("foo", "bar"), want: MakeStringLatestMap(). - Set("foo", now, "bar"), + Set("foo", "bar"), }, "Disjoint a & b": { a: MakeStringLatestMap(). - Set("foo", now, "bar"), + Set("foo", "bar"), b: MakeStringLatestMap(). - Set("baz", now, "bop"), + Set("baz", "bop"), want: MakeStringLatestMap(). - Set("foo", now, "bar"). - Set("baz", now, "bop"), + Set("foo", "bar"). + Set("baz", "bop"), }, "Common a & b": { a: MakeStringLatestMap(). - Set("foo", now, "bar"), + Set("foo", "bar"), b: MakeStringLatestMap(). - Set("foo", then, "baz"), + Set("foo", "baz"), want: MakeStringLatestMap(). - Set("foo", now, "bar"), + Set("foo", "bar"), }, "Longer": { a: MakeStringLatestMap(). - Set("PID", now, "23128"). - Set("Name", now, "curl"), + Set("PID", "23128"). + Set("Name", "curl"), b: MakeStringLatestMap(). - Set("PID", then, "0"). - Set("Name", now, "curl"). - Set("Domain", now, "node-a.local"), + Set("PID", "0"). + Set("Name", "curl"). + Set("Domain", "node-a.local"), want: MakeStringLatestMap(). - Set("PID", now, "23128"). - Set("Name", now, "curl"). - Set("Domain", now, "node-a.local"), + Set("PID", "23128"). + Set("Name", "curl"). + Set("Domain", "node-a.local"), }, } { if have := c.a.Merge(c.b); !reflect.DeepEqual(c.want, have) { t.Errorf("%s:\n%s", name, test.Diff(c.want, have)) } - if have := c.b.Merge(c.a); !reflect.DeepEqual(c.want, have) { - t.Errorf("%s:\n%s", name, test.Diff(c.want, have)) - } } } -func makeBenchmarkMap(start, finish int, timestamp time.Time) StringLatestMap { +func makeBenchmarkMap(start, finish int) StringLatestMap { ret := MakeStringLatestMap() for i := start; i < finish; i++ { - ret = ret.Set(fmt.Sprint(i), timestamp, "1") + ret = ret.Set(fmt.Sprint(i), "1") } return ret } func BenchmarkLatestMapMerge(b *testing.B) { // two large maps with some overlap - left := makeBenchmarkMap(0, 1000, time.Now()) - right := makeBenchmarkMap(700, 1700, time.Now().Add(1*time.Minute)) + left := makeBenchmarkMap(0, 1000) + right := makeBenchmarkMap(700, 1700) b.ResetTimer() @@ -154,7 +139,7 @@ func BenchmarkLatestMapMerge(b *testing.B) { } func BenchmarkLatestMapEncode(b *testing.B) { - map1 := makeBenchmarkMap(0, 1000, time.Now()) + map1 := makeBenchmarkMap(0, 1000) b.ResetTimer() @@ -165,7 +150,7 @@ func BenchmarkLatestMapEncode(b *testing.B) { } func BenchmarkLatestMapDecode(b *testing.B) { - map1 := makeBenchmarkMap(0, 1000, time.Now()) + map1 := makeBenchmarkMap(0, 1000) buf := &bytes.Buffer{} codec.NewEncoder(buf, &codec.MsgpackHandle{}).Encode(&map1) @@ -178,25 +163,16 @@ func BenchmarkLatestMapDecode(b *testing.B) { } func TestLatestMapDecoding(t *testing.T) { - ts, _ := time.Parse(time.RFC3339Nano, "2018-02-26T09:50:43Z") want := MakeStringLatestMap(). - Set("foo", ts, "bar"). - Set("bar", ts, "baz"). - Set("emptyval", ts, "") + Set("foo", "bar"). + Set("bar", "baz"). + Set("emptyval", "") // The following string is carefully constructed to have 'emptyval' not in alphabetical order data := ` { - "bar": { - "timestamp": "2018-02-26T09:50:43Z", - "value": "baz" - }, - "foo": { - "timestamp": "2018-02-26T09:50:43Z", - "value": "bar" - }, - "emptyval": { - "timestamp": "2018-02-26T09:50:43Z" - } + "bar": "baz", + "foo": "bar", + "emptyval": }` h := &codec.JsonHandle{} decoder := codec.NewDecoder(bytes.NewBufferString(data), h) @@ -208,10 +184,9 @@ func TestLatestMapDecoding(t *testing.T) { } func TestLatestMapEncoding(t *testing.T) { - now := time.Now() want := MakeStringLatestMap(). - Set("foo", now, "bar"). - Set("bar", now, "baz") + Set("foo", "bar"). + Set("bar", "baz") for _, h := range []codec.Handle{ codec.Handle(&codec.MsgpackHandle{}), @@ -255,7 +230,7 @@ func TestLatestMapMergeEqualDecoderTypes(t *testing.T) { t.Error("Merging two maps with the same decoders should not panic") } }() - m1 := MakeStringLatestMap().Set("a", time.Now(), "bar") - m2 := MakeStringLatestMap().Set("b", time.Now(), "foo") + m1 := MakeStringLatestMap().Set("a", "bar") + m2 := MakeStringLatestMap().Set("b", "foo") m1.Merge(m2) } diff --git a/report/map_helpers.go b/report/map_helpers.go index 169f953f4..f7473e7c5 100644 --- a/report/map_helpers.go +++ b/report/map_helpers.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "sort" - "time" "github.com/ugorji/go/codec" "github.com/weaveworks/ps" @@ -147,31 +146,15 @@ func (m StringLatestMap) Len() int { return len(m) } func (m StringLatestMap) Swap(i, j int) { m[i], m[j] = m[j], m[i] } func (m StringLatestMap) Less(i, j int) bool { return m[i].key < m[j].key } -// sort entries and shuffle down any duplicates. NOTE: may modify contents of m. -func (m StringLatestMap) sortedAndDeduplicated() StringLatestMap { - sort.Sort(m) - for i := 1; i < len(m); { - if m[i-1].key == m[i].key { - if m[i-1].Timestamp.Before(m[i].Timestamp) { - m = append(m[:i-1], m[i:]...) - } else { - m = append(m[:i], m[i+1:]...) - } - } else { - i++ - } - } - return m -} - -// add several entries at the same timestamp -func (m StringLatestMap) addMapEntries(ts time.Time, n map[string]string) StringLatestMap { +// add several entries: any values in n overwrite values with the same key in m +func (m StringLatestMap) addMapEntries(n map[string]string) StringLatestMap { out := make(StringLatestMap, len(m), len(m)+len(n)) copy(out, m) for k, v := range n { - out = append(out, stringLatestEntry{key: k, Value: v, Timestamp: ts}) + i := out.locate(k) + out[i] = stringLatestEntry{key: k, value: v} } - return out.sortedAndDeduplicated() + return out } // Propagate a set of latest values from one set to another. @@ -179,9 +162,10 @@ func (m StringLatestMap) Propagate(from StringLatestMap, keys ...string) StringL out := make(StringLatestMap, len(m), len(m)+len(keys)) copy(out, m) for _, k := range keys { - if v, ts, ok := from.LookupEntry(k); ok { - out = append(out, stringLatestEntry{key: k, Value: v, Timestamp: ts}) + if v, ok := from.Lookup(k); ok { + i := out.locate(k) + out[i] = stringLatestEntry{key: k, value: v} } } - return out.sortedAndDeduplicated() + return out } diff --git a/report/node.go b/report/node.go index 9883cd7e2..0c6a4f392 100644 --- a/report/node.go +++ b/report/node.go @@ -16,7 +16,7 @@ type Node struct { Counters Counters `json:"counters,omitempty"` Sets Sets `json:"sets,omitempty"` Adjacency IDList `json:"adjacency,omitempty"` - Latest StringLatestMap `json:"latest,omitempty"` + Latest StringLatestMap `json:"strs,omitempty"` Metrics Metrics `json:"metrics,omitempty" deepequal:"nil==empty"` Parents Sets `json:"parents,omitempty"` Children NodeSet `json:"children,omitempty"` @@ -68,15 +68,16 @@ func (n Node) After(other Node) bool { } // WithLatests returns a fresh copy of n, with Metadata m merged in. +// Any values in m overwrite values with the same key in n func (n Node) WithLatests(m map[string]string) Node { - ts := mtime.Now() - n.Latest = n.Latest.addMapEntries(ts, m) + n.Latest = n.Latest.addMapEntries(m) return n } // WithLatest produces a new Node with k mapped to v in the Latest metadata. -func (n Node) WithLatest(k string, ts time.Time, v string) Node { - n.Latest = n.Latest.Set(k, ts, v) +// TODO: remove backwards-compatibility time argument +func (n Node) WithLatest(k string, _ time.Time, v string) Node { + n.Latest = n.Latest.Set(k, v) return n } @@ -161,6 +162,7 @@ func (n Node) WithChild(child Node) Node { // Merge mergses the individual components of a node and returns a // fresh node. +// TODO: we must know at this point that n is at least as new as other func (n Node) Merge(other Node) Node { id := n.ID if id == "" { @@ -199,7 +201,7 @@ func (n *Node) UnsafeUnMerge(other Node) bool { // We either keep a whole section or drop it if anything changed // - a trade-off of some extra data size in favour of faster simpler code. // (in practice, very few values reported by Scope probes do change over time) - if n.Latest.EqualIgnoringTimestamps(other.Latest) { + if n.Latest.DeepEqual(other.Latest) { n.Latest = nil } else { remove = false diff --git a/report/node_test.go b/report/node_test.go index 5751fe922..2843d21f5 100644 --- a/report/node_test.go +++ b/report/node_test.go @@ -23,6 +23,7 @@ func TestWithLatest(t *testing.T) { latests1 := map[string]string{Name: "x"} latests2 := map[string]string{PID: "123"} + latests3 := map[string]string{PID: "456"} node1 := report.MakeNode("node1").WithLatests(latests1) assert.Equal(t, 1, node1.Latest.Len()) node2 := node1.WithLatests(latests1) @@ -31,6 +32,11 @@ func TestWithLatest(t *testing.T) { assert.Equal(t, 2, node3.Latest.Len()) node4 := node1.WithLatests(latests2) assert.Equal(t, node3, node4) + node5 := node3.WithLatests(latests3) + checkPid, ok := node5.Latest.Lookup(PID) + assert.Equal(t, true, ok) + assert.Equal(t, "456", checkPid) + assert.Equal(t, 2, node5.Latest.Len()) } func TestMergeNodes(t *testing.T) { @@ -102,41 +108,19 @@ func TestMergeNodes(t *testing.T) { }), }, }, - "Merge conflict with rank difference": { + "Merge conflict": { a: report.Nodes{ ":192.168.1.1:12345": report.MakeNodeWith(":192.168.1.1:12345", map[string]string{ - PID: "23128", - Name: "curl", - Domain: "node-a.local", - }), - }, - b: report.Nodes{ - ":192.168.1.1:12345": report.MakeNodeWith(":192.168.1.1:12345", map[string]string{ // <-- same ID Name: "curl", Domain: "node-a.local", }).WithLatest(PID, time.Now().Add(-1*time.Minute), "0"), }, - want: report.Nodes{ - ":192.168.1.1:12345": report.MakeNodeWith(":192.168.1.1:12345", map[string]string{ - PID: "23128", - Name: "curl", - Domain: "node-a.local", - }), - }, - }, - "Merge conflict with no rank difference": { - a: report.Nodes{ - ":192.168.1.1:12345": report.MakeNodeWith(":192.168.1.1:12345", map[string]string{ - PID: "23128", - Name: "curl", - Domain: "node-a.local", - }), - }, b: report.Nodes{ ":192.168.1.1:12345": report.MakeNodeWith(":192.168.1.1:12345", map[string]string{ // <-- same ID + PID: "23128", Name: "curl", Domain: "node-a.local", - }).WithLatest(PID, time.Now().Add(-1*time.Minute), "0"), + }), }, want: report.Nodes{ ":192.168.1.1:12345": report.MakeNodeWith(":192.168.1.1:12345", map[string]string{ diff --git a/report/report.go b/report/report.go index b1c0aa2bf..85798a67d 100644 --- a/report/report.go +++ b/report/report.go @@ -85,6 +85,9 @@ var topologyNames = []string{ // stored by apps. It's composed of multiple topologies, each representing // a different (related, but not equivalent) view of the network. type Report struct { + // TS is the time this report was generated + TS time.Time + // Endpoint nodes are individual (address, port) tuples on each host. // They come from inspecting active connections and can (theoretically) // be traced back to a process. Edges are present. @@ -319,6 +322,7 @@ func MakeReport() Report { // Copy returns a value copy of the report. func (r Report) Copy() Report { newReport := Report{ + TS: r.TS, DNS: r.DNS.Copy(), Sampling: r.Sampling, Window: r.Window, @@ -335,12 +339,20 @@ func (r Report) Copy() Report { // Merge merges another Report into the receiver and returns the result. The // original is not modified. func (r Report) Merge(other Report) Report { - newReport := r.Copy() - newReport.UnsafeMerge(other) + var newReport Report + // Find the newer one and merge in the older one + if r.TS.Before(other.TS) { + newReport = other.Copy() + newReport.UnsafeMerge(r) + } else { + newReport = r.Copy() + newReport.UnsafeMerge(other) + } return newReport } // UnsafeMerge merges another Report into the receiver. The original is modified. +// TODO: r must be at least as new as other func (r *Report) UnsafeMerge(other Report) { r.DNS = r.DNS.Merge(other.DNS) r.Sampling = r.Sampling.Merge(other.Sampling) diff --git a/report/table.go b/report/table.go index a05fef984..d9f244e72 100644 --- a/report/table.go +++ b/report/table.go @@ -4,7 +4,6 @@ import ( "fmt" "sort" "strings" - "time" log "github.com/sirupsen/logrus" "github.com/weaveworks/common/mtime" @@ -61,7 +60,7 @@ func (node Node) ExtractMulticolumnTable(template TableTemplate) (rows []Row) { // with the given prefix. If it is possible to traverse the keys in the Latest map // in a sorted order, then having LowerBoundEntry(key) and UpperBoundEntry(key) // methods should be enough to implement ForEachWithPrefix(prefix) straightforwardly. - node.Latest.ForEach(func(key string, _ time.Time, value string) { + node.Latest.ForEach(func(key string, value string) { if keyWithoutPrefix, ok := WithoutPrefix(key, template.Prefix); ok { ids := strings.Split(keyWithoutPrefix, tableEntryKeySeparator) rowID, columnID := ids[0], ids[1] @@ -94,7 +93,7 @@ func (node Node) ExtractPropertyList(template TableTemplate) (rows []Row) { // Itearate through the whole of our map to extract all the values with the key // with the given prefix as well as the keys corresponding to the fixed table rows. - node.Latest.ForEach(func(key string, _ time.Time, value string) { + node.Latest.ForEach(func(key string, value string) { if label, ok := template.FixedRows[key]; ok { valuesMapByLabel[label] = value } else if label, ok := WithoutPrefix(key, template.Prefix); ok {