[Findit] Using ts_mon to track swarming/isolated server outages

[Findit] Using ts_mon to track swarming/isolated server outages

Swarming server/isolated servers can go down and become unreachable, in which case Findit cannot continue to trigger/monitor swarming tasks whose results are used by try jobs and the flake checker. This change is to include monitoring using ts_mon so such outages can be monitored more effectively.

BUG=669215
Committed: https://chromium.googlesource.com/infra/infra/+/dc8bb5ccaee0bffbf50ebeba97b9b3f32af27eda

[lijeffrey, 2016-12-01 22:23:11]

Description was changed from

==========
[Findit] (Do not commit) Using ts_mon to track swarming/isolated server outages

BUG=
==========

to

==========
[Findit] Using ts_mon to track swarming/isolated server outages

Not 100% confident on this change yet, so for now adding (Do not commit) to
description

Swarming server/isolated servers can go down and become unreachable, in which
case Findit cannot continue to trigger/monitor swarming tasks whose results are
used by try jobs and the flake checker. This change is to include monitoring
using ts_mon so such outages can be monitored more effectively.

BUG=669215
==========

[lijeffrey, 2016-12-01 22:23:11]

lijeffrey@chromium.org changed reviewers:
+ chanli@chromium.org, stgao@chromium.org

[stgao, 2016-12-01 22:25:15]

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/monitoring.py (right):

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/monitoring.py:11: 'findit/swarmingserverfailures',
Maybe unify these two metrics as 'findit/httperrors', and use a field called
host for different server like Swarming, Isolate, buildbot, etc.

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/swarming_util.py (right):

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/swarming_util.py:90: exception_type_name =
type(exception).__name__
Why not pass over the exception name from code below instead?

Usually __ means not expected to be used out of the class.

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/swarming_util.py:92: if isolated_server_host in url:
Extract the host from the url, and make it a field instead.

[lijeffrey, 2016-12-01 22:54:34]

comments addressed, ptal

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/monitoring.py (right):

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/monitoring.py:11: 'findit/swarmingserverfailures',
On 2016/12/01 22:25:15, stgao (slow on Monday) wrote:
> Maybe unify these two metrics as 'findit/httperrors', and use a field called
> host for different server like Swarming, Isolate, buildbot, etc.

Done.

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/swarming_util.py (right):

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/swarming_util.py:90: exception_type_name =
type(exception).__name__
On 2016/12/01 22:25:15, stgao (slow on Monday) wrote:
> Why not pass over the exception name from code below instead?
> 
> Usually __ means not expected to be used out of the class.

Done.

https://codereview.chromium.org/2547713002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/swarming_util.py:92: if isolated_server_host in url:
On 2016/12/01 22:25:15, stgao (slow on Monday) wrote:
> Extract the host from the url, and make it a field instead.

Done.

[stgao, 2016-12-01 23:04:00]

lgtm after comments are addressed

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/monitoring.py (right):

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/monitoring.py:11: 'findit/httperrors',
description='Failed http calls to various servers')
nit: calls -> requests.

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/swarming_util.py (right):

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_util.py:90: if not host:  # pragma: no cover
Do an assert instead?

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_util.py:158: except Exception as e:  #
pragma: no cover
Should we have a error logging for this?

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/test/swarming_util_test.py (right):

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/test/swarming_util_test.py:689:
swarming_util._OnConnectionFailed(url, DeadlineExceededError)
I guess we don't need this test anymore?
At least it is not that effective as nothing is verified.

[lijeffrey, 2016-12-01 23:24:27]

Description was changed from

==========
[Findit] Using ts_mon to track swarming/isolated server outages

Not 100% confident on this change yet, so for now adding (Do not commit) to
description

Swarming server/isolated servers can go down and become unreachable, in which
case Findit cannot continue to trigger/monitor swarming tasks whose results are
used by try jobs and the flake checker. This change is to include monitoring
using ts_mon so such outages can be monitored more effectively.

BUG=669215
==========

to

==========
[Findit] Using ts_mon to track swarming/isolated server outages

