From 823853d36420800675071fc50be07f25dd561f7b Mon Sep 17 00:00:00 2001 From: Bryan Burgers Date: Fri, 16 Aug 2019 11:50:15 -0500 Subject: [PATCH 1/2] Include merge_sha in more metadata comments Include the `merge_sha` in BuildFailed, TryBuildFailed, and TimedOut metadata comments. This helps with synchronizing historical data, and will help in the future correlating which build a failure is associated with. --- homu/comments.py | 6 +++--- homu/main.py | 3 ++- homu/server.py | 15 ++++++++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/homu/comments.py b/homu/comments.py index d289931..8c88ac8 100644 --- a/homu/comments.py +++ b/homu/comments.py @@ -119,7 +119,7 @@ def render(self): class BuildFailed(Comment): - params = ["builder_url", "builder_name"] + params = ["builder_url", "builder_name", "merge_sha"] def render(self): return ":broken_heart: Test failed - [%s](%s)" % ( @@ -128,7 +128,7 @@ def render(self): class TryBuildFailed(Comment): - params = ["builder_url", "builder_name"] + params = ["builder_url", "builder_name", "merge_sha"] def render(self): return ":broken_heart: Test failed - [%s](%s)" % ( @@ -137,7 +137,7 @@ def render(self): class TimedOut(Comment): - params = [] + params = ["merge_sha"] def render(self): return ":boom: Test timed out" diff --git a/homu/main.py b/homu/main.py index 6d136fa..7cc8a06 100644 --- a/homu/main.py +++ b/homu/main.py @@ -381,6 +381,7 @@ def timed_out(): def timed_out(self): print('* Test timed out: {}'.format(self)) + merge_sha = self.merge_sha self.merge_sha = '' self.save() self.set_status('failure') @@ -392,7 +393,7 @@ def timed_out(self): '', 'Test timed out', context='homu') - self.add_comment(comments.TimedOut()) + self.add_comment(comments.TimedOut(merge_sha=merge_sha)) self.change_labels(LabelEvent.TIMED_OUT) def record_retry_log(self, src, body): diff --git a/homu/server.py b/homu/server.py index eeca4e4..82c4990 100644 --- a/homu/server.py +++ b/homu/server.py @@ -581,6 +581,8 @@ def fail(err): except ValueError: return 'OK' + sha = info['sha'] + status_name = "" if 'status' in repo_cfg: for name, value in repo_cfg['status'].items(): @@ -596,7 +598,7 @@ def fail(err): if row['name'] == state.base_ref: return 'OK' - report_build_res(info['state'] == 'success', info['target_url'], + report_build_res(info['state'] == 'success', sha, info['target_url'], 'status-' + status_name, state, logger, repo_cfg) elif event_type == 'check_run': @@ -605,6 +607,8 @@ def fail(err): except ValueError: return 'OK' + sha = info['check_run']['head_sha'] + current_run_name = info['check_run']['name'] checks_name = None if 'checks' in repo_cfg: @@ -624,6 +628,7 @@ def fail(err): report_build_res( info['check_run']['conclusion'] == 'success', + sha, info['check_run']['details_url'], 'checks-' + checks_name, state, logger, repo_cfg, @@ -632,7 +637,7 @@ def fail(err): return 'OK' -def report_build_res(succ, url, builder, state, logger, repo_cfg): +def report_build_res(succ, sha, url, builder, state, logger, repo_cfg): lazy_debug(logger, lambda: 'build result {}: builder = {}, succ = {}, current build_res = {}' # noqa .format(state, builder, succ, @@ -700,12 +705,14 @@ def report_build_res(succ, url, builder, state, logger, repo_cfg): state.add_comment(comments.TryBuildFailed( builder_url=url, builder_name=builder, + merge_sha=sha, )) state.change_labels(LabelEvent.TRY_FAILED) else: state.add_comment(comments.BuildFailed( builder_url=url, builder_name=builder, + merge_sha=sha, )) state.change_labels(LabelEvent.FAILED) @@ -730,6 +737,8 @@ def buildbot(): if not props['revision']: continue + sha = props['revision'] + try: state, repo_label = find_state(props['revision']) except ValueError: @@ -808,7 +817,7 @@ def buildbot(): else: logger.error('Corrupt payload from Buildbot') - report_build_res(build_succ, url, info['builderName'], + report_build_res(build_succ, sha, url, info['builderName'], state, logger, repo_cfg) elif row['event'] == 'buildStarted': From 6c7056254d6664b4280c52f48b38fad9176ae329 Mon Sep 17 00:00:00 2001 From: Bryan Burgers Date: Fri, 16 Aug 2019 12:12:55 -0500 Subject: [PATCH 2/2] Include start/end times in metadata comments Timing information could be gleaned from comment times, but make it explicit in the metadata. This means we can get the true times, even if GitHub has delays. This also gives us the opportunity to have more control in the future. For example, the [check_run event][check_run] includes `started_at` and `completed_at` information that is independent from when GitHub sent us the webhook. [check_run]: https://developer.github.com/v3/activity/events/types/#checkrunevent-api-payload --- homu/comments.py | 14 +++++++------- homu/main.py | 12 ++++++++++-- homu/server.py | 9 ++++++++- homu/utils.py | 8 ++++++++ 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/homu/comments.py b/homu/comments.py index 8c88ac8..8ce60ef 100644 --- a/homu/comments.py +++ b/homu/comments.py @@ -72,7 +72,7 @@ def render(self): class BuildStarted(Comment): - params = ["head_sha", "merge_sha"] + params = ["head_sha", "merge_sha", "started_at"] def render(self): return ":hourglass: Testing commit %s with merge %s..." % ( @@ -81,7 +81,7 @@ def render(self): class TryBuildStarted(Comment): - params = ["head_sha", "merge_sha"] + params = ["head_sha", "merge_sha", "started_at"] def render(self): return ":hourglass: Trying commit %s with merge %s..." % ( @@ -90,7 +90,7 @@ def render(self): class BuildCompleted(Comment): - params = ["approved_by", "base_ref", "builders", "merge_sha"] + params = ["approved_by", "base_ref", "builders", "merge_sha", "ended_at"] def render(self): urls = ", ".join( @@ -107,7 +107,7 @@ def render(self): class TryBuildCompleted(Comment): - params = ["builders", "merge_sha"] + params = ["builders", "merge_sha", "ended_at"] def render(self): urls = ", ".join( @@ -119,7 +119,7 @@ def render(self): class BuildFailed(Comment): - params = ["builder_url", "builder_name", "merge_sha"] + params = ["builder_url", "builder_name", "merge_sha", "ended_at"] def render(self): return ":broken_heart: Test failed - [%s](%s)" % ( @@ -128,7 +128,7 @@ def render(self): class TryBuildFailed(Comment): - params = ["builder_url", "builder_name", "merge_sha"] + params = ["builder_url", "builder_name", "merge_sha", "ended_at"] def render(self): return ":broken_heart: Test failed - [%s](%s)" % ( @@ -137,7 +137,7 @@ def render(self): class TimedOut(Comment): - params = ["merge_sha"] + params = ["merge_sha", "ended_at"] def render(self): return ":boom: Test timed out" diff --git a/homu/main.py b/homu/main.py index 7cc8a06..46bc80b 100644 --- a/homu/main.py +++ b/homu/main.py @@ -8,7 +8,10 @@ from . import utils from .parse_issue_comment import parse_issue_comment from .auth import verify as verify_auth -from .utils import lazy_debug +from .utils import ( + iso_utc_now, + lazy_debug, +) import logging from threading import Thread, Lock, Timer import time @@ -393,7 +396,10 @@ def timed_out(self): '', 'Test timed out', context='homu') - self.add_comment(comments.TimedOut(merge_sha=merge_sha)) + self.add_comment(comments.TimedOut( + merge_sha=merge_sha, + ended_at=iso_utc_now(), + )) self.change_labels(LabelEvent.TIMED_OUT) def record_retry_log(self, src, body): @@ -1302,11 +1308,13 @@ def start_build(state, repo_cfgs, buildbot_slots, logger, db, git_cfg): state.add_comment(comments.TryBuildStarted( head_sha=state.head_sha, merge_sha=state.merge_sha, + started_at=iso_utc_now(), )) else: state.add_comment(comments.BuildStarted( head_sha=state.head_sha, merge_sha=state.merge_sha, + started_at=iso_utc_now(), )) return True diff --git a/homu/server.py b/homu/server.py index 82c4990..877d31b 100644 --- a/homu/server.py +++ b/homu/server.py @@ -11,7 +11,10 @@ ) from . import comments from . import utils -from .utils import lazy_debug +from .utils import ( + iso_utc_now, + lazy_debug, +) import github3 import jinja2 import requests @@ -659,6 +662,7 @@ def report_build_res(succ, sha, url, builder, state, logger, repo_cfg): base_ref=state.base_ref, builders={k: v["url"] for k, v in state.build_res.items()}, merge_sha=state.merge_sha, + ended_at=iso_utc_now(), )) state.change_labels(LabelEvent.SUCCEED) try: @@ -690,6 +694,7 @@ def report_build_res(succ, sha, url, builder, state, logger, repo_cfg): state.add_comment(comments.TryBuildCompleted( builders={k: v["url"] for k, v in state.build_res.items()}, merge_sha=state.merge_sha, + ended_at=iso_utc_now(), )) state.change_labels(LabelEvent.TRY_SUCCEED) @@ -706,6 +711,7 @@ def report_build_res(succ, sha, url, builder, state, logger, repo_cfg): builder_url=url, builder_name=builder, merge_sha=sha, + ended_at=iso_utc_now(), )) state.change_labels(LabelEvent.TRY_FAILED) else: @@ -713,6 +719,7 @@ def report_build_res(succ, sha, url, builder, state, logger, repo_cfg): builder_url=url, builder_name=builder, merge_sha=sha, + ended_at=iso_utc_now(), )) state.change_labels(LabelEvent.FAILED) diff --git a/homu/utils.py b/homu/utils.py index 48a03c6..cb8cbde 100644 --- a/homu/utils.py +++ b/homu/utils.py @@ -6,6 +6,14 @@ import traceback import requests import time +import datetime + + +def iso_utc_now(): + return datetime.datetime \ + .utcnow() \ + .replace(tzinfo=datetime.timezone.utc, microsecond=0) \ + .isoformat() def github_set_ref(repo, ref, sha, *, force=False, auto_create=True, retry=1):