From 0ac7b9170ad91fdf0b1934371a36409316445279 Mon Sep 17 00:00:00 2001 From: Nick Mitchell Date: Mon, 10 Apr 2023 09:57:57 -0400 Subject: [PATCH] fix: index events by worker ordinal, as we attempt to do for log lines also try not to show heartbeat events over and over again --- package-lock.json | 34 +--- .../plugin-codeflare-dashboard/package.json | 2 - .../src/components/Dashboard/Timeline.tsx | 23 ++- .../src/controller/dashboard/index.ts | 6 +- .../src/controller/dashboard/options.ts | 2 +- .../src/controller/dashboard/status/Live.ts | 175 ++++++++++-------- .../controller/dashboard/utilization/Live.ts | 7 +- plugins/plugin-codeflare/package.json | 2 +- 8 files changed, 130 insertions(+), 121 deletions(-) diff --git a/package-lock.json b/package-lock.json index cdf0985f..c913b6ab 100644 --- a/package-lock.json +++ b/package-lock.json @@ -932,9 +932,9 @@ "license": "MIT" }, "node_modules/@guidebooks/store": { - "version": "7.5.17", - "resolved": "https://registry.npmjs.org/@guidebooks/store/-/store-7.5.17.tgz", - "integrity": "sha512-lu0xCUCC2/peFd8HwV1ifA/Y5KqMNK93dSZslhTjdNGRLe3DxrZuPvDb1/V/UUzTGXQR/y3Q3ht8U/yHW3dEtQ==" + "version": "7.5.18", + "resolved": "https://registry.npmjs.org/@guidebooks/store/-/store-7.5.18.tgz", + "integrity": "sha512-X/mcC8eVlaNQVZl75o08BvnshUJIFypVe50AtFwfMuxy7xW7tk+uwv8FHeYpQSuPNSu+xt8NXfSrAES3jxkE5w==" }, "node_modules/@humanwhocodes/config-array": { "version": "0.11.8", @@ -1891,12 +1891,6 @@ "@types/unist": "*" } }, - "node_modules/@types/heap": { - "version": "0.2.31", - "resolved": "https://registry.npmjs.org/@types/heap/-/heap-0.2.31.tgz", - "integrity": "sha512-cKL2veQoMEQGGSib6GTq7Of6mAX1e6gH/7F9ddE4+qsga/RRK/ThAkZcKF/i1yVc30ZxlY2r6vZ74NZNtAA0Yw==", - "dev": true - }, "node_modules/@types/html-minifier-terser": { "version": "6.1.0", "dev": true, @@ -14614,7 +14608,7 @@ }, "plugins/plugin-client-default": { "name": "@kui-shell/plugin-client", - "version": "4.4.11", + "version": "4.5.0", "license": "Apache-2.0" }, "plugins/plugin-codeflare": { @@ -14622,7 +14616,7 @@ "version": "0.0.1", "license": "Apache-2.0", "dependencies": { - "@guidebooks/store": "^7.5.17", + "@guidebooks/store": "^7.5.18", "@logdna/tail-file": "^3.0.1", "@patternfly/react-charts": "^6.94.18", "@patternfly/react-core": "^4.276.6", @@ -14651,14 +14645,12 @@ "dependencies": { "@logdna/tail-file": "^3.0.1", "chokidar": "^3.5.3", - "heap": "^0.2.7", "ink": "^3.2.0", "madwizard": "^8.0.13", "pretty-ms": "^7.0.1", "strip-ansi": "6.0.0" }, "devDependencies": { - "@types/heap": "^0.2.31", "@types/split2": "^3.2.1", "react-devtools-core": "^4.27.4" } @@ -15289,9 +15281,9 @@ "dev": true }, "@guidebooks/store": { - "version": "7.5.17", - "resolved": "https://registry.npmjs.org/@guidebooks/store/-/store-7.5.17.tgz", - "integrity": "sha512-lu0xCUCC2/peFd8HwV1ifA/Y5KqMNK93dSZslhTjdNGRLe3DxrZuPvDb1/V/UUzTGXQR/y3Q3ht8U/yHW3dEtQ==" + "version": "7.5.18", + "resolved": "https://registry.npmjs.org/@guidebooks/store/-/store-7.5.18.tgz", + "integrity": "sha512-X/mcC8eVlaNQVZl75o08BvnshUJIFypVe50AtFwfMuxy7xW7tk+uwv8FHeYpQSuPNSu+xt8NXfSrAES3jxkE5w==" }, "@humanwhocodes/config-array": { "version": "0.11.8", @@ -15534,7 +15526,7 @@ "@kui-shell/plugin-codeflare": { "version": "file:plugins/plugin-codeflare", "requires": { - "@guidebooks/store": "^7.5.17", + "@guidebooks/store": "^7.5.18", "@logdna/tail-file": "^3.0.1", "@patternfly/react-charts": "^6.94.18", "@patternfly/react-core": "^4.276.6", @@ -15569,10 +15561,8 @@ "version": "file:plugins/plugin-codeflare-dashboard", "requires": { "@logdna/tail-file": "^3.0.1", - "@types/heap": "^0.2.31", "@types/split2": "^3.2.1", "chokidar": "^3.5.3", - "heap": "^0.2.7", "ink": "^3.2.0", "madwizard": "^8.0.13", "pretty-ms": "^7.0.1", @@ -16059,12 +16049,6 @@ "@types/unist": "*" } }, - "@types/heap": { - "version": "0.2.31", - "resolved": "https://registry.npmjs.org/@types/heap/-/heap-0.2.31.tgz", - "integrity": "sha512-cKL2veQoMEQGGSib6GTq7Of6mAX1e6gH/7F9ddE4+qsga/RRK/ThAkZcKF/i1yVc30ZxlY2r6vZ74NZNtAA0Yw==", - "dev": true - }, "@types/html-minifier-terser": { "version": "6.1.0", "dev": true diff --git a/plugins/plugin-codeflare-dashboard/package.json b/plugins/plugin-codeflare-dashboard/package.json index 535ea35d..77d866c4 100644 --- a/plugins/plugin-codeflare-dashboard/package.json +++ b/plugins/plugin-codeflare-dashboard/package.json @@ -23,14 +23,12 @@ "access": "public" }, "devDependencies": { - "@types/heap": "^0.2.31", "@types/split2": "^3.2.1", "react-devtools-core": "^4.27.4" }, "dependencies": { "@logdna/tail-file": "^3.0.1", "chokidar": "^3.5.3", - "heap": "^0.2.7", "ink": "^3.2.0", "madwizard": "^8.0.13", "pretty-ms": "^7.0.1", diff --git a/plugins/plugin-codeflare-dashboard/src/components/Dashboard/Timeline.tsx b/plugins/plugin-codeflare-dashboard/src/components/Dashboard/Timeline.tsx index 3c30e940..2ddf6535 100644 --- a/plugins/plugin-codeflare-dashboard/src/components/Dashboard/Timeline.tsx +++ b/plugins/plugin-codeflare-dashboard/src/components/Dashboard/Timeline.tsx @@ -33,12 +33,13 @@ export default class Timeline extends React.PureComponent { latest: "▏", } + /** This will help us compute whether we are about to overflow terminal width. */ private get maxLabelLength() { return this.props.gridModels.filter((_) => !_.isQualitative).reduce((N, { title }) => Math.max(N, title.length), 0) } /** @return max number of time cells, across all grids and all workers */ - private nTimeCells() { + private get nTimeCells() { // outer loop: iterate across grids return this.props?.workers.reduce((nTimes, _) => { if (Array.isArray(_) && _.length > 0) { @@ -95,17 +96,13 @@ export default class Timeline extends React.PureComponent { return } - const nTimes = this.nTimeCells() + // timeStartIdx: once we overflow the viewport's width, we display + // the suffix of history information, starting at this index + // /- we have a 1-cell margin between row labels and timeline cells + const timeStartIdx = Math.abs(Math.max(0, 1 + this.nTimeCells + this.maxLabelLength - process.stdout.columns)) + // total number of cells in the model -/ \- room for row labels \- fit in viewport - // to help us compute whether we are about to overflow terminal width - const maxLabelLength = this.props.gridModels.reduce((N, spec) => { - return Math.max(N, spec.title.length) - }, 0) - - // once we overflow, display the suffix of history information, starting at this index - const timeStartIdx = Math.abs(Math.max(0, nTimes + maxLabelLength - process.stdout.columns)) - - if (nTimes === 0) { + if (this.nTimeCells === 0) { // none of the grids have any temporal information, yet return } else { @@ -113,7 +110,9 @@ export default class Timeline extends React.PureComponent { return ( {this.props.workers.map((workers, gridIdx) => ( - {this.timeline(workers, this.props.gridModels[gridIdx], nTimes, timeStartIdx)} + + {this.timeline(workers, this.props.gridModels[gridIdx], this.nTimeCells, timeStartIdx)} + ))} ) diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/index.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/index.ts index a149927e..cc01e287 100644 --- a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/index.ts +++ b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/index.ts @@ -35,6 +35,10 @@ export type Options = Arguments["parsedOptions"] & { p: string profile: string theme: string + + /** Frequency of updates to the timeline, in seconds */ + u: number + "update-frequency": number } /** Behave like top, where the screen is cleared just for this process */ @@ -153,7 +157,7 @@ export default async function dashboard(args: Arguments, cmd: "db" | "d } const historyConfig: HistoryConfig = { - width: 5000, + width: args.parsedOptions.u ? 1000 * args.parsedOptions.u : 5000, } const db = dashboardUI(profile, jobId, await gridForA(kind, historyConfig), { scale }) diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/options.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/options.ts index 2cfcf16b..6d0ca1ea 100644 --- a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/options.ts +++ b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/options.ts @@ -35,5 +35,5 @@ export default Options export const flags = { boolean: ["demo"], - alias: { events: ["e"], lines: ["l"], theme: ["t"], demo: ["d"], scale: ["s"] }, + alias: { events: ["e"], lines: ["l"], theme: ["t"], demo: ["d"], scale: ["s"], "update-frequency": ["u"] }, } diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/status/Live.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/status/Live.ts index 61f6d7ce..d99641bb 100644 --- a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/status/Live.ts +++ b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/status/Live.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import Heap from "heap" +import chalk from "chalk" import ansiRegex from "ansi-regex" import stripAnsi from "strip-ansi" import type { TextProps } from "ink" @@ -28,7 +28,12 @@ import type { OnData, Worker } from "../../../components/Dashboard/types.js" import { rankFor, stateFor } from "./states.js" -type Event = { line: string; stateRank: number; timestamp: number } +type Lined = { line: string } +type Timestamped = { timestamp: number } +type Identified = { id: T } + +/** Event record */ +type Event = Identified & Timestamped & Lined & { stateRank: number } /** * Keep track of a local timestamp so we can prioritize and show the @@ -37,7 +42,12 @@ type Event = { line: string; stateRank: number; timestamp: number } * we received it. Perhaps suboptimal, but we cannot guarantee that * random log lines from applications are timestamped. */ -type LogLineRecord = { id: string; line: string; timestamp: number } +type LogLineRecord = Identified & + Timestamped & + Lined & { + /** Keep track of whether we've already shown a "heartbeat" message for each worker */ + alreadySeenHeartbeat: boolean + } /** * Maintain a model of live data from a given set of file streams @@ -45,30 +55,20 @@ type LogLineRecord = { id: string; line: string; timestamp: number } * */ export default class Live { - /** Model of status per worker */ - private readonly workers: Record = {} + /** Give an ordinal identifier [0, numWorkers) to a worker name */ + private ordinals: Record = {} + + /** Model of status per worker, indexed by the worker's ordinal */ + private readonly workers: Worker[] = [] /** Number of lines of event output to retain. TODO this depends on height of terminal? */ private static readonly MAX_HEAP = 1000 - /** Model of logLines. TODO circular buffer and obey options.lines */ + /** Model of logLines */ private logLine: Record = {} - /** Model of the lines of output */ - private readonly events = new Heap((a, b) => { - if (a.line === b.line) { - // later timestamps get higher priority - return b.timestamp - a.timestamp - } - - const stateDiff = a.stateRank - b.stateRank - if (stateDiff !== 0) { - return stateDiff - } else { - // later timestamps get higher priority - return b.timestamp - a.timestamp - } - }) + /** Model of the lines of output, indexed by worker ordinal */ + private readonly events: Event[] = [] public constructor( historyConfig: HistoryConfig, @@ -84,7 +84,7 @@ export default class Live { if (data) { if (tail.kind === "logs") { // handle a log line - this.pushLineAndPublish(stripColors(data), cb) + this.pushLineAndPublish(data, cb) return } @@ -110,9 +110,11 @@ export default class Live { return } else { const update = (name: string) => { - if (!this.workers[name]) { + const ordinal = this.ordinalOf(name) + + if (!this.workers[ordinal]) { // never seen this named worker before - this.workers[name] = { + this.workers[ordinal] = { name, metric, metricHistory: [], @@ -120,24 +122,24 @@ export default class Live { lastUpdate: timestamp, style: styleOf[metric], } - } else if (this.workers[name].lastUpdate <= timestamp) { + } else if (this.workers[ordinal].lastUpdate <= timestamp) { // we have seen it before, update the metric value and // timestamp; note that we only update the model if our // timestamp is after the lastUpdate for this worker - this.workers[name].metric = metric - this.workers[name].lastUpdate = timestamp - this.workers[name].style = styleOf[metric] + this.workers[ordinal].metric = metric + this.workers[ordinal].lastUpdate = timestamp + this.workers[ordinal].style = styleOf[metric] } else { // out of date event, drop it return } - this.pushEvent(data, metric, timestamp) + this.pushEvent(ordinal, data, metric, timestamp) } if (name === "*") { // this event affects every worker - Object.keys(this.workers).forEach(update) + this.workers.forEach((_) => update(_.name)) } else { // this event affects a specific worker update(name) @@ -146,7 +148,7 @@ export default class Live { // inform the UI that we have updates cb({ events: this.importantEvents(), - workers: Object.values(this.workers), + workers: this.workers, }) } } @@ -156,18 +158,6 @@ export default class Live { }) } - /** @return the most important events, to be shown in the UI */ - private importantEvents() { - if (this.opts.events === 0) { - return [] - } else { - return this.events - .toArray() - .slice(0, this.opts.events || 8) // 8 highest priority - .sort((a, b) => a.timestamp - b.timestamp) // sorted by time - } - } - /** Replace any timestamps with a placeholder, so that the UI can use a "5m ago" style */ private timestamped(line: string) { return line.replace(/\s*(\d\d\d\d-\d\d-\d\dT\d\d:\d\d:\d\d(\.\d+)?Z)\s*/, "{timestamp}") @@ -184,68 +174,97 @@ export default class Live { private stripPageClear(line: string) { // eslint-disable-next-line no-control-regex return line.replace(/\x1b\x5B\[2J/g, "") + // ^^^ [2J is part of clear screen; we don't want those to flow through } + /** Strip off offending elements, such as certain ansi control characters */ private prepareLineForUI(line: string) { - return this.stripPageClear(this.stripWorkerName(this.timestamped(line))) + return stripColors(this.stripPageClear(this.stripWorkerName(this.timestamped(line)))) } - private readonly lookup: Record = {} - - /** Add `line` to our heap `this.events` */ - private pushEvent(line: string, metric: WorkerState, timestamp: number) { - const key = this.prepareLineForUI(line) - - // ^^^ [2J is part of clear screen; we don't want those to flow through - + /** Add `line` to our `this.model */ + private pushEvent(ordinal: number, rawLine: string, metric: WorkerState, timestamp: number) { const rec = { + id: ordinal, timestamp, stateRank: rankFor[metric], - line: stripColors(key), + line: this.ordinalPrefix(ordinal) + this.prepareLineForUI(rawLine), } - const already = this.lookup[rec.line] - if (already) { - already.timestamp = timestamp - this.events.updateItem(already) + this.events[ordinal] = rec + } + + /** This helps us parse out a [W5] style prefix for loglines, so we can intuit the worker id of the log line */ + private readonly workerIdPattern = new RegExp("^(" + ansiRegex().source + ")?\\[([^\\]]+)\\]") + + /** @return the [0,numWorkers) ordinal for the given `workerName` */ + private ordinalOf(workerName: string): number { + if (workerName in this.ordinals) { + return this.ordinals[workerName] } else { - this.lookup[rec.line] = rec - if (this.events.size() >= Live.MAX_HEAP) { - this.events.replace(rec) - } else { - this.events.push(rec) - } + return (this.ordinals[workerName] = Object.keys(this.ordinals).length) } + } - /* if (this.opts.events === 0) { - return [] - } else { - return this.importantEvents() - } */ + /** See if we already have extracted an ordinal in the logline */ + private ordinalFromLine(logLine: string) { + const match = logLine.match(this.workerIdPattern) + return match ? match[2] : "notsure" } - /** This helps us parse out a [W5] style prefix for loglines, so we can intuit the worker id of the log line */ - private readonly workerIdPattern = new RegExp("^(" + ansiRegex().source + ")?\\[([^\\]]+)\\]") + /** @return a pretty signifier of a worker's ordinal */ + private ordinalPrefix(ordinal: number) { + return chalk.bold.yellow(`[W${ordinal}] `) + } - private readonly timeSorter = (a: LogLineRecord, b: LogLineRecord): number => { + private readonly timeSorter = (a: Timestamped, b: Timestamped): number => { return a.timestamp - b.timestamp } - private readonly idSorter = (a: LogLineRecord, b: LogLineRecord): number => { + private readonly idSorterS = (a: Identified, b: Identified): number => { return a.id.localeCompare(b.id) } + private readonly idSorterN = (a: Identified, b: Identified): number => { + return a.id - b.id + } + + /** @return the most important events, to be shown in the UI */ + private importantEvents() { + if (this.opts.events === 0) { + // user asked to show no events + return [] + } else { + const k = this.opts.events || 8 + return ( + this.events + // .sort(this.timeSorter) + .slice(0, k) + .sort(this.idSorterN) + ) + } + } + /** Add the given `line` to our logLines model and pass the updated model to `cb` */ private pushLineAndPublish(logLine: string, cb: OnData) { if (logLine) { - // here we avoid a flood of React renders by batching them up a - // bit; i thought react 18 was supposed to help with this. hmm. - const match = logLine.match(this.workerIdPattern) - const id = match ? match[2] : "notsure" + const id = this.ordinalFromLine(logLine) if (id) { + const isHeartbeat = /Job is active/.test(logLine) + const alreadySeenHeartbeat = this.logLine[id] && this.logLine[id].alreadySeenHeartbeat + if (isHeartbeat && alreadySeenHeartbeat) { + // don't repeat the point... + return + } + const idx = logLine.lastIndexOf(" ") const timestamp = idx < 0 ? undefined : this.asMillisSinceEpoch(stripAnsi(logLine.slice(idx + 1))) - this.logLine[id] = { id, line: this.prepareLineForUI(logLine), timestamp: timestamp || Date.now() } + this.logLine[id] = { + id, + line: this.prepareLineForUI(logLine), + timestamp: timestamp || Date.now(), + alreadySeenHeartbeat: alreadySeenHeartbeat || isHeartbeat, + } // display the k most recent logLines per worker, ordering the display by worker id const k = 4 @@ -253,7 +272,7 @@ export default class Live { logLine: Object.values(this.logLine) .sort(this.timeSorter) // so we can pick off the most recent .slice(0, k) // now pick off the k most recent - .sort(this.idSorter), // sort those k by worker id, so there is a consistent ordering in the UI + .sort(this.idSorterS), // sort those k by worker id, so there is a consistent ordering in the UI }) } } diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/utilization/Live.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/utilization/Live.ts index 3f27b27c..3939c838 100644 --- a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/utilization/Live.ts +++ b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/utilization/Live.ts @@ -34,6 +34,11 @@ export default class Live { /** Model of status per worker */ private readonly workers: Record = {} + /** + * Map a [0,100] number-as-string to a `metricIdx`, which is an + * index into the `states` array, and also return the parsed integer + * `value`. + */ private stateFor(util: string) { const value = parseInt(util.replace(/%$/, ""), 10) const bucketWidth = ~~(100 / states.length) @@ -86,7 +91,7 @@ export default class Live { if (!name || !timestamp) { // console.error("Bad status record", line) return - } else if ((name.includes("/") && !/^pod\//.test(name)) || /cleaner/.test(name)) { + } else if (!/^pod\//.test(name) || /cleaner/.test(name)) { // only track pod events, and ignore our custodial pods return } else { diff --git a/plugins/plugin-codeflare/package.json b/plugins/plugin-codeflare/package.json index a35cd55e..e4de0114 100644 --- a/plugins/plugin-codeflare/package.json +++ b/plugins/plugin-codeflare/package.json @@ -30,7 +30,7 @@ "@types/split2": "^3.2.1" }, "dependencies": { - "@guidebooks/store": "^7.5.17", + "@guidebooks/store": "^7.5.18", "@logdna/tail-file": "^3.0.1", "@patternfly/react-charts": "^6.94.18", "@patternfly/react-core": "^4.276.6",