From 1ce2b10a1063a07224983b6902bdbaf6caa00e6d Mon Sep 17 00:00:00 2001 From: circlecrystalin Date: Fri, 16 Jan 2026 18:33:13 +0100 Subject: [PATCH] Address code review feedback: refactor update function, fix parameter order, use constants, improve numeric value handling, and add database migration --- db/knex_init_db.js | 1 + ...-28-0001-add-numeric-value-to-heartbeat.js | 12 + server/model/monitor.js | 7 +- server/monitor-types/snmp.js | 2 +- .../socket-handlers/chart-socket-handler.js | 3 + server/uptime-calculator.js | 338 +++++++++++------- 6 files changed, 224 insertions(+), 139 deletions(-) create mode 100644 db/knex_migrations/2026-01-28-0001-add-numeric-value-to-heartbeat.js diff --git a/db/knex_init_db.js b/db/knex_init_db.js index 9d8341495..eed1de070 100644 --- a/db/knex_init_db.js +++ b/db/knex_init_db.js @@ -140,6 +140,7 @@ async function createTables() { table.integer("ping"); table.integer("duration").notNullable().defaultTo(0); table.integer("down_count").notNullable().defaultTo(0); + table.float("numeric_value").nullable().defaultTo(null).comment("Numeric value from monitor check (e.g., from JSON query or SNMP)"); table.index("important"); table.index(["monitor_id", "time"], "monitor_time_index"); diff --git a/db/knex_migrations/2026-01-28-0001-add-numeric-value-to-heartbeat.js b/db/knex_migrations/2026-01-28-0001-add-numeric-value-to-heartbeat.js new file mode 100644 index 000000000..28b446687 --- /dev/null +++ b/db/knex_migrations/2026-01-28-0001-add-numeric-value-to-heartbeat.js @@ -0,0 +1,12 @@ +exports.up = function (knex) { + return knex.schema.alterTable("heartbeat", function (table) { + table.float("numeric_value").nullable().defaultTo(null).comment("Numeric value from monitor check (e.g., from JSON query or SNMP)"); + }); +}; + +exports.down = function (knex) { + return knex.schema.alterTable("heartbeat", function (table) { + table.dropColumn("numeric_value"); + }); +}; + diff --git a/server/model/monitor.js b/server/model/monitor.js index 1fa9fa89a..aadb7927b 100644 --- a/server/model/monitor.js +++ b/server/model/monitor.js @@ -715,7 +715,7 @@ class Monitor extends BeanModel { bean.status = UP; bean.msg = `JSON query passes (comparing ${response} ${this.jsonPathOperator} ${this.expectedValue})`; // Extract numeric value for aggregation (will be passed to uptime calculator) - bean.numeric_value = this.extractNumericValue(response); + bean.numeric_value = this.parseToNumberOrNull(response); } else { throw new Error( `JSON query does not pass (comparing ${response} ${this.jsonPathOperator} ${this.expectedValue})` @@ -1097,8 +1097,7 @@ class Monitor extends BeanModel { // Calculate uptime let uptimeCalculator = await UptimeCalculator.getUptimeCalculator(this.id); - // Extract numeric value from bean if available (set by json-query or snmp monitors) - const numericValue = bean.numeric_value !== undefined ? bean.numeric_value : null; + const numericValue = bean.numeric_value ?? null; let endTimeDayjs = await uptimeCalculator.update(bean.status, parseFloat(bean.ping), numericValue); bean.end_time = R.isoDateTimeMillis(endTimeDayjs); @@ -2075,7 +2074,7 @@ class Monitor extends BeanModel { * @param {any} value Value to check and potentially extract * @returns {number|null} Numeric value or null */ - extractNumericValue(value) { + parseToNumberOrNull(value) { // Check if value is numeric (number or string that can be converted to number) let numericValue = null; diff --git a/server/monitor-types/snmp.js b/server/monitor-types/snmp.js index 86613bcf9..7926a3ca1 100644 --- a/server/monitor-types/snmp.js +++ b/server/monitor-types/snmp.js @@ -56,7 +56,7 @@ class SNMPMonitorType extends MonitorType { heartbeat.status = UP; heartbeat.msg = `JSON query passes (comparing ${response} ${monitor.jsonPathOperator} ${monitor.expectedValue})`; // Extract numeric value for aggregation (will be passed to uptime calculator) - heartbeat.numeric_value = monitor.extractNumericValue(response); + heartbeat.numeric_value = monitor.parseToNumberOrNull(response); } else { throw new Error( `JSON query does not pass (comparing ${response} ${monitor.jsonPathOperator} ${monitor.expectedValue})` diff --git a/server/socket-handlers/chart-socket-handler.js b/server/socket-handlers/chart-socket-handler.js index 9dc9537c8..63c1725c5 100644 --- a/server/socket-handlers/chart-socket-handler.js +++ b/server/socket-handlers/chart-socket-handler.js @@ -61,10 +61,13 @@ module.exports.chartSocketHandler = (socket) => { // Filter and convert to format expected by frontend // Only include entries with numeric values + // Use numeric_value as the main value, with min/max for reference const data = dataArray .filter((entry) => entry.avgNumeric !== null && entry.avgNumeric !== undefined) .map((entry) => ({ value: parseFloat(entry.avgNumeric), + min: entry.minNumeric !== null && entry.minNumeric !== undefined ? parseFloat(entry.minNumeric) : null, + max: entry.maxNumeric !== null && entry.maxNumeric !== undefined ? parseFloat(entry.maxNumeric) : null, timestamp: entry.timestamp, time: dayjs.unix(entry.timestamp).utc().format("YYYY-MM-DD HH:mm:ss"), })); diff --git a/server/uptime-calculator.js b/server/uptime-calculator.js index 89e1f498e..7c931a143 100644 --- a/server/uptime-calculator.js +++ b/server/uptime-calculator.js @@ -4,6 +4,8 @@ const { LimitQueue } = require("./utils/limit-queue"); const { log } = require("../src/util"); const { R } = require("redbean-node"); +const FIRST_BEAT_COUNT = 1; + /** * Calculates the uptime of a monitor. */ @@ -141,6 +143,7 @@ class UptimeCalculator { avgNumeric: bean.numeric_value, minNumeric: bean.numeric_min, maxNumeric: bean.numeric_max, + sumNumeric: bean.numeric_value !== null && bean.numeric_value !== undefined ? bean.numeric_value * bean.up : 0, }; if (bean.extras != null) { @@ -170,6 +173,7 @@ class UptimeCalculator { avgNumeric: bean.numeric_value, minNumeric: bean.numeric_min, maxNumeric: bean.numeric_max, + sumNumeric: bean.numeric_value !== null && bean.numeric_value !== undefined ? bean.numeric_value * bean.up : 0, }; if (bean.extras != null) { @@ -198,6 +202,7 @@ class UptimeCalculator { avgNumeric: bean.numeric_value, minNumeric: bean.numeric_min, maxNumeric: bean.numeric_max, + sumNumeric: bean.numeric_value !== null && bean.numeric_value !== undefined ? bean.numeric_value * bean.up : 0, }; if (bean.extras != null) { @@ -212,32 +217,15 @@ class UptimeCalculator { } /** - * @param {number} status status - * @param {number} ping Ping - * @param {number|null} numericValue Numeric value (e.g., from JSON query or SNMP) - * @param {dayjs.Dayjs} date Date (Only for migration) - * @returns {Promise} date - * @throws {Error} Invalid status + * Update status counts for minutely, hourly, and daily data + * @private + * @param {object} minutelyData Minutely data object + * @param {object} hourlyData Hourly data object + * @param {object} dailyData Daily data object + * @param {number} status Status value + * @param {number} flatStatus Flattened status value */ - async update(status, ping = 0, numericValue = null, date) { - if (!date) { - date = this.getCurrentDate(); - } - - let flatStatus = this.flatStatus(status); - - if (flatStatus === DOWN && ping > 0) { - log.debug("uptime_calc", "The ping is not effective when the status is DOWN"); - } - - let divisionKey = this.getMinutelyKey(date); - let hourlyKey = this.getHourlyKey(date); - let dailyKey = this.getDailyKey(date); - - let minutelyData = this.minutelyUptimeDataList[divisionKey]; - let hourlyData = this.hourlyUptimeDataList[hourlyKey]; - let dailyData = this.dailyUptimeDataList[dailyKey]; - + updateStatusCounts(minutelyData, hourlyData, dailyData, status, flatStatus) { if (status === MAINTENANCE) { minutelyData.maintenance = minutelyData.maintenance ? minutelyData.maintenance + 1 : 1; hourlyData.maintenance = hourlyData.maintenance ? hourlyData.maintenance + 1 : 1; @@ -246,129 +234,147 @@ class UptimeCalculator { minutelyData.up += 1; hourlyData.up += 1; dailyData.up += 1; - - // Only UP status can update the ping - if (!isNaN(ping)) { - // Add avg ping - // The first beat of the minute, the ping is the current ping - if (minutelyData.up === 1) { - minutelyData.avgPing = ping; - minutelyData.minPing = ping; - minutelyData.maxPing = ping; - } else { - minutelyData.avgPing = (minutelyData.avgPing * (minutelyData.up - 1) + ping) / minutelyData.up; - minutelyData.minPing = Math.min(minutelyData.minPing, ping); - minutelyData.maxPing = Math.max(minutelyData.maxPing, ping); - } - - // Add avg ping - // The first beat of the hour, the ping is the current ping - if (hourlyData.up === 1) { - hourlyData.avgPing = ping; - hourlyData.minPing = ping; - hourlyData.maxPing = ping; - } else { - hourlyData.avgPing = (hourlyData.avgPing * (hourlyData.up - 1) + ping) / hourlyData.up; - hourlyData.minPing = Math.min(hourlyData.minPing, ping); - hourlyData.maxPing = Math.max(hourlyData.maxPing, ping); - } - - // Add avg ping (daily) - // The first beat of the day, the ping is the current ping - if (dailyData.up === 1) { - dailyData.avgPing = ping; - dailyData.minPing = ping; - dailyData.maxPing = ping; - } else { - dailyData.avgPing = (dailyData.avgPing * (dailyData.up - 1) + ping) / dailyData.up; - dailyData.minPing = Math.min(dailyData.minPing, ping); - dailyData.maxPing = Math.max(dailyData.maxPing, ping); - } - } - - // Only UP status can update the numeric value - if (numericValue !== null && !isNaN(numericValue)) { - const numValue = parseFloat(numericValue); - // Add avg numeric - // The first beat of the minute, the numeric is the current numeric - if (minutelyData.up === 1) { - minutelyData.avgNumeric = numValue; - minutelyData.minNumeric = numValue; - minutelyData.maxNumeric = numValue; - } else { - if (minutelyData.avgNumeric === null) { - minutelyData.avgNumeric = numValue; - minutelyData.minNumeric = numValue; - minutelyData.maxNumeric = numValue; - } else { - minutelyData.avgNumeric = - (minutelyData.avgNumeric * (minutelyData.up - 1) + numValue) / minutelyData.up; - minutelyData.minNumeric = Math.min(minutelyData.minNumeric, numValue); - minutelyData.maxNumeric = Math.max(minutelyData.maxNumeric, numValue); - } - } - - // Add avg numeric - // The first beat of the hour, the numeric is the current numeric - if (hourlyData.up === 1) { - hourlyData.avgNumeric = numValue; - hourlyData.minNumeric = numValue; - hourlyData.maxNumeric = numValue; - } else { - if (hourlyData.avgNumeric === null) { - hourlyData.avgNumeric = numValue; - hourlyData.minNumeric = numValue; - hourlyData.maxNumeric = numValue; - } else { - hourlyData.avgNumeric = - (hourlyData.avgNumeric * (hourlyData.up - 1) + numValue) / hourlyData.up; - hourlyData.minNumeric = Math.min(hourlyData.minNumeric, numValue); - hourlyData.maxNumeric = Math.max(hourlyData.maxNumeric, numValue); - } - } - - // Add avg numeric (daily) - // The first beat of the day, the numeric is the current numeric - if (dailyData.up === 1) { - dailyData.avgNumeric = numValue; - dailyData.minNumeric = numValue; - dailyData.maxNumeric = numValue; - } else { - if (dailyData.avgNumeric === null) { - dailyData.avgNumeric = numValue; - dailyData.minNumeric = numValue; - dailyData.maxNumeric = numValue; - } else { - dailyData.avgNumeric = (dailyData.avgNumeric * (dailyData.up - 1) + numValue) / dailyData.up; - dailyData.minNumeric = Math.min(dailyData.minNumeric, numValue); - dailyData.maxNumeric = Math.max(dailyData.maxNumeric, numValue); - } - } - } } else if (flatStatus === DOWN) { minutelyData.down += 1; hourlyData.down += 1; dailyData.down += 1; } + } - if (minutelyData !== this.lastUptimeData) { - this.lastUptimeData = minutelyData; + /** + * Update ping statistics for minutely, hourly, and daily data + * @private + * @param {object} minutelyData Minutely data object + * @param {object} hourlyData Hourly data object + * @param {object} dailyData Daily data object + * @param {number} ping Ping value + */ + updatePing(minutelyData, hourlyData, dailyData, ping) { + if (isNaN(ping)) { + return; } - if (hourlyData !== this.lastHourlyUptimeData) { - this.lastHourlyUptimeData = hourlyData; + // Update minutely ping + if (minutelyData.up === FIRST_BEAT_COUNT) { + minutelyData.avgPing = ping; + minutelyData.minPing = ping; + minutelyData.maxPing = ping; + } else { + minutelyData.avgPing = (minutelyData.avgPing * (minutelyData.up - 1) + ping) / minutelyData.up; + minutelyData.minPing = Math.min(minutelyData.minPing, ping); + minutelyData.maxPing = Math.max(minutelyData.maxPing, ping); } - if (dailyData !== this.lastDailyUptimeData) { - this.lastDailyUptimeData = dailyData; + // Update hourly ping + if (hourlyData.up === FIRST_BEAT_COUNT) { + hourlyData.avgPing = ping; + hourlyData.minPing = ping; + hourlyData.maxPing = ping; + } else { + hourlyData.avgPing = (hourlyData.avgPing * (hourlyData.up - 1) + ping) / hourlyData.up; + hourlyData.minPing = Math.min(hourlyData.minPing, ping); + hourlyData.maxPing = Math.max(hourlyData.maxPing, ping); } - // Don't store data in test mode - if (process.env.TEST_BACKEND) { - log.debug("uptime_calc", "Skip storing data in test mode"); - return date; + // Update daily ping + if (dailyData.up === FIRST_BEAT_COUNT) { + dailyData.avgPing = ping; + dailyData.minPing = ping; + dailyData.maxPing = ping; + } else { + dailyData.avgPing = (dailyData.avgPing * (dailyData.up - 1) + ping) / dailyData.up; + dailyData.minPing = Math.min(dailyData.minPing, ping); + dailyData.maxPing = Math.max(dailyData.maxPing, ping); + } + } + + /** + * Update numeric value statistics for minutely, hourly, and daily data + * @private + * @param {object} minutelyData Minutely data object + * @param {object} hourlyData Hourly data object + * @param {object} dailyData Daily data object + * @param {number} numericValue Numeric value + */ + updateNumeric(minutelyData, hourlyData, dailyData, numericValue) { + if (numericValue === null || isNaN(numericValue)) { + return; } + // Update minutely numeric + if (minutelyData.up === FIRST_BEAT_COUNT) { + minutelyData.sumNumeric = numericValue; + minutelyData.avgNumeric = numericValue; + minutelyData.minNumeric = numericValue; + minutelyData.maxNumeric = numericValue; + } else { + if (minutelyData.avgNumeric === null) { + minutelyData.sumNumeric = numericValue; + minutelyData.avgNumeric = numericValue; + minutelyData.minNumeric = numericValue; + minutelyData.maxNumeric = numericValue; + } else { + minutelyData.sumNumeric += numericValue; + minutelyData.avgNumeric = minutelyData.sumNumeric / minutelyData.up; + minutelyData.minNumeric = Math.min(minutelyData.minNumeric, numericValue); + minutelyData.maxNumeric = Math.max(minutelyData.maxNumeric, numericValue); + } + } + + // Update hourly numeric + if (hourlyData.up === FIRST_BEAT_COUNT) { + hourlyData.sumNumeric = numericValue; + hourlyData.avgNumeric = numericValue; + hourlyData.minNumeric = numericValue; + hourlyData.maxNumeric = numericValue; + } else { + if (hourlyData.avgNumeric === null) { + hourlyData.sumNumeric = numericValue; + hourlyData.avgNumeric = numericValue; + hourlyData.minNumeric = numericValue; + hourlyData.maxNumeric = numericValue; + } else { + hourlyData.sumNumeric += numericValue; + hourlyData.avgNumeric = hourlyData.sumNumeric / hourlyData.up; + hourlyData.minNumeric = Math.min(hourlyData.minNumeric, numericValue); + hourlyData.maxNumeric = Math.max(hourlyData.maxNumeric, numericValue); + } + } + + // Update daily numeric + if (dailyData.up === FIRST_BEAT_COUNT) { + dailyData.sumNumeric = numericValue; + dailyData.avgNumeric = numericValue; + dailyData.minNumeric = numericValue; + dailyData.maxNumeric = numericValue; + } else { + if (dailyData.avgNumeric === null) { + dailyData.sumNumeric = numericValue; + dailyData.avgNumeric = numericValue; + dailyData.minNumeric = numericValue; + dailyData.maxNumeric = numericValue; + } else { + dailyData.sumNumeric += numericValue; + dailyData.avgNumeric = dailyData.sumNumeric / dailyData.up; + dailyData.minNumeric = Math.min(dailyData.minNumeric, numericValue); + dailyData.maxNumeric = Math.max(dailyData.maxNumeric, numericValue); + } + } + } + + /** + * Store statistics to database + * @private + * @param {object} minutelyData Minutely data object + * @param {object} hourlyData Hourly data object + * @param {object} dailyData Daily data object + * @param {number} divisionKey Minutely division key + * @param {number} hourlyKey Hourly key + * @param {number} dailyKey Daily key + * @param {dayjs.Dayjs} date Date + * @returns {Promise} + */ + async storeStatistics(minutelyData, hourlyData, dailyData, divisionKey, hourlyKey, dailyKey, date) { let dailyStatBean = await this.getDailyStatBean(dailyKey); dailyStatBean.up = dailyData.up; dailyStatBean.down = dailyData.down; @@ -389,6 +395,7 @@ class UptimeCalculator { avgNumeric: _avgNumeric, minNumeric: _minNumeric, maxNumeric: _maxNumeric, + sumNumeric: _sumNumeric, timestamp: _timestamp, ...extras } = dailyData; @@ -424,6 +431,7 @@ class UptimeCalculator { avgNumeric: _avgNumeric, minNumeric: _minNumeric, maxNumeric: _maxNumeric, + sumNumeric: _sumNumeric, timestamp: _timestamp, ...extras } = hourlyData; @@ -458,6 +466,7 @@ class UptimeCalculator { avgNumeric: _avgNumeric, minNumeric: _minNumeric, maxNumeric: _maxNumeric, + sumNumeric: _sumNumeric, timestamp: _timestamp, ...extras } = minutelyData; @@ -484,6 +493,64 @@ class UptimeCalculator { this.getHourlyKey(currentDate.subtract(this.statHourlyKeepDay, "day")), ]); } + } + + /** + * @param {number} status status + * @param {number} ping Ping + * @param {number|null} numericValue Numeric value (e.g., from JSON query or SNMP) + * @param {dayjs.Dayjs} date Date (Only for migration) + * @returns {Promise} date + * @throws {Error} Invalid status + */ + async update(status, ping = 0, numericValue = null, date = null) { + if (!date) { + date = this.getCurrentDate(); + } + + let flatStatus = this.flatStatus(status); + + if (flatStatus === DOWN && ping > 0) { + log.debug("uptime_calc", "The ping is not effective when the status is DOWN"); + } + + let divisionKey = this.getMinutelyKey(date); + let hourlyKey = this.getHourlyKey(date); + let dailyKey = this.getDailyKey(date); + + let minutelyData = this.minutelyUptimeDataList[divisionKey]; + let hourlyData = this.hourlyUptimeDataList[hourlyKey]; + let dailyData = this.dailyUptimeDataList[dailyKey]; + + // Update status counts + this.updateStatusCounts(minutelyData, hourlyData, dailyData, status, flatStatus); + + // Only UP status can update ping and numeric values + if (flatStatus === UP) { + this.updatePing(minutelyData, hourlyData, dailyData, ping); + this.updateNumeric(minutelyData, hourlyData, dailyData, numericValue); + } + + // Update last data references + if (minutelyData !== this.lastUptimeData) { + this.lastUptimeData = minutelyData; + } + + if (hourlyData !== this.lastHourlyUptimeData) { + this.lastHourlyUptimeData = hourlyData; + } + + if (dailyData !== this.lastDailyUptimeData) { + this.lastDailyUptimeData = dailyData; + } + + // Don't store data in test mode + if (process.env.TEST_BACKEND) { + log.debug("uptime_calc", "Skip storing data in test mode"); + return date; + } + + await this.storeStatistics(minutelyData, hourlyData, dailyData, divisionKey, hourlyKey, dailyKey, date); return date; } @@ -576,6 +643,7 @@ class UptimeCalculator { avgNumeric: null, minNumeric: null, maxNumeric: null, + sumNumeric: 0, }); } @@ -604,6 +672,7 @@ class UptimeCalculator { avgNumeric: null, minNumeric: null, maxNumeric: null, + sumNumeric: 0, }); } @@ -631,6 +700,7 @@ class UptimeCalculator { avgNumeric: null, minNumeric: null, maxNumeric: null, + sumNumeric: 0, }); }