From 6eef4bd300d24d2cdd2598e56826099348f0822f Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 16 Dec 2020 20:33:17 +0100 Subject: [PATCH] Make metrics gathering timeout configurable When reducing the collection interval in Prometheus it may be necessary to to reduce the metrics gathering timeout to ensure it's less than the interval. Add config options to adjust the timeouts used for metrics gathering so that these can be set accordingly. --- CHANGELOG.md | 1 + docs/configuration.md | 9 +++++++++ packages/lib/config/definitions.js | 7 +++++++ packages/master/master.js | 3 ++- plugins/statistics_exporter/README.md | 17 ++++++++++++++++- plugins/statistics_exporter/info.js | 13 +++++++++++++ plugins/statistics_exporter/instance.js | 7 ++++--- plugins/statistics_exporter/test/plugin.js | 4 +++- test/mock.js | 11 ++++++++--- 9 files changed, 63 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1308051..57d2cf45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Version 2.0.0 - 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. +- Added option to configure timeout for exported Prometheus metrics. ### Changes diff --git a/docs/configuration.md b/docs/configuration.md index 4951fe5d..74409f5c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -112,6 +112,15 @@ unavoidable. Defaults to 30. +### master.metrics_timeout + +Timeout in seconds before a call to gather metrics from a slave times +out. This should be less than both the timeout and collection interval +configured for the collection job in Prometheus. + +Defaults to 8. + + ### master.default_role_id Role to automatically grant to new users. If null no role is granted. diff --git a/packages/lib/config/definitions.js b/packages/lib/config/definitions.js index 2d342b9b..223f53f1 100644 --- a/packages/lib/config/definitions.js +++ b/packages/lib/config/definitions.js @@ -94,6 +94,13 @@ MasterGroup.define({ type: "number", initial_value: 30, }); +MasterGroup.define({ + name: "metrics_timeout", + title: "Metrics Timeout", + description: "Timeout in seconds for metrics gathering from slaves", + type: "number", + initial_value: 8, +}); MasterGroup.define({ name: "default_role_id", title: "Default role", diff --git a/packages/master/master.js b/packages/master/master.js index c2c2b0f7..14439ada 100755 --- a/packages/master/master.js +++ b/packages/master/master.js @@ -190,8 +190,9 @@ async function getMetrics(req, res, next) { } let requests = []; + let timeout = masterConfig.get("master.metrics_timeout") * 1000; for (let slaveConnection of slaveConnections.values()) { - requests.push(libHelpers.timeout(libLink.messages.getMetrics.send(slaveConnection), 8e3, null)); + requests.push(libHelpers.timeout(libLink.messages.getMetrics.send(slaveConnection), timeout, null)); } for await (let result of await libPrometheus.defaultRegistry.collect()) { diff --git a/plugins/statistics_exporter/README.md b/plugins/statistics_exporter/README.md index b22a9892..7dc08021 100644 --- a/plugins/statistics_exporter/README.md +++ b/plugins/statistics_exporter/README.md @@ -7,7 +7,7 @@ Clusterio. Installation -============ +------------ Run the following commands in the folder Clusterio is installed to: @@ -16,3 +16,18 @@ Run the following commands in the folder Clusterio is installed to: Substitute clusteriomaster with clusterioslave or clusterioctl if this a dedicate slave or ctl installation respectively. + + +Instance Configuration +---------------------- + +### statistics_exporter.command_timeout + +Timeout in seconds to wait for the statitics gathering command to the +Factorio server return the result. If the timeout is exceeded the +previous collected results are returned instead. Normally the command +returns immediatly with the data, but if the RCON interface is +overloaded then the command may take too long to timely answer the +metrics collection request. + +Defaults to `1`. diff --git a/plugins/statistics_exporter/info.js b/plugins/statistics_exporter/info.js index e79bbd89..c89f5c1e 100644 --- a/plugins/statistics_exporter/info.js +++ b/plugins/statistics_exporter/info.js @@ -1,8 +1,21 @@ "use strict"; +const libConfig = require("@clusterio/lib/config"); + +class InstanceConfigGroup extends libConfig.PluginConfigGroup {} +InstanceConfigGroup.groupName = "statistics_exporter"; +InstanceConfigGroup.define({ + name: "command_timeout", + title: "Command Timeout", + description: "Timeout in seconds of the metrics command before returning results from the previous invocation.", + type: "number", + initial_value: 1, +}); +InstanceConfigGroup.finalize(); module.exports = { name: "statistics_exporter", title: "Prometheus Statistics Export", description: "Exports statistics to Prometheus", instanceEntrypoint: "instance", + InstanceConfigGroup, }; diff --git a/plugins/statistics_exporter/instance.js b/plugins/statistics_exporter/instance.js index 001d96b6..0784f816 100644 --- a/plugins/statistics_exporter/instance.js +++ b/plugins/statistics_exporter/instance.js @@ -73,11 +73,12 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { } async onMetrics() { - // Wait up to 5 seconds for the metrics to be collected. It may + // Wait configured timeout for the metrics to be collected. It may // take a long time for the command to go through if the command - // stream is overloaded. Should this take more than 5 seconds the + // stream is overloaded. Should the timeout be exceeded the // previous values for the metrics will end up being sent to master. - await libHelpers.timeout(this.gatherMetrics(), 5e3); + let timeout = this.instance.config.get("statistics_exporter.command_timeout") * 1000; + await libHelpers.timeout(this.gatherMetrics(), timeout); } } diff --git a/plugins/statistics_exporter/test/plugin.js b/plugins/statistics_exporter/test/plugin.js index 52d18595..50a28c88 100644 --- a/plugins/statistics_exporter/test/plugin.js +++ b/plugins/statistics_exporter/test/plugin.js @@ -9,8 +9,10 @@ const info = require("../info"); describe("statistics_exporter plugin", function() { describe("class InstancePlugin", function() { let instancePlugin; + let mockInstance = new mock.MockInstance(); + mockInstance.mockConfigEntries.set("statistics_exporter.command_timeout", 1); before(async function() { - instancePlugin = new instance.InstancePlugin(info, new mock.MockInstance()); + instancePlugin = new instance.InstancePlugin(info, mockInstance); await instancePlugin.init(); }); diff --git a/test/mock.js b/test/mock.js index acdf3c96..89ada241 100644 --- a/test/mock.js +++ b/test/mock.js @@ -103,11 +103,16 @@ class MockInstance extends libLink.Link { this.logger = new MockLogger(); this.server = new MockServer(); this.name = "test"; + this.mockConfigEntries = new Map([ + ["instance.id", 7357], + ["factorio.enable_save_patching", true], + ]); this.config = { get: (name) => { - if (name === "instance.id") { return 7357; } - if (name === "factorio.enable_save_patching") { return true; } - throw Error("Not implemented"); + if (this.mockConfigEntries.has(name)) { + return this.mockConfigEntries.get(name); + } + throw Error(`mock for field ${name} is not implemented`); }, }; }