missing test failure emails from try

RESOLVED FIXED in mozilla52

Status

defect
RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: karlt, Assigned: bstack)

Tracking

unspecified
mozilla52
Dependency tree / graph

Details

Attachments

(4 attachments, 1 obsolete attachment)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0fa70104f55
dispatched failure emails for many of the failures, including the Linux(32) test failures, but none of the linux64 test failures.
I had the same experience in https://treeherder.mozilla.org/#/jobs?repo=try&revision=46e35bb7f807: I received failure emails for the 2 buildbot failures, but none for the 4 taskcluster failures (same failure, same test).

--

tryserver@build.mozilla.org
	
5:37 PM (17 hours ago)
		
to me
Your try Server test (46e35bb7f807) completed with warnings on builder try_ubuntu64_vm_test-mochitest-1.

Summary of test results:

The full log for this test run is available at http://archive.mozilla.org/pub/firefox/try-builds/gbrown@mozilla.com-46e35bb7f807b2b0f908f6d654f7874e82f6a3c5/try-linux64/try_ubuntu64_vm_test-mochitest-1-bm122-tests1-linux64-build313.txt.gz.

For an overview of all results see Treeherder.
tryserver@build.mozilla.org
	
6:05 PM (16 hours ago)
		
to me
Your try Server test (46e35bb7f807) completed with warnings on builder try_ubuntu64_vm_test-mochitest-1.

Summary of test results:

The full log for this test run is available at http://archive.mozilla.org/pub/firefox/try-builds/gbrown@mozilla.com-46e35bb7f807b2b0f908f6d654f7874e82f6a3c5/try-linux64/try_ubuntu64_vm_test-mochitest-1-bm122-tests1-linux64-build314.txt.gz.

For an overview of all results see Treeherder.
Do you get an email per failure on a try push?
Is the current experience for Buildbot good enough?
I actually haven't used failure emails much, historically. But in my limited experience, yes, one email is sent for each buildbot test job failure on a try push, and yes, I think that's good enough.

