Migrate fuzzer to use shiny new metrics2 package

Migrate fuzzer to use shiny new metrics2 package

I added a way to get the current count from a metrics2.Counter, similar to go_metrics.Count()

Alerts have been deleted for now.  I will add new ones in a future cl.

BUG=skia:4438
Committed: https://skia.googlesource.com/buildbot/+/eca1b5d6a4ade6e4042c51431e2f21357450691c

[kjlubick, 2016-02-10 14:51:08]

kjlubick@google.com changed reviewers:
+ borenet@google.com

[kjlubick, 2016-02-10 14:51:29]

Description was changed from

==========
Migrate fuzzer to use shiny new metrics2 package

BUG=skia:4438
==========

to

==========
Migrate fuzzer to use shiny new metrics2 package

Alerts have been deleted for now.  I will add new ones in a future cl.

BUG=skia:4438
==========

[kjlubick, 2016-02-10 14:52:07]

Description was changed from

==========
Migrate fuzzer to use shiny new metrics2 package

Alerts have been deleted for now.  I will add new ones in a future cl.

BUG=skia:4438
==========

to

==========
Migrate fuzzer to use shiny new metrics2 package

I added a way to get the current count from a metrics2.Counter, similar to
go_metrics.Count()

Alerts have been deleted for now.  I will add new ones in a future cl.

BUG=skia:4438
==========