Swarming server/isolated servers can go down and become unreachable, in which
case Findit cannot continue to trigger/monitor swarming tasks whose results are
used by try jobs and the flake checker. This change is to include monitoring
using ts_mon so such outages can be monitored more effectively.

BUG=669215
==========

[lijeffrey, 2016-12-01 23:25:30]

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/monitoring.py (right):

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/monitoring.py:11: 'findit/httperrors',
description='Failed http calls to various servers')
On 2016/12/01 23:03:59, stgao (slow on Monday) wrote:
> nit: calls -> requests.

Done.

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/swarming_util.py (right):

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_util.py:90: if not host:  # pragma: no cover
On 2016/12/01 23:03:59, stgao (slow on Monday) wrote:
> Do an assert instead?

Done.

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/swarming_util.py:158: except Exception as e:  #
pragma: no cover
On 2016/12/01 23:03:59, stgao (slow on Monday) wrote:
> Should we have a error logging for this?

Done.

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
File appengine/findit/waterfall/test/swarming_util_test.py (right):

https://codereview.chromium.org/2547713002/diff/40001/appengine/findit/waterf...
appengine/findit/waterfall/test/swarming_util_test.py:689:
swarming_util._OnConnectionFailed(url, DeadlineExceededError)
On 2016/12/01 23:03:59, stgao (slow on Monday) wrote:
> I guess we don't need this test anymore?
> At least it is not that effective as nothing is verified.

Done.

[stgao, 2016-12-01 23:27:51]

lgtm with nit

https://codereview.chromium.org/2547713002/diff/60001/appengine/findit/waterf...
File appengine/findit/waterfall/monitoring.py (right):

https://codereview.chromium.org/2547713002/diff/60001/appengine/findit/waterf...
appengine/findit/waterfall/monitoring.py:11: 'findit/httperrors',
description='Failed http requests to various servers')
nit: rename metric to "outgoinghttperrors" to be more clear?

[lijeffrey, 2016-12-01 23:34:08]

https://codereview.chromium.org/2547713002/diff/60001/appengine/findit/waterf...
File appengine/findit/waterfall/monitoring.py (right):

https://codereview.chromium.org/2547713002/diff/60001/appengine/findit/waterf...
appengine/findit/waterfall/monitoring.py:11: 'findit/httperrors',
description='Failed http requests to various servers')
On 2016/12/01 23:27:51, stgao (slow on Monday) wrote:
> nit: rename metric to "outgoinghttperrors" to be more clear?

Done.

[lijeffrey, 2016-12-01 23:34:12]

The CQ bit was checked by lijeffrey@chromium.org

[lijeffrey, 2016-12-01 23:34:13]

The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org
Link to the patchset: https://codereview.chromium.org/2547713002/#ps80001
(title: "Fixing nit")

[commit-bot: I haz the power, 2016-12-01 23:34:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-01 23:49:10]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1480635252712980,
"parent_rev": "bd2c26251ce33bcf72306da6ec073b46ba40f797", "commit_rev":
"dc8bb5ccaee0bffbf50ebeba97b9b3f32af27eda"}

[commit-bot: I haz the power, 2016-12-01 23:49:13]

Description was changed from

==========
[Findit] Using ts_mon to track swarming/isolated server outages

Swarming server/isolated servers can go down and become unreachable, in which
case Findit cannot continue to trigger/monitor swarming tasks whose results are
used by try jobs and the flake checker. This change is to include monitoring
using ts_mon so such outages can be monitored more effectively.

BUG=669215
==========

to

==========
[Findit] Using ts_mon to track swarming/isolated server outages

Swarming server/isolated servers can go down and become unreachable, in which
case Findit cannot continue to trigger/monitor swarming tasks whose results are
used by try jobs and the flake checker. This change is to include monitoring
using ts_mon so such outages can be monitored more effectively.

BUG=669215

Committed:
https://chromium.googlesource.com/infra/infra/+/dc8bb5ccaee0bffbf50ebeba97b9b...
==========

[commit-bot: I haz the power, 2016-12-01 23:49:14]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/infra/infra/+/dc8bb5ccaee0bffbf50ebeba97b9b...