1160188 - Autophone - support new Treeherder log submission and error summary

Status: RESOLVED FIXED

(Testing Graveyard :: Autophone, defect)

Description

[Bob Clary [:bc] (inactive)]

bug 1080760 introduced the ability to use add_log_reference with a log marked as already parsed and also added the ability to post the error summary directly to Treeherder. Autophone should be modified to use these new features.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: tls.json

camd, I'm trying to add support for the new text_log_summary, but if I add this to the th job artifacts via tj.add_artifact('text_log_summary', 'json', text_log_summary), it prevents the Job Info panel from displaying content or even being stored I think. It also loses track of the fact that I've added logs via add_log_reference.

If you look at the tests in https://treeherder.allizom.org/#/jobs?repo=mozilla-inbound&revision=fc99c65e5956&filter-searchStr=autophone

You can see examples of where I did add the text_log_summary artifact and where I did not.

What's up?

Comment 2

[Cameron Dawson [:camd]]

our code is expecting a text_log_summary artifact to have a ``job_guid`` field in it.  Without that, it's barfing in the code.  We SHOULD be able to just get that from the posted job.

Due to some code that's shared from a few different possible workflows, this allows for submitting lots of artifacts for several different jobs at a time.  But we can work that out.  Just need some extra handling

Comment 3

[Bob Clary [:bc] (inactive)]

Attachment: bug-1160188-patch-1-v1.patch

This patch incorporates changes for the new thclient and the automatic creation of the bug suggestions from the text_log_summary from bug 1080760. The addition of the text_log_summary artifact is currently commented out due to a bug in the current implmentation in bug 1080760 but this is sufficient to start with.

This is to be applied after patch bug-1161631-patch-6-v1.patch from bug 1161631

Comment 4

[Cameron Dawson [:camd]]

BC: added a new bug 1170201 that blocks this.  I have a PR for that and will submit it shortly.

Comment 5

[Geoff Brown [:gbrown]]

Comment on attachment 8613412 [details] [diff] [review]
bug-1160188-patch-1-v1.patch

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

::: autophonetreeherder.py
@@ +86,5 @@
> +                    job_collection)
> +                return
> +            except requests.exceptions.Timeout:
> +                msg = ('Attempt %d to post result to '
> +                       'Treeherder timed out.\n\n\n' % attempt)

Would it be useful to log any other details here? Server? Amount of time waited??

@@ +93,5 @@
>                      self.mailer.send('Attempt %d for Phone %s failed to post to Treeherder' %
>                                       (attempt, machine), msg)
>                  time.sleep(self.retry_wait)
> +            except Exception, e:
> +                logger.exception('Error submitting request to Treeherder')

Consider including "attempt %d" in the error message here.

@@ +95,5 @@
>                  time.sleep(self.retry_wait)
> +            except Exception, e:
> +                logger.exception('Error submitting request to Treeherder')
> +                if self.mailer:
> +                    self.mailer.send('Error submitting request to Treeherder',

...and here.

@@ +103,5 @@
> +                                         machine,
> +                                         e,
> +                                         job_collection.to_json()))
> +                return
> +        logger.error('Error submitting request to Treeherder')

..and/or changing this to distinguish from the exception case, which currently has the identical error message. "Unable to submit request to TH after %d tries"?

Comment 6

[Bob Clary [:bc] (inactive)]

Attachment: bug-1160188-patch-2-v1.patch

enable text_log_summary with line, linenumber attributes, fix log uploading. My previous patch where it cleared the log at the end of the job was incorrect. This version clears the log after the upload to s3 for each test.

This is to be applied after patch 9 in bug 1161631 comment 23

camd: as you can see in https://treeherder.allizom.org/#/jobs?repo=mozilla-inbound&revision=e8d8ea70caed&filter-searchStr=autophone the artifacts are loaded ok, but the Failure summary is never updated.

You can also see that even for tests with success, the Failure summary keeps reloading trying to get the Failure summary.

Comment 7

[Cameron Dawson [:camd]]

So the code was expecting the blob to be a json text string, rather than a dict.  It barfed if it was a dict because the json parser would fail.  The code should handle both.  So I'm making the modification to do that.

Comment 8

[Geoff Brown [:gbrown]]

Comment on attachment 8615820 [details] [diff] [review]
bug-1160188-patch-2-v1.patch

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

::: autophonetreeherder.py
@@ +466,5 @@
>                      line = '%s | %s' % (test, text)
>                  elif text:
>                      line = text
> +                # XXX Need to update the log parser to return the line
> +                # numbers of the errors.

Don't forget! :)

Comment 9

[Bob Clary [:bc] (inactive)]

https://github.com/mozilla/autophone/commit/95cc7e4191fb49717a1426fd132bb476cad28d9b

I went with the json.dumps for now.  Looks good.

https://treeherder.allizom.org/#/jobs?repo=mozilla-inbound&filter-searchStr=autophone&fromchange=001de2286250&startdate=2015-06-06