1455698 - Don't notify for cancelled tasks

Status: RESOLVED FIXED

(Taskcluster :: Services, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

If you enable notification on exception, you currently get notified when a task is cancelled (as that is a kind of exception).

This ends up sending a lot of notifications that are usually no good.

Could we refine this a little so that "cancel" is treated differently from other exceptions?

Comment 1

[Aki Sasaki (not active)]

We may also want to treat `deadline-exceeded` differently... if we have staging releases that we *don't* cancel, thousands of tasks will hit `deadline-exceeded` at roughly the same time.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Maybe we should add "exception.cancelled", "exception.deadline-exceeded", etc. (for all of the reasons) and users can either use "exception" (all exceptions) or the specific reasons they are interested in.

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

See also https://github.com/taskcluster/taskcluster-notify/issues/23

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Jonas points out that this would mean a lot of routes.  Perhaps the best thing to do is to extend
  https://github.com/taskcluster/taskcluster-notify/pull/29 (which just ignores cancel)
to also ignore deadline-exceeded.

Comment 5

[Jonas Finnemann Jensen (:jonasfj)]

Or, we could add:
  task.extra.notify.ignoredReasonResolved = [...]

Or just re-think what it is we want to notify one on.
If your tasks are expiring by deadline you have a problem, like:
 - your cron job isn't running
 - workerType doesn't exist (name was misspelled)
 - workerType is constantly overloaded
 - workerType is not spinning up on demand

---
Perhaps, it's better to update taskcluster-queue to resolve tasks with:
  state: exception
  reasonResolved: 'dependency-unsatisfied'
if we reach a state where task.dependencies can't be satisfied.
Probably, we would have to update queue.rerunTask, such that dependencies resolved 'dependency-unsatisfied' are also rerun.

Just, saying maybe the right place to solve this isn't tc-notify.

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Given the current uses of tc-notify (which are pretty minimal), I think it makes sense to make a minimal fix to this, that applies a hard-coded blacklist of reasonResolved that don't result in a notification -- cancelled and deadline-exceeded, for the moment.  What you've suggested might be a good additional fix to the queue, but perhaps not something we need right now.

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

https://github.com/taskcluster/taskcluster-notify/pull/31

Comment 8

[Nick Thomas [:nthomas] (UTC+12)]

Dustin, we've removed almost all our use of tc-notify in bug 145911. There are just a few notify kinds which send ~3 emails per release, and only on success of the underlying dummy builds. Just wanted to check if there were any temporary blocks/fixes which could be undone now.

Comment 9

[Nick Thomas [:nthomas] (UTC+12)]

Whoops, make that bug 1459116.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Yeah, we could certainly remove the blacklist.  That happened in bug 1458863.