1306503 - Add support for logspam utility in mach

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Eric Rahm [:erahm]]

I'd like to integrate the logspam tool into mach.

The commands would be something like:
  - |mach logspam report| - Generates an overall report of log spamminess
  - |mach logspam file| - Files a bug for a specific spammy warning
  - |mach logspam bisect| - Performs bisection to determine when the spammy warning was introduced

I'll probably keep the logspam tool hosted in github for now and package it via pypi. Integration would be similar to mozregression.

I'm not hard set on the naming, open to suggestions.

Comment 1

[Eric Rahm [:erahm]]

Attachment: Bug 1306503 - Allow mach subcommands to have a parser.

This allows a mach subcommand to provide a custom argument parser. It just
passes it through to the base command.

Review commit: https://reviewboard.mozilla.org/r/82244/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82244/

Comment 2

[Eric Rahm [:erahm]]

Attachment: Bug 1306503 - Properly handle printing help for subcommands with parsers.

This supports printing help for subcommands that use a custom parser rather
than mach decorators. Example usage |mach command help subcommand|.

Review commit: https://reviewboard.mozilla.org/r/82246/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82246/

Comment 3

[Eric Rahm [:erahm]]

Attachment: Bug 1306503 - Split out a reusable pypi tool class.

This splits out the pypi tool logic baked into the mozregression command. It
will be reused for the logspam command.

Review commit: https://reviewboard.mozilla.org/r/82248/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82248/

Comment 4

[Eric Rahm [:erahm]]

Attachment: Bug 1306503 - Add logspam mach commands.

This adds support for invoking the |mach logspam {report|file|bisect}|
commands. The core logic from the pypi base command is reused.

Review commit: https://reviewboard.mozilla.org/r/82250/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82250/

Comment 5

[Eric Rahm [:erahm]]

Comment on attachment 8796370 [details]
Bug 1306503 - Split out a reusable pypi tool class.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82248/diff/1-2/

Comment 6

[Eric Rahm [:erahm]]

Comment on attachment 8796371 [details]
Bug 1306503 - Add logspam mach commands.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/82250/diff/1-2/

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8796368 [details]
Bug 1306503 - Allow mach subcommands to have a parser.

https://reviewboard.mozilla.org/r/82244/#review81094

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8796369 [details]
Bug 1306503 - Properly handle printing help for subcommands with parsers.

https://reviewboard.mozilla.org/r/82246/#review81098

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8796370 [details]
Bug 1306503 - Split out a reusable pypi tool class.

https://reviewboard.mozilla.org/r/82248/#review81100

I didn't know this is how mozregression loaded itself into mach. I'm both impressed and terrified. Nifty little hack.

::: tools/mach_commands.py:310
(Diff revision 2)
>              os.chmod(temp, os.stat(temp).st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH)
>              os.rename(temp, target)
>          return target
>  
> -def mozregression_import():
> -    # Lazy loading of mozregression.
> +
> +class PypiBasedTool:

class PypiBasedTool(object):

The `(object)` makes it a "new style class." This isn't needed in Python 3, fortunately. But we run Python 2.7 :/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8796371 [details]
Bug 1306503 - Add logspam mach commands.

https://reviewboard.mozilla.org/r/82250/#review81102

I'm going to leave r? on this so I can think about it.

I like what logspam is doing. But I question whether a new mach command for analyzing logs is needed. We already have a `mach show-log` command. It defaults to printing the log of the last mach command. I think there is room to hang the logspam functionality off this command. I would also entertain adding a `mach log` command that had subcommands for various functionality, including logspam.

Comment 11

[Eric Rahm [:erahm]]

Comment on attachment 8796371 [details]
Bug 1306503 - Add logspam mach commands.

https://reviewboard.mozilla.org/r/82250/#review81102

https://github.com/EricRahm/log-spam-hell/issues/1

Comment 12

[Gregory Szorc [:gps]]

Since I wrote that comment, mach's logging facility has become slightly more useful. For example, since I wrote that, mach now records logs for the last executed command automatically.

Comment 13

[Eric Rahm [:erahm]]

(In reply to Gregory Szorc [:gps] from comment #12)
> Since I wrote that comment, mach's logging facility has become slightly more
> useful. For example, since I wrote that, mach now records logs for the last
> executed command automatically.

Sorry for being terse, I was on a mobile device, also I think I misinterpreted what you meant.

To clarify I think there are a couple of use cases:
  #1 - Sheriffs - Make it easy for sheriffs to run after m-c merges have completed testing
  #2 - mach try - Make it easy for devs to run after a try push has completed testing (currently it's "hey erahm I autolanded this, did it fix it?")

I like the idea of being able to run against a local log too, but that's not baked in. Currently it expects to need to download the logs from S3 (by way of treeherder).

The point I probably confused is that you think it could hang under |mach log| or something rather than |mach logspam| vs we don't need this at all. That's fine with me, I don't particularly care what it's called.

Comment 14

[Eric Rahm [:erahm]]

What do you think re comment 13?

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8796371 [details]
Bug 1306503 - Add logspam mach commands.

https://reviewboard.mozilla.org/r/82250/#review86576

Comment 16

[Gregory Szorc [:gps]]

I gave you r+ and am fine landing as is. We can always refactor it later. As long as there is good help output.

Sorry for the delay. I was away on a business trip and holiday.

Comment 17

[Julien Cristau [:jcristau] (back August 18)]

Too late for firefox 52, mass-wontfix.

Comment 18

[Eric Rahm [:erahm]]

Realistically I'm not going to get to this, but it'd be nice if someone did and got a periodic job running. I'm happy to provide feedback and guidance.

Comment 19

[Eric Rahm [:erahm]]

Attachment: Bug 1306503 - Split out a reusable pypi tool class. r=#build

This splits out the pypi tool logic baked into the mozregression command. It
will be reused for the logspam command.

Comment 20

[Eric Rahm [:erahm]]

Attachment: Bug 1306503 - Add logspam mach commands. r=#build

This adds support for invoking the |mach logspam {report|file|bisect}|
commands. The core logic from the pypi base command is reused.

Depends on D85492

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f7489fb2f2e
Split out a reusable pypi tool class. r=firefox-build-system-reviewers,mhentges,rstewart
https://hg.mozilla.org/integration/autoland/rev/de4550dedea6
Add logspam mach commands. r=firefox-build-system-reviewers,mhentges,rstewart

Comment 22

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8f7489fb2f2e
https://hg.mozilla.org/mozilla-central/rev/de4550dedea6