[borenet, 2016-02-10 15:32:32]

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
File fuzzer/go/aggregator/aggregator.go (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
fuzzer/go/aggregator/aggregator.go:249:
metrics2.GetInt64Metric("newly_found_fuzzes", map[string]string{"category":
agg.Category}).Update(int64(len(newlyFound)))
Naming conventions: measurements and tag keys should be
words-separated-by-dashes, and measurement names which aren't shared (eg.
"counter", "liveness") should be dot-separated following roughly this format:
domain.category.metric, eg. "fuzzer.fuzzes.newly-found"

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
fuzzer/go/aggregator/aggregator.go:336: defer
metrics2.NewCounter("analysis_process_count", map[string]string{"category":
agg.Category}).Dec(int64(1))
Use dashes, ie. "analysis-process-count", here and below.

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
fuzzer/go/aggregator/aggregator.go:702: }
Does this logic belong in this CL?

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/config/config.go
File fuzzer/go/config/config.go (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/config/config.go#...
fuzzer/go/config/config.go:19: SkipGeneration       bool
Is this related to metrics?  Can we save it for a different CL?

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/generator/afl_gen...
File fuzzer/go/generator/afl_generation.go (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/generator/afl_gen...
fuzzer/go/generator/afl_generation.go:65: g.fuzzProcessCount =
metrics2.NewCounter("afl_fuzz_process_count", map[string]string{"category":
g.Category})
Dashes

https://codereview.chromium.org/1682363003/diff/1/fuzzer/sys/fuzzer-be.service
File fuzzer/sys/fuzzer-be.service (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/sys/fuzzer-be.servic...
fuzzer/sys/fuzzer-be.service:9: --influxdb_database=skmetrics \\
Double slash?

https://codereview.chromium.org/1682363003/diff/1/go/metrics2/counter.go
File go/metrics2/counter.go (right):

https://codereview.chromium.org/1682363003/diff/1/go/metrics2/counter.go#newc...
go/metrics2/counter.go:56: func (c *Counter) Count() int64 {
For consistency, let's just call this Get()

[kjlubick, 2016-02-10 16:14:29]

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
File fuzzer/go/aggregator/aggregator.go (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
fuzzer/go/aggregator/aggregator.go:249:
metrics2.GetInt64Metric("newly_found_fuzzes", map[string]string{"category":
agg.Category}).Update(int64(len(newlyFound)))
On 2016/02/10 at 15:32:32, borenet wrote:
> Naming conventions: measurements and tag keys should be
words-separated-by-dashes, and measurement names which aren't shared (eg.
"counter", "liveness") should be dot-separated following roughly this format:
domain.category.metric, eg. "fuzzer.fuzzes.newly-found"

Done.

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
fuzzer/go/aggregator/aggregator.go:336: defer
metrics2.NewCounter("analysis_process_count", map[string]string{"category":
agg.Category}).Dec(int64(1))
On 2016/02/10 at 15:32:32, borenet wrote:
> Use dashes, ie. "analysis-process-count", here and below.

Done.

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/aggregator/aggreg...
fuzzer/go/aggregator/aggregator.go:702: }
On 2016/02/10 at 15:32:32, borenet wrote:
> Does this logic belong in this CL?

Removed.

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/config/config.go
File fuzzer/go/config/config.go (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/config/config.go#...
fuzzer/go/config/config.go:19: SkipGeneration       bool
On 2016/02/10 at 15:32:32, borenet wrote:
> Is this related to metrics?  Can we save it for a different CL?

Removed.

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/generator/afl_gen...
File fuzzer/go/generator/afl_generation.go (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/go/generator/afl_gen...
fuzzer/go/generator/afl_generation.go:65: g.fuzzProcessCount =
metrics2.NewCounter("afl_fuzz_process_count", map[string]string{"category":
g.Category})
On 2016/02/10 at 15:32:32, borenet wrote:
> Dashes

Done

https://codereview.chromium.org/1682363003/diff/1/fuzzer/sys/fuzzer-be.service
File fuzzer/sys/fuzzer-be.service (right):

https://codereview.chromium.org/1682363003/diff/1/fuzzer/sys/fuzzer-be.servic...
fuzzer/sys/fuzzer-be.service:9: --influxdb_database=skmetrics \\
On 2016/02/10 at 15:32:32, borenet wrote:
> Double slash?

Fixed

https://codereview.chromium.org/1682363003/diff/1/go/metrics2/counter.go
File go/metrics2/counter.go (right):

https://codereview.chromium.org/1682363003/diff/1/go/metrics2/counter.go#newc...
go/metrics2/counter.go:56: func (c *Counter) Count() int64 {
On 2016/02/10 at 15:32:32, borenet wrote:
> For consistency, let's just call this Get()

Done.

[borenet, 2016-02-11 12:23:28]

LGTM with a couple of comments

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/aggregator/ag...
File fuzzer/go/aggregator/aggregator.go (right):

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/aggregator/ag...
fuzzer/go/aggregator/aggregator.go:646:
metrics2.GetInt64Metric("fuzzer.queue-size.bug_report",
map[string]string{"category":
agg.Category}).Update(int64(len(agg.forBugReporting)))
Nit: bug_report -> bug-report

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/fuzzer-be/mai...
File fuzzer/go/fuzzer-be/main.go (right):

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/fuzzer-be/mai...
fuzzer/go/fuzzer-be/main.go:93: if !*forceReanalysis {
Not sure if you meant to remove skipGeneration here.

[kjlubick, 2016-02-11 13:04:38]

The CQ bit was checked by kjlubick@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-11 13:04:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682363003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682363003/40001

[commit-bot: I haz the power, 2016-02-11 13:08:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-11 13:08:59]

Dry run: Try jobs failed on following builders:
  Infra-PerCommit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/Infra-PerCommit-Trybot/b...)

[kjlubick, 2016-02-11 13:17:23]

The CQ bit was checked by kjlubick@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-11 13:17:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682363003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682363003/40001

[commit-bot: I haz the power, 2016-02-11 13:24:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-11 13:24:58]

Dry run: Try jobs failed on following builders:
  Infra-PerCommit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/Infra-PerCommit-Trybot/b...)

[kjlubick, 2016-02-11 14:05:11]

The CQ bit was checked by kjlubick@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-11 14:05:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682363003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682363003/40001

[commit-bot: I haz the power, 2016-02-11 14:14:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-11 14:14:57]

Dry run: This issue passed the CQ dry run.

[kjlubick, 2016-02-11 14:15:28]

The CQ bit was checked by kjlubick@google.com

[kjlubick, 2016-02-11 14:15:28]

The patchset sent to the CQ was uploaded after l-g-t-m from borenet@google.com
Link to the patchset: https://codereview.chromium.org/1682363003/#ps40001
(title: "one last _ to -")

[commit-bot: I haz the power, 2016-02-11 14:15:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1682363003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1682363003/40001

[commit-bot: I haz the power, 2016-02-11 14:15:48]

Description was changed from

==========
Migrate fuzzer to use shiny new metrics2 package

I added a way to get the current count from a metrics2.Counter, similar to
go_metrics.Count()

Alerts have been deleted for now.  I will add new ones in a future cl.

BUG=skia:4438
==========

to

==========
Migrate fuzzer to use shiny new metrics2 package

I added a way to get the current count from a metrics2.Counter, similar to
go_metrics.Count()

Alerts have been deleted for now.  I will add new ones in a future cl.

BUG=skia:4438

Committed:
https://skia.googlesource.com/buildbot/+/eca1b5d6a4ade6e4042c51431e2f21357450...
==========

[commit-bot: I haz the power, 2016-02-11 14:15:49]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/buildbot/+/eca1b5d6a4ade6e4042c51431e2f21357450...

[kjlubick, 2016-02-11 17:54:41]

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/aggregator/ag...
File fuzzer/go/aggregator/aggregator.go (right):

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/aggregator/ag...
fuzzer/go/aggregator/aggregator.go:646:
metrics2.GetInt64Metric("fuzzer.queue-size.bug_report",
map[string]string{"category":
agg.Category}).Update(int64(len(agg.forBugReporting)))
On 2016/02/11 at 12:23:28, borenet wrote:
> Nit: bug_report -> bug-report

Done.

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/fuzzer-be/mai...
File fuzzer/go/fuzzer-be/main.go (right):

https://codereview.chromium.org/1682363003/diff/20001/fuzzer/go/fuzzer-be/mai...
fuzzer/go/fuzzer-be/main.go:93: if !*forceReanalysis {
On 2016/02/11 at 12:23:28, borenet wrote:
> Not sure if you meant to remove skipGeneration here.

It'll be fine.  The very next CL will add the other logic back in.