From a89106f4a3058313ac97004d5f32f264f7b35d37 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 7 Dec 2020 17:12:29 +0100 Subject: [PATCH 1/4] Add changelog entry for logging system --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ded288a..4bf8200c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ Version 2.0.0 configuration remotely. - Packaged and uploaded to npm again, greatly simplifying distribution and installation of both Clusterio and plugins. +- Added centralized logging for the cluster. All logs from both Clusterio + and Factorio is stored in a shared log which can be inspected and queried. ### Features From 71164faa140a333a076af7b9cb442ec6fb3f31e1 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 7 Dec 2020 17:15:25 +0100 Subject: [PATCH 2/4] Fix docs and usage of Collector callback Fix callback typo in Collector constructor jsdocs, add notice that the collector is passed as the first argument, and use this first arugument in the built-in collector implementations. --- packages/lib/prometheus.js | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/lib/prometheus.js b/packages/lib/prometheus.js index bb163ea2..cc167625 100644 --- a/packages/lib/prometheus.js +++ b/packages/lib/prometheus.js @@ -337,9 +337,10 @@ class LabeledCollector extends Collector { * @param {boolean=} options.register - * If true registers this collector with the default registry. * Defaults to true. - * @param {function()=} options.callaback - + * @param {function()=} options.callback - * Possibly async function that is called before the metric is - * collected. + * collected. The collector being collected is passed as the + * argument. * @param { * function(module:lib/prometheus.LabeledCollector,string) * } childClass - @@ -479,9 +480,10 @@ class ValueCollector extends LabeledCollector { * @param {boolean=} options.register - * If true registers this collector with the default registry. * Defaults to true. - * @param {function()=} options.callaback - + * @param {function()=} options.callback - * Possibly async function that is called before the collector value - * is collected. + * is collected. The collector being collected is passed as the + * argument. * @param { * function( * new:module:lib/prometheus.ValueCollectorChild, @@ -666,9 +668,10 @@ class Counter extends ValueCollector { * @param {boolean=} options.register - * If true registers this collector with the default registry. * Defaults to true. - * @param {function()=} options.callaback - + * @param {function()=} options.callback - * Possibly async function that is called when the metric is - * collected. + * collected. The collector being collected is passed as the + * argument. */ constructor(name, help, options = {}) { super("counter", name, help, options, CounterChild); @@ -855,9 +858,10 @@ class Gauge extends ValueCollector { * @param {boolean=} options.register - * If true registers this collector with the default registry. * Defaults to true. - * @param {function()=} options.callaback - + * @param {function()=} options.callback - * Possibly async function that is called when the metric is - * collected. + * collected. The collector being collected is passed as the + * argument. */ constructor(name, help, options = {}) { super("gauge", name, help, options, GaugeChild); @@ -1102,9 +1106,10 @@ class Histogram extends LabeledCollector { * @param {boolean=} options.register - * If true registers this collector with the default registry. * Defaults to true. - * @param {function()=} options.callaback - + * @param {function()=} options.callback - * Possibly async function that is called when the metric is - * collected. + * collected. The collector being collected is passed as the + * argument. */ constructor(name, help, options = {}) { // These defaults are taken from the Python Prometheus client @@ -1525,9 +1530,9 @@ defaultCollectors.processCpuSecondsTotal = new Gauge( "process_cpu_seconds_total", "Total user and system CPU time spent in seconds.", { - callback: async function() { + callback: async function(collector) { let usage = process.cpuUsage(); - this.set((usage.user + usage.system) / 1e6); + collector.set((usage.user + usage.system) / 1e6); }, }, ); @@ -1536,8 +1541,8 @@ defaultCollectors.processResidentMemoryBytes = new Gauge( "process_resident_memory_bytes", "Resident memory size in bytes.", { - callback: function() { - this.set(process.memoryUsage().rss); + callback: function(collector) { + collector.set(process.memoryUsage().rss); }, }, ); @@ -1546,8 +1551,8 @@ defaultCollectors.processHeapBytes = new Gauge( "process_heap_bytes", "Process heap size in bytes.", { - callback: function() { - this.set(process.memoryUsage().heapUsed); + callback: function(collector) { + collector.set(process.memoryUsage().heapUsed); }, }, ); From 450e27e00138ac3e6acfd956362e4363b328cc2a Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 7 Dec 2020 17:22:17 +0100 Subject: [PATCH 3/4] Add metric mapping slave and instance id to name Add exported metrics that map instance id and slave id to their respective names. This allows getting the instance name of a metric by doing a left join using query of the form 'metric * on(instance_id) group_left(instance_name) clusterio_master_instance_mapping'. --- CHANGELOG.md | 2 ++ packages/master/master.js | 42 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bf8200c..c1308051 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,8 @@ Version 2.0.0 - Added stripping of long paths in the Factorio server log. - Added option to configure maximum number of commands to send in parallel. - Added option to auto start instances on slave startup. +- Added metric mapping of slave and instance ids to their names, allowing + them to be displayed by name in queries that join with the mapping. ### Changes diff --git a/packages/master/master.js b/packages/master/master.js index 59523acb..f511e05e 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -42,6 +42,10 @@ let debugEvents = new events.EventEmitter(); let loadedPlugins = {}; let devMiddleware; let clusterLogger; +let db = { + instances: new Map(), + slaves: new Map(), +}; // homebrew modules const generateSslCert = require("./src/generate_ssl_cert"); @@ -106,6 +110,41 @@ function serveWeb(route) { app.use(express.static(path.join(__dirname, "static"))); app.use(express.static("static")); // Used for data export files +const slaveMappingGauge = new libPrometheus.Gauge( + "clusterio_master_slave_mapping", + "Mapping of Slave ID to name", + { + labels: ["slave_id", "slave_name"], + callback: function() { + slaveMappingGauge.clear(); + for (let [id, slave] of db.slaves) { + slaveMappingGauge.labels({ + slave_id: String(id), + slave_name: slave.name, + }).set(1); + }; + }, + } +); + +const instanceMappingGauge = new libPrometheus.Gauge( + "clusterio_master_instance_mapping", + "Mapping of Instance ID to name and slave", + { + labels: ["instance_id", "instance_name", "slave_id"], + callback: function() { + instanceMappingGauge.clear(); + for (let [id, instance] of db.instances) { + instanceMappingGauge.labels({ + instance_id: String(id), + instance_name: String(instance.config.get("instance.name")), + slave_id: String(instance.config.get("instance.assigned_slave")), + }).set(1); + } + }, + } +); + const endpointHitCounter = new libPrometheus.Counter( "clusterio_master_http_endpoint_hits_total", "How many requests a particular HTTP endpoint has gotten", @@ -280,9 +319,6 @@ const masterConnectedClientsCount = new libPrometheus.Gauge( }, ); -// set up database -const db = {}; - /** * Load Map from JSON file in the database directory. * From 2e5333af29384a0572d636e6f0f2be15065dc517 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 7 Dec 2020 18:49:25 +0100 Subject: [PATCH 4/4] Fix banlist sync being broken Fix the logic for determining which entries in the banlist changed being incorrect and causing the banlist sync to be broken. Add unit tests for the syncUserListsEventHandler to ensure it functions as exxpected. --- packages/slave/slave.js | 11 ++-- plugins/statistics_exporter/instance.js | 15 ++++- test/slave.js | 81 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 9 deletions(-) diff --git a/packages/slave/slave.js b/packages/slave/slave.js index 47cfb3bb..b717228e 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1059,8 +1059,8 @@ class Slave extends libLink.Link { let updateList = (list, updatedList, prop, event) => { let added = new Set(updatedList); let removed = new Set(list); - list.forEach(Set.prototype.delete.bind(added)); - updatedList.forEach(Set.prototype.delete.bind(removed)); + list.forEach(el => added.delete(el)); + updatedList.forEach(el => removed.delete(el)); for (let name of added) { list.add(name); @@ -1080,15 +1080,14 @@ class Slave extends libLink.Link { updateList(this.adminlist, message.data.adminlist, "admin", libLink.messages.adminlistUpdate); updateList(this.whitelist, message.data.whitelist, "whitelisted", libLink.messages.whitelistUpdate); - let updatedBanlist = new Map(message.data.banlist); - let addedOrChanged = new Map(updatedBanlist); + let addedOrChanged = new Map(message.data.banlist); let removed = new Set(this.banlist.keys()); - removed.forEach((name, reason) => { + addedOrChanged.forEach((_, name) => removed.delete(name)); + this.banlist.forEach((reason, name) => { if (addedOrChanged.get(name) === reason) { addedOrChanged.delete(name); } }); - updatedBanlist.forEach(Set.prototype.delete.bind(removed)); for (let [name, reason] of addedOrChanged) { this.banlist.set(name, reason); diff --git a/plugins/statistics_exporter/instance.js b/plugins/statistics_exporter/instance.js index 002e51f6..001d96b6 100644 --- a/plugins/statistics_exporter/instance.js +++ b/plugins/statistics_exporter/instance.js @@ -21,6 +21,17 @@ const instanceGameFlowStatistics = new Gauge( { labels: ["instance_id", "statistic", "direction", "name"] }, ); +function setForceFlowStatistic(instanceId, forceName, statisticName, direction, item, value) { + instanceForceFlowStatistics.labels(String(instanceId), forceName, statisticName, direction, item).set(value); + + // For item and fluid statistics it's useful to compare the input flow with the + // output flow, to simplify the comparison ensure both directions have a value. + if (["item_production_statistics", "fluid_production_statistic"].includes(statisticName)) { + let reverseDirection = direction === "input" ? "output" : "input"; + instanceForceFlowStatistics.labels(String(instanceId), forceName, statisticName, reverseDirection, item); + } +} + class InstancePlugin extends libPlugin.BaseInstancePlugin { async init() { @@ -46,9 +57,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { for (let [statisticName, statistic] of Object.entries(flowStatistics)) { for (let [direction, counts] of Object.entries(statistic)) { for (let [item, value] of Object.entries(counts)) { - instanceForceFlowStatistics.labels( - String(instanceId), forceName, statisticName, direction, item - ).set(value); + setForceFlowStatistic(instanceId, forceName, statisticName, direction, item, value); } } } diff --git a/test/slave.js b/test/slave.js index 7e0801e1..f436b312 100644 --- a/test/slave.js +++ b/test/slave.js @@ -162,4 +162,85 @@ describe("Slave testing", function() { ])); }); }); + + describe("class Slave", function() { + describe(".syncUserListsEventHandler()", function() { + let mockSlave; + beforeEach(function() { + mockSlave = { + adminlist: new Set(), + whitelist: new Set(), + banlist: new Map(), + broadcasts: [], + broadcastEventToInstance(message, event) { + this.broadcasts.push(message["data"]); + }, + syncUserListsEventHandler: slave._Slave.prototype.syncUserListsEventHandler, + syncLists(adminlist, banlist, whitelist) { + return this.syncUserListsEventHandler({ "data": { + "adminlist": adminlist, + "banlist": banlist, + "whitelist": whitelist, + }}); + }, + }; + }); + + it("should broadcast new entries to adminlist", async function() { + await mockSlave.syncLists(["admin1"], [], []); + await mockSlave.syncLists(["admin1", "admin2"], [], []); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "admin1", "admin": true }, + { "name": "admin2", "admin": true }, + ]); + }); + + it("should broadcast removals from adminlist", async function() { + mockSlave.adminlist.add("admin1").add("admin2"); + await mockSlave.syncLists(["admin1"], [], []); + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "admin2", "admin": false }, + ]); + }); + + it("should broadcast new entries to whitelist", async function() { + await mockSlave.syncLists([], [], ["player1"]); + await mockSlave.syncLists([], [], ["player1", "player2"]); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "player1", "whitelisted": true }, + { "name": "player2", "whitelisted": true }, + ]); + }); + + it("should broadcast removals from whitelist", async function() { + mockSlave.whitelist.add("player1").add("player2"); + await mockSlave.syncLists([], [], ["player1"]); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "player2", "whitelisted": false }, + ]); + }); + + it("should broadcast new entries to banlist", async function() { + await mockSlave.syncLists([], [["badie1", "greifing"]], []); + await mockSlave.syncLists([], [["badie1", "greifing"], ["badie2", "annoying"]], []); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "badie1", "banned": true, "reason": "greifing" }, + { "name": "badie2", "banned": true, "reason": "annoying" }, + ]); + }); + + it("should broadcast removals to banlist", async function() { + mockSlave.banlist.set("badie1", "greifing").set("badie2", "annoying"); + await mockSlave.syncLists([], [["badie1", "greifing"]], []); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "badie2", "banned": false, "reason": "" }, + ]); + }); + }); + }); });