Closed Bug 1188351 Opened 9 years ago Closed 8 years ago

Commit messages should not be included in the build/test logs (eg by the read-buildbot-config step)

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
Tracking Status
firefox46 --- fixed

People

(Reporter: emorley, Assigned: emorley)

References

Details

(Keywords: treeherder)

Attachments

(1 file, 1 obsolete file)

Commit messages often contain strings that match those in the log parse regex, since the message might be saying something like "Fix ERROR foo".

At best this causes false-positives in the error summary (and thus bug suggestions) shown in Treeherder, at worst (eg in the case of bug 1188132), this causes the log parser to get very confused.

IMO there is no need to include these messages; the SHA / revision URL is more than enough :-)

eg:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-macosx64-debug/1438022143/mozilla-inbound_snowleopard-debug_test-mochitest-1-bm108-tests1-macosx-build892.txt.gz

12:59:25     INFO - ##### Running read-buildbot-config step.
12:59:25     INFO - #####
12:59:25     INFO - Running pre-action listener: _resource_record_pre_action
12:59:25     INFO - Running main action method: read_buildbot_config
12:59:25     INFO - Using buildbot properties:
12:59:25     INFO - {
12:59:25     INFO -     "properties": {
...
12:59:25     INFO -     }, 
12:59:25     INFO -     "sourcestamp": {
...
12:59:25     INFO -         "changes": [
12:59:25     INFO -             {
...
12:59:25     INFO -                 "comments": "Bug 1187082 - Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data. r=jlund",
Chris, would you be fine with me just delete the "sourcestamp" key, before dumping to the log, here:
https://mxr.mozilla.org/build-central/source/mozharness/mozharness/mozilla/buildbot.py#65

The revision is the key piece of information, and it's already visible under the separate "properties" key (see example log URL in comment 0, and search for "read-buildbot-config").
Flags: needinfo?(catlee)
Yeah, it's probably fine to strip this. I only hesitate because it's possible that something is relying on the comments (especially on try), and stripping them out of the properties makes reproducing the problem a bit trickier.

Would prefixing the output with something that TH knows to ignore work instead?
Flags: needinfo?(catlee)
(In reply to Chris AtLee [:catlee] from comment #3)
> Yeah, it's probably fine to strip this. I only hesitate because it's
> possible that something is relying on the comments (especially on try), and
> stripping them out of the properties makes reproducing the problem a bit
> trickier.

The revision is still there - so the commit messages can be retrieved from Hg.

The file is now at:
https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/buildbot.py#64

A new example log is:
http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-linux/1447695400/mozilla-inbound-linux-bm73-build1-build1301.txt.gz

And object being dumped:

{
    "properties": {
        "buildnumber": 1301, 
        "product": "firefox", 
        "script_repo_revision": "production", 
        "branch": "mozilla-inbound", 
        "repository": "", 
        "buildername": "Linux mozilla-inbound build", 
        "buildid": "20151116093640", 
        "basedir": "/builds/slave/m-in-lx-0000000000000000000000", 
        "project": "", 
        "platform": "linux", 
        "master": "http://buildbot-master73.bb.releng.usw2.mozilla.com:8001/", 
        "builduid": "465b712c34b64d1388ea3831eaaea7ae", 
        "scheduler": "mozilla-inbound-firefox", 
        "repo_path": "integration/mozilla-inbound", 
        "slavename": "bld-linux64-spot-373", 
        "commit_titles": [
            "Bug 1223734 - AudioChannelService should not be re-initialized after the XPCOM shutdown, r=smaug"
        ], 
        "revision": "b22d1abe0289b6bf6ab30c03015a6553e7aefd1d"
    }, 
    "sourcestamp": {
        "repository": "", 
        "hasPatch": false, 
        "project": "", 
        "branch": "integration/mozilla-inbound", 
        "changes": [
            {
                "category": null, 
                "files": [
                    {
                        "url": null, 
                        "name": "dom/audiochannel/AudioChannelAgent.cpp"
                    }, 
                    ...
                    {
                        "url": null, 
                        "name": "dom/system/gonk/AudioManager.cpp"
                    }
                ], 
                "repository": "", 
                "rev": "b22d1abe0289b6bf6ab30c03015a6553e7aefd1d", 
                "who": "amarchesini@mozilla.com", 
                "when": 1447695342, 
                "number": 6666881, 
                "comments": "Bug 1223734 - AudioChannelService should not be re-initialized after the XPCOM shutdown, r=smaug", 
                "project": "", 
                "at": "Mon 16 Nov 2015 09:35:42", 
                "branch": "integration/mozilla-inbound", 
                "revlink": "https://hg.mozilla.org/integration/mozilla-inbound/rev/b22d1abe0289b6bf6ab30c03015a6553e7aefd1d", 
                "properties": [
                    [
                        "commit_titles", 
                        [
                            "Bug 1223734 - AudioChannelService should not be re-initialized after the XPCOM shutdown, r=smaug"
                        ], 
                        "BaseHgPoller"
                    ]
                ], 
                "revision": "b22d1abe0289b6bf6ab30c03015a6553e7aefd1d"
            }
        ], 
        "revision": "b22d1abe0289b6bf6ab30c03015a6553e7aefd1d"
    }
}
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Instead of the entire config object being dumped to the log (see comment 4), now only the 'properties' key within is output, and even then only after the
'commit_titles' sub-key is deleted.
Attachment #8688017 - Flags: review?(catlee)
(sorry forgot the qref)
Attachment #8688017 - Attachment is obsolete: true
Attachment #8688017 - Flags: review?(catlee)
Attachment #8688019 - Flags: review?(catlee)
Chris, any idea when you might be able to review this? :-)
Flags: needinfo?(catlee)
Attachment #8688019 - Flags: review?(catlee) → review+
Flags: needinfo?(catlee)
https://hg.mozilla.org/mozilla-central/rev/2c0d64cea673
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: