52357be3947a4643b735276094b8d01da8e3f8f0 braney Tue Sep 8 10:30:02 2026 -0700 ottoMonitor: check the last closed grace window, and let a job be owned by whoever is on duty Three changes, all from comments on the ticket. Lou: civic has no individual owner, so it belongs to the otto person. The owner column of ottoOwners.tsv now accepts ottoOnDuty, which the monitor resolves from the ottoOnDuty header at run time, so the rotation stays a one-line edit. The ticket body says the job has no individual owner, and the same person is not added as a watcher twice when the owner is also the person on duty. The grace window is now measured back from the last scheduled time whose window has already closed, instead of forward from the latest scheduled time. Written the other way, a daily job scheduled fewer than graceHours before the monitor's own 12:15 run could never be reported late, because every check landed inside a fresh window. Six of the forty jobs were in that hole: clinGen, genArkPushRR, grcIncidentDb, liftRequest, omim and pubtatorDbSnp. Max: a uniprot run can take days, and how long depends on the size of the release. Its stamp is created by a > redirect when the run starts, so the grace does not have to cover the run length, and a fresh stamp does not mean the run worked. That limit is now written down in the stamps table and the README, with the live case: the uniprot run dies after about 37 minutes on a missing lxml and has produced no output since January 2025, while the monitor reads it as on time. refs #38101 diff --git src/hg/utils/otto/ottoMonitor/ottoMonitor.py src/hg/utils/otto/ottoMonitor/ottoMonitor.py index 618587b0e10..d0fbde9c06d 100755 --- src/hg/utils/otto/ottoMonitor/ottoMonitor.py +++ src/hg/utils/otto/ottoMonitor/ottoMonitor.py @@ -37,30 +37,35 @@ import subprocess import sys from datetime import datetime, timedelta selfDir = os.path.dirname(os.path.abspath(__file__)) defaultOwners = "/hive/data/outside/otto/ottoMonitor/ottoOwners.tsv" defaultStamps = os.path.join(selfDir, "ottoMonitorStamps.tsv") defaultState = "/hive/data/outside/otto/ottoMonitor/state.json" redmineCli = os.path.expanduser("~/kent/src/utils/redmineCli") # A run stamp is allowed to be this stale before the job counts as late, on top # of the job's own graceHours. Covers clock skew and a cron that starts slow. extraGraceMinutes = 15 +# A job with no individual owner carries this in the owner column of +# ottoOwners.tsv, and belongs to whoever the ottoOnDuty header names. Lou set +# that rule for civic on #38101, 2026-09-08. +onDutyOwner = "ottoOnDuty" + dowNames = {"sun": 0, "mon": 1, "tue": 2, "wed": 3, "thu": 4, "fri": 5, "sat": 6} def parseCronField(field, lo, hi, names=None): """Expand one cron field into a set of ints. Handles *, a,b,c, a-b and */n.""" values = set() for part in field.split(","): step = 1 if "/" in part: part, stepText = part.split("/", 1) step = int(stepText) if part == "*": first, last = lo, hi elif "-" in part.strip("-"): firstText, lastText = part.split("-", 1) @@ -251,109 +256,124 @@ def loadState(path): if os.path.exists(path): with open(path) as fh: return(json.load(fh)) return({}) def saveState(path, state): os.makedirs(os.path.dirname(path), exist_ok=True) tmp = path + ".tmp" with open(tmp, "w") as fh: json.dump(state, fh, indent=2, sort_keys=True) os.rename(tmp, path) -def checkJob(job, owners, stamps, now): +def checkJob(job, owners, stamps, now, onDuty=None): """Everything known about one job's last run. Returns a dict.""" ownerRow = owners[job] stampRow = stamps.get(job) - result = {"job": job, "owner": ownerRow[1], "sourceUrl": ownerRow[5], + owner = ownerRow[1] + ownerIsOnDuty = owner == onDutyOwner + if ownerIsOnDuty: + owner = onDuty or "?" + result = {"job": job, "owner": owner, "ownerIsOnDuty": ownerIsOnDuty, + "sourceUrl": ownerRow[5], "cron": ownerRow[6], "verdict": "ok", "detail": ""} if stampRow is None: result["verdict"] = "unlisted" result["detail"] = "in the crontab with no row in ottoMonitorStamps.tsv" return(result) - stampGlob, graceHours = stampRow[1], stampRow[2] + stampGlob, graceHours = stampRow[1], float(stampRow[2]) if stampGlob == "-": result["verdict"] = "blind" result["detail"] = stampRow[3] if len(stampRow) > 3 else "no run stamp" return(result) - due = prevScheduledRun(result["cron"], now) + # Check against the most recent scheduled time whose grace window has + # already CLOSED, not against the latest one. Measuring the grace forward + # from the latest scheduled time leaves a job permanently unflaggable + # whenever its scheduled hour is less than graceHours before this script's + # own run time, because every check then lands inside a fresh grace window. + # Six of the forty were in that hole at the 12:15 cron: clinGen, + # genArkPushRR, grcIncidentDb, liftRequest, omim and pubtatorDbSnp. + due = prevScheduledRun(result["cron"], + now - timedelta(hours=graceHours, minutes=extraGraceMinutes)) if due is None: result["verdict"] = "unparsed" result["detail"] = "could not parse cron spec %r" % result["cron"] return(result) lastRun = newestMtime(stampGlob) result["due"] = due.strftime("%Y-%m-%d %H:%M") result["lastRun"] = lastRun.strftime("%Y-%m-%d %H:%M") if lastRun else "never" - deadline = due + timedelta(hours=float(graceHours), minutes=extraGraceMinutes) if lastRun is not None and lastRun >= due: return(result) - if now < deadline: - result["detail"] = "due %s, still inside its grace window" % result["due"] - return(result) result["verdict"] = "late" late = now - due result["detail"] = "no run stamp since %s, due %s, %d hours late" % ( result["lastRun"], result["due"], late.total_seconds() // 3600) return(result) def classifyLate(result): """A late job is only somebody's bug if its source is actually up.""" up, detail = sourceIsUp(result["sourceUrl"]) result["probe"] = detail if up is False: result["verdict"] = "sourceDown" else: result["verdict"] = "realFailure" return(result) def fileTicket(result, onDuty, dryRun): """One GB Bug per failing job, to whoever is running otto, with the job's owner as a watcher and named in the body.""" owner = result["owner"] subject = "otto job %s has not run since %s" % (result["job"], result["lastRun"]) - ownerLine = ("The recorded owner of this job is %s." % owner if owner != "?" - else "This job has no recorded owner in ottoOwners.tsv.") + if result.get("ownerIsOnDuty"): + ownerLine = "This job has no individual owner, so it belongs to whoever is running otto." + elif owner != "?": + ownerLine = "The recorded owner of this job is %s." % owner + else: + ownerLine = "This job has no recorded owner in ottoOwners.tsv." body = "\n".join([ "The otto failure monitor found this job late. Refs #38101.", "", ownerLine, "", "Schedule: %s" % result["cron"], "Last run stamp: %s" % result["lastRun"], "Due: %s" % result.get("due", "-"), "Source check: %s" % result.get("probe", "-"), ]) cmd = [redmineCli, "create", "--project", "genomebrowser", "--tracker", "Bug", "--subject", subject, "--description", body] if onDuty: cmd += ["--assigned-to", onDuty] if dryRun: print(" would file a ticket, run with --file to do it: %s" % subject) return(None) done = subprocess.run(cmd, capture_output=True, text=True) print(done.stdout.strip()) - for name in [n for n in (owner, onDuty) if n and n != "?"]: + # dict.fromkeys keeps the order and drops the duplicate when the job's owner + # is the person on duty + for name in dict.fromkeys(n for n in (owner, onDuty) if n and n != "?"): ticketId = "".join(c for c in done.stdout.split("#")[-1][:6] if c.isdigit()) if ticketId: subprocess.run([redmineCli, "watch", ticketId, name], capture_output=True, text=True) return(done.stdout.strip()) def main(): parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) parser.add_argument("--owners", default=defaultOwners, help="ottoOwners.tsv") parser.add_argument("--stamps", default=defaultStamps, help="ottoMonitorStamps.tsv") parser.add_argument("--state", default=defaultState, help="state.json") parser.add_argument("--file", action="store_true", help="file a ticket for a real failure. Off by default") @@ -365,31 +385,31 @@ args = parser.parse_args() owners, comments = readTable(args.owners, 10) stamps, _ = readTable(args.stamps, 3) onDuty = findOnDuty(comments) now = datetime.now() state = {} if args.no_state else loadState(args.state) watched = [j for j, row in owners.items() if row[4] == "yes" and (args.job is None or j == args.job)] if args.job and not watched: sys.exit("no watched job named %s" % args.job) late, blind, other, fine = [], [], [], [] for job in sorted(watched): - result = checkJob(job, owners, stamps, now) + result = checkJob(job, owners, stamps, now, onDuty) if result["verdict"] == "late": result = classifyLate(result) entry = state.setdefault(job, {}) previous = entry.get("verdict") if result["verdict"] in ("sourceDown", "realFailure"): entry["strikes"] = entry.get("strikes", 0) + 1 if previous == result["verdict"] else 1 late.append(result) else: entry["strikes"] = 0 entry["verdict"] = result["verdict"] entry["lastRun"] = result.get("lastRun", "-") entry["checked"] = now.strftime("%Y-%m-%d %H:%M") result["strikes"] = entry["strikes"] if result["verdict"] == "blind": blind.append(result)