I found https://dxr.mozilla.org/build-central/source/buildbotcustom/bin/try_mailer.py for buildbot support.
(In reply to Armen Zambrano [:armenzg] - Engineering productivity from comment #2)
> Do you get an email per failure on a try push?

Yes, when it works, which I assume is when buildbot is used.

> Is the current experience for Buildbot good enough?

Yes.
See Also: → 1088238
This feature activates when someone uses -f (for email failures) in their try syntax:
> try: -b o -f -p linux64 -u reftest-1 -t none

This feature was used 78 times in the last 2 months (May 8th). That's about 8-9 times a week:
https://hg.mozilla.org/try/pushloghtml?startdate=2+months+ago&enddate=now
Brian, maybe this is something we can add to the decision task now that we have tc-notify?
Assignee: nobody → bstack
So long as we know who to email, then it's just a matter of adding a route that is 'notify.email.<email>.on-failed' and 'notify.email.<email>.on-exception' to every task we want to know the status of (which sounds like all of them?).
Yeah, sounds perfect!

I think that could best be implemented within the task description transforms in taskcluster/taskgraph/transforms/task.py
Attachment #8794909 - Attachment is obsolete: true
Attachment #8794909 - Flags: review?(dustin)
Comment on attachment 8794906 [details]
Bug 1275774 - Fix taskcluster how-to docs example command

https://reviewboard.mozilla.org/r/80592/#review79924
Attachment #8794906 - Flags: review?(dustin) → review+
Comment on attachment 8794908 [details]
Bug 1275774 - Add notify routes to taskcluster tasks for certain try flags

https://reviewboard.mozilla.org/r/80596/#review79932

Just some minor fixes!

::: taskcluster/taskgraph/target_tasks.py:63
(Diff revision 2)
>                  task.attributes['task_duplicates'] = options.trigger_tests
>  
> +    # Add notifications here as well
> +    if options.notifications:
> +        for task in full_task_graph:
> +            owner = task.task.get('tags', {}).get('createdForUser')

I'd prefer to get this from the source (`parameters['owner']`) rather than depend on this tag.  TBH I have no idea what tags are or how they are used, and I suspect only jlal really does, so that tag may disappear someday.

::: taskcluster/taskgraph/target_tasks.py:65
(Diff revision 2)
> +    # Add notifications here as well
> +    if options.notifications:
> +        for task in full_task_graph:
> +            owner = task.task.get('tags', {}).get('createdForUser')
> +            routes = task.task.get('routes')
> +            if not (owner or routes):

If `routes` is `[]`, we still want to add the routes, I think.  Although I can hardly see a case where this would occur, since we have index and TH routes.  Perhaps the line before should read `routes = task.task.setdefault('routes', [])` and `routes` should be removed from this conditional.
Attachment #8794908 - Flags: review?(dustin) → review-
Comment on attachment 8794907 [details]
Bug 1275774 - Add try-syntax parsing of email notifications to taskcluster

https://reviewboard.mozilla.org/r/80594/#review79938
Attachment #8794907 - Flags: review?(dustin) → review+
Comment on attachment 8794908 [details]
Bug 1275774 - Add notify routes to taskcluster tasks for certain try flags

https://reviewboard.mozilla.org/r/80596/#review79932

> I'd prefer to get this from the source (`parameters['owner']`) rather than depend on this tag.  TBH I have no idea what tags are or how they are used, and I suspect only jlal really does, so that tag may disappear someday.

I initially grabbed the owner from that field as well, but that resulted in some of them having an owner of aki iirc. Specifically the signer builds or whatever. If that's desired functionality that aki gets emails on those tasks finishing, then that makes sense, but using this field does appear to do The Right Thing™. Is it possible that the bug here is with aki's task generation stuff? Does it really want him to be the owner of the task every time no matter whether he runs it or not?
I suspect you saw jlund, from the android build-signing task.  Yes, that's a bug in the task.  However, I'm suggesting using *parameters*, which contains the inputs to the decision task (including the pushing user), rather than task['metadata']['owner'] (which, for all tasks except the build-signing tasks, is copied from parameters['owner'])
Comment on attachment 8794908 [details]
Bug 1275774 - Add notify routes to taskcluster tasks for certain try flags

https://reviewboard.mozilla.org/r/80596/#review80340
Attachment #8794908 - Flags: review?(dustin) → review+
Comment on attachment 8795768 [details]
Bug 1275774 - Add tests for new taskcluster try flag parsing

https://reviewboard.mozilla.org/r/81710/#review80342

yay, tests!
Attachment #8795768 - Flags: review?(dustin) → review+
Great. Now that there are a bunch of little green boxes in mozreview. The next thing we need to figure out is assigning scopes to allow this to be used. 

https://github.com/taskcluster/mozilla-taskcluster/pull/101 is how I think we can do this for try. If that is the correct way of doing it then we can land that and then land this and we're set I believe. 

For moz-tree:level:2 and moz-tree:level:3, I figure we can give `route.notify.*` to those roles. Does that make sense?
Flags: needinfo?(dustin)
Try doesn't run on levels 2 or 3, so this patch won't ever result in task graphs at those levels sending notifications.  That said, I think adding those scopes in case someone wants to use them someday (via some landed patch in the taskgraph generation) is a good idea.
Flags: needinfo?(dustin)
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d596d23a7efe
Fix taskcluster how-to docs example command r=dustin
https://hg.mozilla.org/integration/autoland/rev/5202c2328850
Add try-syntax parsing of email notifications to taskcluster r=dustin
https://hg.mozilla.org/integration/autoland/rev/229984f2cc93
Add notify routes to taskcluster tasks for certain try flags r=dustin
https://hg.mozilla.org/integration/autoland/rev/afffe391d026
Add tests for new taskcluster try flag parsing r=dustin
Component: Integration → Services
You need to log in before you can comment on or make changes to this bug.