Closed
Bug 1188351
Opened 9 years ago
Closed 9 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)
Release Engineering
General
Tracking
(firefox46 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: emorley, Assigned: emorley)
References
Details
(Keywords: treeherder)
Attachments
(1 file, 1 obsolete file)
2.68 KB,
patch
|
catlee
:
review+
|
Details | Diff | Splinter Review |
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",
Assignee | ||
Comment 1•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
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)
Assignee | ||
Comment 4•9 years ago
|
||
(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
Assignee | ||
Comment 5•9 years ago
|
||
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)
Assignee | ||
Comment 6•9 years ago
|
||
(sorry forgot the qref)
Attachment #8688017 -
Attachment is obsolete: true
Attachment #8688017 -
Flags: review?(catlee)
Attachment #8688019 -
Flags: review?(catlee)
Assignee | ||
Comment 7•9 years ago
|
||
Chris, any idea when you might be able to review this? :-)
Flags: needinfo?(catlee)
Updated•9 years ago
|
Attachment #8688019 -
Flags: review?(catlee) → review+
Updated•9 years ago
|
Flags: needinfo?(catlee)
Comment 9•9 years ago
|
||
bugherder |
Updated•7 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•