From a47cf0a2aa4fcb73c76c9b9ce726cc0c352b32f4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 6 Mar 2020 15:03:43 +0000 Subject: [PATCH 1/3] Remove copying Merge() on Report It was only used in a few places, and all of those were better off using the Unsafe variant. --- app/collector_test.go | 4 ++-- probe/plugins/registry.go | 2 +- render/theinternet_test.go | 4 ++-- report/report.go | 8 -------- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/app/collector_test.go b/app/collector_test.go index c986d2ecc..22af88167 100644 --- a/app/collector_test.go +++ b/app/collector_test.go @@ -50,8 +50,8 @@ func TestCollector(t *testing.T) { c.Add(ctx, r2, nil) merged := report.MakeReport() - merged = merged.Merge(r1) - merged = merged.Merge(r2) + merged.UnsafeMerge(r1) + merged.UnsafeMerge(r2) have, err = c.Report(ctx, mtime.Now()) if err != nil { t.Error(err) diff --git a/probe/plugins/registry.go b/probe/plugins/registry.go index 132dda94e..f7492fc8e 100644 --- a/probe/plugins/registry.go +++ b/probe/plugins/registry.go @@ -222,7 +222,7 @@ func (r *Registry) Report() (report.Report, error) { if plugin.Implements("controller") { r.updateAndRegisterControlsInReport(&pluginReport) } - rpt = rpt.Merge(pluginReport) + rpt.UnsafeMerge(pluginReport) }) return rpt, nil } diff --git a/render/theinternet_test.go b/render/theinternet_test.go index a040d6772..7b85fc2fd 100644 --- a/render/theinternet_test.go +++ b/render/theinternet_test.go @@ -10,7 +10,7 @@ import ( ) func TestReportLocalNetworks(t *testing.T) { - r := report.MakeReport().Merge(report.Report{ + r := report.Report{ Host: report.Topology{ Nodes: report.Nodes{ "nonets": report.MakeNode("nonets"), @@ -27,7 +27,7 @@ func TestReportLocalNetworks(t *testing.T) { ), }, }, - }) + }.Copy() want := report.MakeNetworks() for _, cidr := range []string{"10.0.0.1/8", "192.168.1.1/24", "10.32.0.1/12"} { if err := want.AddCIDR(cidr); err != nil { diff --git a/report/report.go b/report/report.go index 38c502b16..77025ebeb 100644 --- a/report/report.go +++ b/report/report.go @@ -334,14 +334,6 @@ func (r Report) Copy() Report { return newReport } -// 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) - return newReport -} - // UnsafeMerge merges another Report into the receiver. The original is modified. func (r *Report) UnsafeMerge(other Report) { r.DNS = r.DNS.Merge(other.DNS) From 3098267be44387fe67a975ea2bb1ea57e8e7e720 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 6 Mar 2020 15:39:06 +0000 Subject: [PATCH 2/3] Add a timestamp on each report saying when it was generated Not currently used in the code, but useful when troubleshooting. --- probe/probe.go | 2 ++ probe/probe_internal_test.go | 1 + report/report.go | 8 ++++++++ 3 files changed, 11 insertions(+) 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/probe/probe_internal_test.go b/probe/probe_internal_test.go index 97af02904..e2ba43623 100644 --- a/probe/probe_internal_test.go +++ b/probe/probe_internal_test.go @@ -69,6 +69,7 @@ func TestProbe(t *testing.T) { node := report.MakeNodeWith("a", map[string]string{"b": "c"}) want.Endpoint.AddNode(node) + want.TS = now pub := mockPublisher{make(chan report.Report, 10)} diff --git a/report/report.go b/report/report.go index 77025ebeb..66fc0cdcb 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. @@ -321,6 +324,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, @@ -336,6 +340,10 @@ func (r Report) Copy() Report { // UnsafeMerge merges another Report into the receiver. The original is modified. func (r *Report) UnsafeMerge(other Report) { + // Merged report has the earliest non-zero timestamp + if !other.TS.IsZero() && (r.TS.IsZero() || other.TS.Before(r.TS)) { + r.TS = other.TS + } r.DNS = r.DNS.Merge(other.DNS) r.Sampling = r.Sampling.Merge(other.Sampling) r.Window = r.Window + other.Window From 72145dc37866eff9d1e6ac9c83ace595509e94b1 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 6 Mar 2020 15:43:54 +0000 Subject: [PATCH 3/3] Set the Window duration which each report covers This is used by the multi-tenant code. --- probe/probe.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/probe/probe.go b/probe/probe.go index e6d329b45..e54ec6c84 100644 --- a/probe/probe.go +++ b/probe/probe.go @@ -236,6 +236,7 @@ ForLoop: func (p *Probe) publishLoop() { defer p.done.Done() + startTime := mtime.Now() pubTick := time.Tick(p.publishInterval) publishCount := 0 var lastFullReport report.Report @@ -249,6 +250,8 @@ func (p *Probe) publishLoop() { if !fullReport { rpt.UnsafeUnMerge(lastFullReport) } + rpt.Window = mtime.Now().Sub(startTime) + startTime = mtime.Now() err = p.publisher.Publish(rpt) if err == nil { if fullReport {