Closed Bug 1187082 Opened 4 years ago Closed 4 years ago

Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data

Categories

(Testing :: Talos, defect)

defect
Not set

Tracking

(firefox42 fixed, firefox43 fixed)

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: wlach, Assigned: parkouss)

References

Details

Attachments

(2 files)

It looks like the mozharness changes in bug 1172918 made it so that talos no longer prints the TALOSDATA line, which is needed by perfherder to be able to ingest data.

I single out the mozharness changes specifically as it doesn't look like the changes to talos have yet been deployed (talos.json is still pointing to deca965654a3, which is one revision earlier: http://hg.mozilla.org/build/talos/log)
Kind of unusual, but the patch to fix this is in bug 1186844 comment 7.
whilst this is fixed on inbound, should we duplicate this bug to bug 1186844, or make it so this bug tracks ensuring we always have talosdata json available?
I'm ok with changing the purpose of this bug to track the talosdata availability.
Summary: Changes to talos broke perfherder data ingestion → Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data
So, I've been working on this, here is a first version.

I pushed it to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=823d8730663c

this testing against my talos copy:

http://www.bytebucket.org/parkouss/talos/commits/branch/default

The first c (green) was the execution on the commit:

 e47c566 Bug 1186844 - ...

The orange one was the execution on the next commit:

 dea26e9 break perfherder data ingestion

That commit is based on the regression we had. :)


So this works great! And in the error log you can see at the end: CRITICAL - No talos data in output!
in http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/j.parkouss@gmail.com-823d8730663c/try-linux64/try_ubuntu64_hw_test-chromez-bm105-tests1-linux-build514.txt.gz

Not sure if/how I can make the error more obvious from a treeherder point of view - but at least here the build fail and we have a CRITICAL message in the log (the message can be improved though - maybe "Missing TALOSDATA output for PerfHerder!" or something).
Assignee: nobody → j.parkouss
Status: NEW → ASSIGNED
Attachment #8638934 - Flags: review?(wlachance)
Comment on attachment 8638934 [details] [diff] [review]
harness_talos_data_ingestion_failure.patch

This looks good, but I'm not the right person to review talos mozharness patches. Based on revision history, it looks like :jlund was the most recent person to touch this file so I'll leave this to him.

Thanks for doing this!
Attachment #8638934 - Flags: review?(wlachance)
Attachment #8638934 - Flags: review?(jlund)
Attachment #8638934 - Flags: feedback+
Comment on attachment 8638934 [details] [diff] [review]
harness_talos_data_ingestion_failure.patch

Review of attachment 8638934 [details] [diff] [review]:
-----------------------------------------------------------------

this looks great. My knowledge of mh talos is weak at best but there is not likely many who are still around that would know more.

::: testing/mozharness/mozharness/mozilla/testing/talos.py
@@ +47,5 @@
> +        super(TalosOutputParser, self).__init__(**kwargs)
> +        self.minidump_output = None
> +        self.found_talosdata = False
> +
> +    def update_worst_log_and_tbpl_levels(self, log_level, tbpl_level):

thanks for getting rid of some duplication
Attachment #8638934 - Flags: review?(jlund) → review+
https://hg.mozilla.org/mozilla-central/rev/ea838fbd5518
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Duplicate of this bug: 1188123
So unfortunately the regex in here is not exactly the same as the one used in perfherder, e.g:

https://github.com/mozilla/treeherder/blob/master/treeherder/log_parser/parsers.py#L452

This is now blocking for bug 1196415, since we change the output format of talos.
Blocks: 1196415
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Bug 1187082 - Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data. r=wlach

This synchronise the TALOSDATA regex with the one used in perfherder.
Also this ensure that we only see that message once.
Attachment #8656200 - Flags: review?(wlachance)
Comment on attachment 8656200 [details]
MozReview Request: Bug 1187082 - Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data. r=wlach

https://reviewboard.mozilla.org/r/18115/#review16203

::: testing/mozharness/mozharness/mozilla/testing/talos.py:50
(Diff revision 1)
> -        self.found_talosdata = False
> +        self.found_talosdata = 0

Maybe this should be `num_times_found_talosdata` ?

This looks fine except for one minor improvement.
Attachment #8656200 - Flags: review?(wlachance)
Comment on attachment 8656200 [details]
MozReview Request: Bug 1187082 - Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data. r=wlach

Bug 1187082 - Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data. r=wlach

This synchronise the TALOSDATA regex with the one used in perfherder.
Also this ensure that we only see that message once.
Attachment #8656200 - Flags: review?(wlachance)
Comment on attachment 8656200 [details]
MozReview Request: Bug 1187082 - Ensure talos always produces TALOSDATA json structure in logs so perfherder can ingest data. r=wlach

https://reviewboard.mozilla.org/r/18115/#review16205
Attachment #8656200 - Flags: review?(wlachance) → review+
https://hg.mozilla.org/mozilla-central/rev/f0a54b1e9b60
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.