From 27633d51b651e6cdb125931305125bbe33df3433 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Thu, 16 Feb 2023 01:22:30 +0100 Subject: [PATCH] Fix log filtering in Web UI Remove broken alternative logFilter implementation in web_ui/src/websocket in favour of having one proper function in lib/logging --- packages/lib/logging.js | 54 ++++++++++++++++++++++ packages/lib/logging_utils.js | 57 +----------------------- packages/master/src/ControlConnection.js | 4 +- packages/web_ui/src/util/websocket.jsx | 20 +-------- 4 files changed, 60 insertions(+), 75 deletions(-) diff --git a/packages/lib/logging.js b/packages/lib/logging.js index 9dd1068e..4c1ecff2 100644 --- a/packages/lib/logging.js +++ b/packages/lib/logging.js @@ -82,6 +82,59 @@ class WebConsoleFormat { } } +/** + * Filter object for logs + * @typedef {Object} module:lib/logging~LogFilter + * @property {string} [max_level] - + * Maximum log level to include. Higher levels are more verbose. + * @property {boolean} [all] - + * Include log entries from master, all slaves and all instances. + * @property {boolean} [master] - + * Include log entries from the master server. + * @property {Array} [slave_ids] - + * Include log entries for the given slaves and instances of those + * slaves by id. + * @property {Array} [instance_ids] - + * Include log entries for the given instances by id. + */ + +/** + * Create log filter by level and source. + * + * @param {module:lib/logging~LogFilter} filter - + * Filter to filter log entries by. + * @returns {function(object): boolean} + * filter returning true for log entries that match it. + * @static + */ +function logFilter({ all, master, slave_ids, instance_ids, max_level }) { + return info => { + // Note: reversed to filter out undefined levels + if (max_level && !(levels[info.level] <= levels[max_level])) { + return false; + } + + if (all) { + return true; + } + if (master && info.slave_id === undefined) { + return true; + } + if ( + slave_ids + && info.slave_id !== undefined + && info.instance_id === undefined + && slave_ids.includes(info.slave_id) + ) { + return true; + } + if (instance_ids && info.instance_id !== undefined && instance_ids.includes(info.instance_id)) { + return true; + } + return false; + }; +} + const logger = winston.createLogger({ level: "verbose", levels, @@ -93,5 +146,6 @@ module.exports = { WebConsoleFormat, colors, levels, + logFilter, logger, }; diff --git a/packages/lib/logging_utils.js b/packages/lib/logging_utils.js index d23a80cb..0451f56e 100644 --- a/packages/lib/logging_utils.js +++ b/packages/lib/logging_utils.js @@ -18,7 +18,7 @@ const chalk = require("chalk"); const libLink = require("./link"); const libErrors = require("./errors"); const libFileOps = require("./file_ops"); -const { levels, logger } = require("./logging"); +const { levels, logFilter, logger } = require("./logging"); const libStream = require("./stream"); const finished = util.promisify(stream.finished); @@ -140,21 +140,6 @@ class LinkTransport extends Transport { } const logFileGlob = /^[a-z]+-(\d{4}-\d{2}-\d{2})\.log$/; -/** - * Filter object for logs - * @typedef {Object} module:lib/logging_utils~LogFilter - * @property {string} [max_level] - - * Maximum log level to include. Higher levels are more verbose. - * @property {boolean} [all] - - * Include log entries from master, all slaves and all instances. - * @property {boolean} [master] - - * Include log entries from the master server. - * @property {Array} [slave_ids] - - * Include log entries for the given slaves and instances of those - * slaves by id. - * @property {Array} [instance_ids] - - * Include log entries for the given instances by id. - */ /** * Keeps an index over a log directory to speed up queries to it @@ -289,7 +274,7 @@ class LogIndex { * * @param {string} file - * Name of file in the log directory to check for. - * @param {module:lib/logging_utils~LogFilter} filter - + * @param {module:lib/logging~LogFilter} filter - * Filter to check index if file contains entries for. * @returns {boolean} * true if the file may contain entries included by the filter. @@ -326,43 +311,6 @@ class LogIndex { } } -/** - * Create log filter by level and source. - * - * @param {module:lib/logging_utils~LogFilter} filter - - * Filter to filter log entries by. - * @returns {function(object): boolean} - * filter returning true for log entries that match it. - * @static - */ -function logFilter({ all, master, slave_ids, instance_ids, max_level }) { - return info => { - // Note: reversed to filter out undefined levels - if (max_level && !(levels[info.level] <= levels[max_level])) { - return false; - } - - if (all) { - return true; - } - if (master && info.slave_id === undefined) { - return true; - } - if ( - slave_ids - && info.slave_id !== undefined - && info.instance_id === undefined - && slave_ids.includes(info.slave_id) - ) { - return true; - } - if (instance_ids && info.instance_id !== undefined && instance_ids.includes(info.instance_id)) { - return true; - } - return false; - }; -} - /** * Filter object for querying logs * @typedef {Object} module:lib/logging_utils~QueryLogFilter @@ -515,7 +463,6 @@ module.exports = { TerminalFormat, LinkTransport, LogIndex, - logFilter, queryLog, migrateLogs, diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index 43379633..f695156e 100644 --- a/packages/master/src/ControlConnection.js +++ b/packages/master/src/ControlConnection.js @@ -9,7 +9,7 @@ const libData = require("@clusterio/lib/data"); const libErrors = require("@clusterio/lib/errors"); const libHelpers = require("@clusterio/lib/helpers"); const libLink = require("@clusterio/lib/link"); -const { logger } = require("@clusterio/lib/logging"); +const { logFilter, logger } = require("@clusterio/lib/logging"); const libLoggingUtils = require("@clusterio/lib/logging_utils"); const libPlugin = require("@clusterio/lib/plugin"); const libPrometheus = require("@clusterio/lib/prometheus"); @@ -674,7 +674,7 @@ class ControlConnection extends BaseConnection { this.logTransport = new libLoggingUtils.LinkTransport({ link: this }); this._master.clusterLogger.add(this.logTransport); } - this.logTransport.filter = libLoggingUtils.logFilter(this.logSubscriptions); + this.logTransport.filter = logFilter(this.logSubscriptions); } else if (this.logTransport) { this._master.clusterLogger.remove(this.logTransport); diff --git a/packages/web_ui/src/util/websocket.jsx b/packages/web_ui/src/util/websocket.jsx index 35181681..78e72ec5 100644 --- a/packages/web_ui/src/util/websocket.jsx +++ b/packages/web_ui/src/util/websocket.jsx @@ -1,5 +1,5 @@ import { libData, libErrors, libLink, libLogging, libPlugin } from "@clusterio/lib"; -const { logger } = libLogging; +const { logFilter, logger } = libLogging; import packageJson from "../../package.json"; @@ -27,22 +27,6 @@ export class ControlConnector extends libLink.WebSocketClientConnector { } } -function logFilter({ all, master, slave_ids, instance_ids }, info) { - if (all) { - return true; - } - if (master && info.slave_id === undefined) { - return true; - } - if (info.slave_id !== undefined && slave_ids.includes(info.slave_id)) { - return true; - } - if (info.instance_id !== undefined && instance_ids.includes(info.instance_id)) { - return true; - } - return false; -} - /** * Handles running the control * @@ -504,7 +488,7 @@ export class Control extends libLink.Link { let info = message.data.info; for (let [filter, handlers] of this.logHandlers) { - if (logFilter(filter, info)) { + if (logFilter(filter)(info)) { for (let handler of handlers) { handler(info); }