From 2edf64c1323599cb3d4ac1c2c98017b760944d92 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Tue, 29 Sep 2020 18:16:29 -0400 Subject: [PATCH] (core) remove metrics Summary: This removes some old metric code. There's also a user preference dialog that has a single option (whether to allow metrics) this is left in place with a dummy option. It could be ripped out as well, probably. Test Plan: existing tests pass Reviewers: dsagal Reviewed By: dsagal Differential Revision: https://phab.getgrist.com/D2622 --- app/common/GristServerAPI.ts | 15 -- app/common/MetricCollector.js | 101 ------------ app/common/UserConfig.ts | 1 - app/common/metricConfig.js | 252 ------------------------------ app/common/metricTools.js | 261 -------------------------------- app/common/urlUtils.ts | 7 - app/server/declarations.d.ts | 1 - app/server/devServerMain.ts | 1 - app/server/lib/DocManager.ts | 8 - app/server/lib/FlexServer.ts | 22 +-- app/server/lib/ServerMetrics.js | 205 ------------------------- app/server/mergedServerMain.ts | 5 - 12 files changed, 2 insertions(+), 877 deletions(-) delete mode 100644 app/common/MetricCollector.js delete mode 100644 app/common/metricConfig.js delete mode 100644 app/common/metricTools.js delete mode 100644 app/server/lib/ServerMetrics.js diff --git a/app/common/GristServerAPI.ts b/app/common/GristServerAPI.ts index 42ab05ac..dcd5b718 100644 --- a/app/common/GristServerAPI.ts +++ b/app/common/GristServerAPI.ts @@ -8,26 +8,11 @@ export interface GristServerAPI extends DocListAPI, LoginSessionAPI, BasketClientAPI, - ServerMetricsAPI, UserAPI, SharingAPI, MiscAPI {} -interface ServerMetricsAPI { - /** - * Registers the list of client metric names. The calls to pushClientMetrics() send metric - * values as an array parallel to this list of names. - */ - registerClientMetrics(clientMetricsList: string[]): Promise; - - /** - * Sends bucketed client metric data to the server. The .values arrays contain one value for - * each of the registered metric names, as a parallel array. - */ - pushClientMetrics(clientBuckets: Array<{startTime: number, values: number[]}>): Promise; -} - interface UserAPI { /** * Gets the Grist configuration from the server. diff --git a/app/common/MetricCollector.js b/app/common/MetricCollector.js deleted file mode 100644 index 64931dd4..00000000 --- a/app/common/MetricCollector.js +++ /dev/null @@ -1,101 +0,0 @@ -const _ = require('underscore'); -const metricConfig = require('./metricConfig'); -const metricTools = require('./metricTools'); -const gutil = require('app/common/gutil'); - -/** - * Base class for metrics collection used by both the server metrics collector, ServerMetrics.js, - * and the client metrics collector, ClientMetrics.js. Should not be instantiated. - * Establishes interval attempts to push metrics to the server on creation. - */ -function MetricsCollector() { - this.startTime = metricTools.getBucketStartTime(Date.now()); - this.readyToExport = []; - // used (as a protected member) by the derived ServerMetrics class. - this._collect = setTimeout(() => this.scheduleBucketPreparation(), metricTools.getDeltaMs(Date.now())); -} - -// Should return a map from metric names (as entered in metricConfig.js) to their metricTools. -MetricsCollector.prototype.getMetrics = function() { - throw new Error("Not implemented"); -}; - -// Should return a promise that is resolved when the metrics have been pushed. -MetricsCollector.prototype.pushMetrics = function() { - throw new Error("Not implemented"); -}; - -// Should return a bucket of metric data, formatted for either the client or server. -MetricsCollector.prototype.createBucket = function(bucketStart) { - throw new Error("Not implemented"); -}; - -// Takes a list of metrics specifications and creates an object mapping metric names to -// a new instance of the metric gathering tool matching that metric's type. -MetricsCollector.prototype.initMetricTools = function(metricsList) { - var metrics = {}; - metricsList.forEach(metricInfo => { - metrics[metricInfo.name] = new metricTools[metricInfo.type](metricInfo.name); - }); - return metrics; -}; - -// Called each push interval. -MetricsCollector.prototype.attemptPush = function() { - this.pushMetrics(this.readyToExport); - this.readyToExport = []; -}; - -// Pushes bucket to the end of the readyToExport queue. Should be called sequentially, since it -// handles deletion of buckets older than the export memory limit. -MetricsCollector.prototype.queueBucket = function(bucket) { - // If readyToExport is at maximum length, delete the oldest element - this.readyToExport.push(bucket); - var length = this.readyToExport.length; - if (length > metricConfig.MAX_PENDING_BUCKETS) { - this.readyToExport.splice(0, length - metricConfig.MAX_PENDING_BUCKETS); - } -}; - -MetricsCollector.prototype.scheduleBucketPreparation = function() { - this.prepareCompletedBuckets(Date.now()); - this._collect = setTimeout(() => this.scheduleBucketPreparation(), metricTools.getDeltaMs(Date.now())); -}; - -/** - * Checks if each bucket since the last update is completed and for each one adds all data and - * pushes it to the export ready array. - */ -MetricsCollector.prototype.prepareCompletedBuckets = function(now) { - var bucketStart = metricTools.getBucketStartTime(now); - while (bucketStart > this.startTime) { - this.queueBucket(this.createBucket(this.startTime)); - this.startTime += metricConfig.BUCKET_SIZE; - } -}; - -/** - * Collects primitive metrics tools into a list. - */ -MetricsCollector.prototype.collectPrimitiveMetrics = function() { - var metricTools = []; - _.forEach(this.getMetrics(), metricTool => { - gutil.arrayExtend(metricTools, metricTool.getPrimitiveMetrics()); - }); - return metricTools; -}; - -/** - * Loops through metric tools for a chosen bucket and performs the provided callback on each. - * Resets each tool after the callback is performed. - * @param {Number} bucketStart - The desired bucket's start time in milliseconds - * @param {Function} callback - The callback to perform on each metric tool. - */ -MetricsCollector.prototype.forEachBucketMetric = function(bucketEnd, callback) { - this.collectPrimitiveMetrics().forEach(tool => { - callback(tool); - tool.reset(bucketEnd); - }); -}; - -module.exports = MetricsCollector; diff --git a/app/common/UserConfig.ts b/app/common/UserConfig.ts index c53d7fb7..dc1cc76b 100644 --- a/app/common/UserConfig.ts +++ b/app/common/UserConfig.ts @@ -2,7 +2,6 @@ * Interface for the user's config found in config.json. */ export interface UserConfig { - enableMetrics?: boolean; docListSortBy?: string; docListSortDir?: number; features?: ISupportedFeatures; diff --git a/app/common/metricConfig.js b/app/common/metricConfig.js deleted file mode 100644 index 417a40c1..00000000 --- a/app/common/metricConfig.js +++ /dev/null @@ -1,252 +0,0 @@ -/** - * File for configuring the metric collection bucket duration, data push intervals between client, server, - * and Grist Metrics EC2 instance, as well as individual metrics collected in the client and server. - */ - -// Time interval settings (ms) -exports.BUCKET_SIZE = 60 * 1000; - -exports.CLIENT_PUSH_INTERVAL = 120 * 1000; -exports.SERVER_PUSH_INTERVAL = 120 * 1000; -exports.MAX_PENDING_BUCKETS = 40; -exports.CONN_RETRY = 20 * 1000; - -// Metrics use the general form: -// . -// With prefixes, measurement type, and clientId/serverId added automatically on send. - -// 'type' is the measurement tool type, with options 'Switch', 'Counter', 'Gauge', 'Timer', and -// 'ExecutionTimer'. (See metricTools.js for details) -// Suffixes are added to the metric names depending on their measurement tool. -// 'Switch' => '.instances' -// 'Gauge' => '.total' -// 'Counter' => '.count' -// 'Timer' => '.time' -// 'ExecutionTimer' => '.execution_time', '.count' (Execution timer automatically records a count) - -exports.clientMetrics = [ - // General - { - name: 'sidepane.opens', - type: 'Counter', - desc: 'Number of times the side pane is opened' - }, - { - name: 'app.client_active_span', - type: 'Timer', - desc: 'Total client time spent using grist' - }, - { - name: 'app.connected_to_server_span', - type: 'Timer', - desc: 'Total time spent connected to the server' - }, - { - name: 'app.disconnected_from_server_span', - type: 'Timer', - desc: 'Total time spent disconnected from the server' - }, - // Docs - { - name: 'docs.num_open_6+_tables', - type: 'SamplingGauge', - desc: 'Number of open docs with more than 5 tables' - }, - { - name: 'docs.num_open_0-5_tables', - type: 'SamplingGauge', - desc: 'Number of open docs with 0-5 tables' - }, - // Tables - { - name: 'tables.num_tables', - type: 'SamplingGauge', - desc: 'Number of open tables' - }, - { - name: 'tables.num_summary_tables', - type: 'SamplingGauge', - desc: 'Number of open sections in the current view' - }, - // Views - { - name: 'views.code_view_open_span', - type: 'Timer', - desc: 'Time spent with code viewer open' - }, - // Sections - { - name: 'sections.grid_open_span', - type: 'Timer', - desc: 'Time spent with gridview open' - }, - { - name: 'sections.detail_open_span', - type: 'Timer', - desc: 'Time spent with gridview open' - }, - { - name: 'sections.num_grid_sections', - type: 'SamplingGauge', - desc: 'Number of open sections in the current view' - }, - { - name: 'sections.num_detail_sections', - type: 'SamplingGauge', - desc: 'Number of open sections in the current view' - }, - { - name: 'sections.num_chart_sections', - type: 'SamplingGauge', - desc: 'Number of open sections in the current view' - }, - { - name: 'sections.multiple_open_span', - type: 'Timer', - desc: 'Time spent with multiple sections open' - }, - // Performance - { - name: 'performance.server_action', - type: 'ExecutionTimer', - desc: 'Time for a server action to complete' - }, - { - name: 'performance.doc_load', - type: 'ExecutionTimer', - desc: 'Time to load a document' - }, - // Columns - { - name: 'cols.num_formula_cols', - type: 'SamplingGauge', - desc: 'Number of formula columns in open documents' - }, - { - name: 'cols.num_text_cols', - type: 'SamplingGauge', - desc: 'Number of text columns in open documents' - }, - { - name: 'cols.num_int_cols', - type: 'SamplingGauge', - desc: 'Number of integer columns in open documents' - }, - { - name: 'cols.num_numeric_cols', - type: 'SamplingGauge', - desc: 'Number of numeric columns in open documents' - }, - { - name: 'cols.num_date_cols', - type: 'SamplingGauge', - desc: 'Number of date columns in open documents' - }, - { - name: 'cols.num_datetime_cols', - type: 'SamplingGauge', - desc: 'Number of datetime columns in open documents' - }, - { - name: 'cols.num_ref_cols', - type: 'SamplingGauge', - desc: 'Number of reference columns in open documents' - }, - { - name: 'cols.num_attachments_cols', - type: 'SamplingGauge', - desc: 'Number of attachments columns in open documents' - }, - { - name: 'performance.front_end_errors', - type: 'Counter', - desc: 'Number of frontend errors' - } - // TODO: Implement the following: - // { - // name: 'grist-rt.performance.view_swap', - // type: 'ExecutionTimer', - // desc: 'Time to swap views' - // } -]; - -exports.serverMetrics = [ - // General - { - name: 'app.server_active', - type: 'Switch', - desc: 'Number of users currently using grist' - }, - { - name: 'app.server_active_span', - type: 'Timer', - desc: 'Total server time spent using grist' - }, - { - name: 'app.have_doc_open', - type: 'Switch', - desc: 'Number of users with at least one doc open' - }, - { - name: 'app.doc_open_span', - type: 'Timer', - desc: 'Total time spent with at least one doc open' - }, - // Docs - { - name: 'docs.num_open', - type: 'Gauge', - desc: 'Number of open docs' - }, - { - name: 'performance.node_memory_usage', - type: 'SamplingGauge', - desc: 'Memory utilization in bytes of the node process' - } - // TODO: Implement the following: - // { - // name: 'grist-rt.docs.total_size_open', - // type: 'Gauge', - // desc: 'Cumulative size of open docs' - // } - // { - // name: 'grist-rt.performance.open_standalone_app', - // type: 'ExecutionTimer', - // desc: 'Time to start standalone app' - // } - // { - // name: 'grist-rt.performance.sandbox_recalculation', - // type: 'ExecutionTimer', - // desc: 'Time for sandbox recalculation to occur' - // } - // { - // name: 'grist-rt.performance.open_standalone_app', - // type: 'ExecutionTimer', - // desc: 'Time to start standalone app' - // } - // { - // name: 'grist-rt.performance.node_cpu_usage', - // type: 'SamplingGauge', - // desc: 'Amount of time node was using the cpu in the interval' - // } - // { - // name: 'grist-rt.performance.sandbox_cpu_usage', - // type: 'SamplingGauge', - // desc: 'Amount of time the sandbox was using the cpu in the interval' - // } - // { - // name: 'grist-rt.performance.chrome_cpu_usage', - // type: 'SamplingGauge', - // desc: 'Amount of time chrome was using the cpu in the interval' - // } - // { - // name: 'grist-rt.performance.sandbox_memory_usage', - // type: 'SamplingGauge', - // desc: 'Memory utilization in bytes of the sandbox process' - // } - // { - // name: 'grist-rt.performance.chrome_memory_usage', - // type: 'SamplingGauge', - // desc: 'Memory utilization in bytes of the chrome process' - // } -]; diff --git a/app/common/metricTools.js b/app/common/metricTools.js deleted file mode 100644 index 146355c4..00000000 --- a/app/common/metricTools.js +++ /dev/null @@ -1,261 +0,0 @@ -const _ = require('underscore'); -const gutil = require('./gutil'); -const metricConfig = require('./metricConfig'); - -// TODO: Create a metric test class and write tests for each metric tool. - -/** - * Base class for tools to gather metrics. Should not be instantiated. - */ -function MetricTool(name) { - this.name = name; -} - -// Should be implemented by extending classes -MetricTool.prototype._getSuffix = function() { - throw new Error("Not implemented"); -}; - -// Should be overridden by extending classes depending on desired reset behavior -MetricTool.prototype.reset = _.noop; - -// Returns the name of the metric with its suffix appended to the end. -// NOTE: Should return names in the same order as getValues. -MetricTool.prototype.getName = function() { - return this.name + '.' + this._getSuffix(); -}; - -// Should be implemented by extending classes. Returns the value of the tool for a bucket. -// @param {Number} bucketEndTime - The desired bucket's end time in milliseconds -MetricTool.prototype.getValue = function(bucketEndTime) { - throw new Error("Not implemented"); -}; - -// Returns a list of all primitive metrics this tool is made up of. -// Only requires overridding by non-primitive metrics. -MetricTool.prototype.getPrimitiveMetrics = function() { - return [this]; -}; - -/** - * Counts the number of times an event has occurred in the current bucket. - */ -function Counter(name) { - MetricTool.call(this, name); - this.val = 0; -} -_.extend(Counter.prototype, MetricTool.prototype); - -Counter.prototype.inc = function() { - this.val += 1; -}; - -Counter.prototype._getSuffix = function() { - return 'count'; -}; - -Counter.prototype.getValue = function(bucketEndTime) { - // If the bucket is more recent than the last one where counting occurred, return 0 - return this.val; -}; - -Counter.prototype.reset = function(bucketEndTime) { - this.val = 0; -}; -exports.Counter = Counter; - - -/** - * Keeps track of a count that persists across buckets. - */ -function Gauge(name) { - MetricTool.call(this, name); - this.val = null; -} -_.extend(Gauge.prototype, MetricTool.prototype); - -Gauge.prototype.set = function(num) { - this.val = num; -}; - -Gauge.prototype.inc = function() { - this.val = (this.val ? this.val + 1 : 1); -}; - -Gauge.prototype.dec = function() { - this.val -= 1; -}; - -Gauge.prototype._getSuffix = function() { - return 'total'; -}; - -Gauge.prototype.getValue = function(bucketEndTime) { - return this.val; -}; -exports.Gauge = Gauge; - - -/** - * A gauge that pulls samples using a callback function - */ -function SamplingGauge(name) { - MetricTool.call(this, name); - this.callback = _.constant(null); -} -_.extend(SamplingGauge.prototype, MetricTool.prototype); - -SamplingGauge.prototype.assignCallback = function(callback) { - this.callback = callback; -}; - -SamplingGauge.prototype._getSuffix = function() { - return 'total'; -}; - -SamplingGauge.prototype.getValue = function(bucketEndTime) { - return this.callback(); -}; -exports.SamplingGauge = SamplingGauge; - - -/** - * Keeps track of whether or not a certain condition is met. Useful for statistics - * which measure the number of users who meet a certain criteria. Persists across buckets. - */ -function Switch(name) { - MetricTool.call(this, name); - this.val = null; -} -_.extend(Switch.prototype, Gauge.prototype); - -Switch.prototype.set = function(bool) { - this.val = bool ? 1 : 0; -}; - -Switch.prototype._getSuffix = function() { - return 'instances'; -}; - -Switch.prototype.getValue = function(bucketEndTime) { - return this.val; -}; -exports.Switch = Switch; - - -/** - * Keeps track of the amount of time in each bucket that an event is occurring (ms). - */ -function Timer(name) { - MetricTool.call(this, name); - this.val = 0; // The sum of all runtimes in the last updated bucket - this.startTime = 0; // The time (in ms since the bucket started) when the timer was started - this.running = false; -} -_.extend(Timer.prototype, MetricTool.prototype); - -Timer.prototype.setRunning = function(bool) { - return bool ? this.start() : this.stop(); -}; - -Timer.prototype.start = function() { - if (this.running) { - return; - } - // Record start time and set to running - this.startTime = Date.now(); - this.running = true; -}; - -Timer.prototype.stop = function() { - if (!this.running) { - return; - } - // Add time since start to value and set running to false - var stopTime = Date.now(); - this.val += stopTime - this.startTime; - this.running = false; -}; - -Timer.prototype._getSuffix = function() { - return 'time'; -}; - -Timer.prototype.getValue = function(bucketEndTime) { - // Add the value and the time to the end of the bucket if the timer is running - return this.val + (this.running ? Math.max(0, bucketEndTime - this.startTime) : 0); -}; - -Timer.prototype.reset = function(bucketEndTime) { - this.val = 0; - this.startTime = Math.max(bucketEndTime, this.startTime); -}; -exports.Timer = Timer; - - -/** - * Keeps track of the amount of time in an event takes, and the number of times that event occurs (ms). - */ -function ExecutionTimer(name) { - MetricTool.call(this, name); - this.startTime = 0; // The last time (in ms) the timer was started - this.val = 0; - this.running = false; - // Counter keeps track of the total number of executions in the current bucket. - // An execution is in a bucket if it ended in that bucket. - this.counter = new Counter(name); -} -_.extend(ExecutionTimer.prototype, MetricTool.prototype); - -ExecutionTimer.prototype.setRunning = function(bool) { - return bool ? this.start() : this.stop(); -}; - -ExecutionTimer.prototype.start = function() { - if (this.running) { - return; - } - this.startTime = Date.now(); - this.running = true; -}; - -ExecutionTimer.prototype.stop = function() { - if (!this.running) { - return; - } - var stopTime = Date.now(); - this.val += stopTime - this.startTime; - this.counter.inc(); - this.running = false; -}; - -ExecutionTimer.prototype._getSuffix = function() { - return 'execution_time'; -}; - -ExecutionTimer.prototype.getValue = function(bucketEndTime) { - return this.val; -}; - -ExecutionTimer.prototype.reset = function(bucketEndTime) { - this.val = 0; - this.counter.reset(); -}; - -ExecutionTimer.prototype.getPrimitiveMetrics = function() { - return [this, this.counter]; -}; -exports.ExecutionTimer = ExecutionTimer; - - -// Returns the time rounded down to the start of the current bucket's time window (in ms). -function getBucketStartTime(now) { - return gutil.roundDownToMultiple(now, metricConfig.BUCKET_SIZE); -} -exports.getBucketStartTime = getBucketStartTime; - -// Returns the time until the start of the next bucket (in ms). -function getDeltaMs(now) { - return getBucketStartTime(now) + metricConfig.BUCKET_SIZE - now; -} -exports.getDeltaMs = getDeltaMs; diff --git a/app/common/urlUtils.ts b/app/common/urlUtils.ts index 8dacdf8d..3a2310be 100644 --- a/app/common/urlUtils.ts +++ b/app/common/urlUtils.ts @@ -61,13 +61,6 @@ export function getInitialDocAssignment(): string|null { return getGristConfig().assignmentId || null; } -// Return true if we are on a page that can send metrics. -// TODO: all pages should send suitable metrics. -export function pageHasMetrics(): boolean { - // No metric support on hosted grist. - return !getGristConfig().homeUrl; -} - // Return true if we are on a page that can supply a doc list. // TODO: the doclist object isn't relevant to hosted grist and should be factored out. export function pageHasDocList(): boolean { diff --git a/app/server/declarations.d.ts b/app/server/declarations.d.ts index 4d92661c..025fd10f 100644 --- a/app/server/declarations.d.ts +++ b/app/server/declarations.d.ts @@ -1,6 +1,5 @@ declare module "app/server/lib/ActionLog"; declare module "app/server/lib/sandboxUtil"; -declare module "app/server/lib/ServerMetrics"; declare module "app/server/lib/User"; declare module "app/server/serverMethods"; diff --git a/app/server/devServerMain.ts b/app/server/devServerMain.ts index 52f17e44..419f0d32 100644 --- a/app/server/devServerMain.ts +++ b/app/server/devServerMain.ts @@ -70,7 +70,6 @@ export async function main() { const fileName = path.join(instDir, 'config.json'); if (!(await fse.pathExists(fileName))) { const config = { - enableMetrics: false, untrustedContentOrigin: 'notset', }; await fse.writeFile(fileName, JSON.stringify(config, null, 2)); diff --git a/app/server/lib/DocManager.ts b/app/server/lib/DocManager.ts index 76530fef..0b19bd8c 100644 --- a/app/server/lib/DocManager.ts +++ b/app/server/lib/DocManager.ts @@ -22,7 +22,6 @@ import {GristServer} from 'app/server/lib/GristServer'; import {IDocStorageManager} from 'app/server/lib/IDocStorageManager'; import {makeForkIds, makeId} from 'app/server/lib/idUtils'; import * as log from 'app/server/lib/log'; -import * as ServerMetrics from 'app/server/lib/ServerMetrics'; import {ActiveDoc} from './ActiveDoc'; import {PluginManager} from './PluginManager'; import {getFileUploadInfo, globalUploadSet, makeAccessId, UploadInfo} from './uploads'; @@ -291,10 +290,6 @@ export class DocManager extends EventEmitter { ]); this.emit('open-doc', this.storageManager.getPath(activeDoc.docName)); - ServerMetrics.get('docs.num_open').set(this._activeDocs.size); - ServerMetrics.get('app.have_doc_open').set(true); - ServerMetrics.get('app.doc_open_span').start(); - return { docFD: docSession.fd, clientId: docSession.client.clientId, @@ -327,9 +322,6 @@ export class DocManager extends EventEmitter { public async removeActiveDoc(activeDoc: ActiveDoc): Promise { this._activeDocs.delete(activeDoc.docName); - ServerMetrics.get('docs.num_open').set(this._activeDocs.size); - ServerMetrics.get('app.have_doc_open').set(this._activeDocs.size > 0); - ServerMetrics.get('app.doc_open_span').setRunning(this._activeDocs.size > 0); } public async renameDoc(client: Client, oldName: string, newName: string): Promise { diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 56d9d1a1..41f22b41 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -41,7 +41,6 @@ import {PluginManager} from 'app/server/lib/PluginManager'; import {adaptServerUrl, addOrgToPathIfNeeded, addPermit, getScope, optStringParam, RequestWithGristInfo, stringParam, TEST_HTTPS_OFFSET, trustOrigin} from 'app/server/lib/requestUtils'; import {ISendAppPageOptions, makeSendAppPage} from 'app/server/lib/sendAppPage'; -import * as ServerMetrics from 'app/server/lib/ServerMetrics'; import {getDatabaseUrl} from 'app/server/lib/serverUtils'; import {Sessions} from 'app/server/lib/Sessions'; import * as shutdown from 'app/server/lib/shutdown'; @@ -109,7 +108,6 @@ export class FlexServer implements GristServer { private _pluginManager: PluginManager; private _storageManager: IDocStorageManager; private _docWorkerMap: IDocWorkerMap; - private _serverMetrics: any; private _disabled: boolean = false; private _disableS3: boolean = false; private _healthy: boolean = true; // becomes false if a serious error has occurred and @@ -809,22 +807,14 @@ export class FlexServer implements GristServer { this._docManager = docManager; } - // Add document-related endpoints and related support. A testAvoidMetrics flag - // is added to disable touching metrics if tests have taken care of stubbing them. - public async addDoc(testAvoidMetrics: boolean = false) { + // Add document-related endpoints and related support. + public async addDoc() { this._check('doc', 'start', 'tag', 'json', isSingleUserMode() ? null : 'homedb', 'api-mw', 'map'); // add handlers for cleanup, if we are in charge of the doc manager. if (!this._docManager) { this.addCleanup(); } await this.loadConfig(); this.addComm(); - // TODO: metrics collection might be best left to a separate service, but we need an - // instantiated object because various code attempts to report metrics. - if (!testAvoidMetrics) { - this._serverMetrics = new ServerMetrics(); - this._serverMetrics.handlePreferences(this.settings); - } - if (!isSingleUserMode()) { const s3Bucket = this._disableS3 ? '' : (process.env.GRIST_DOCS_S3_BUCKET || ''); const s3Prefix = process.env.GRIST_DOCS_S3_PREFIX || "docs/"; @@ -888,14 +878,6 @@ export class FlexServer implements GristServer { } } - // Must be called *after* metrics have been created. - public disableMetrics() { - if (!this._serverMetrics) { - throw new Error('disableMetrics called too early'); - } - this._serverMetrics.disable(); - } - public disableS3() { if (this.deps.has('doc')) { throw new Error('disableS3 called too late'); diff --git a/app/server/lib/ServerMetrics.js b/app/server/lib/ServerMetrics.js deleted file mode 100644 index 2bbaee85..00000000 --- a/app/server/lib/ServerMetrics.js +++ /dev/null @@ -1,205 +0,0 @@ -const _ = require('underscore'); -const net = require('net'); -const Promise = require('bluebird'); -const log = require('./log'); -const MetricCollector = require('app/common/MetricCollector'); -const metricConfig = require('app/common/metricConfig'); -const shutdown = require('./shutdown'); -const version = require('app/common/version'); -const crypto = require('crypto'); - -// Grist Metrics EC2 instance host and port -const host = 'metrics.getgrist.com'; -const port = '2023'; // Plain-text port of carbon-aggregator - -// Global reference to an instance of this class established in the constuctor. -var globalServerMetrics = null; - -/** - * Server-facing class for initializing server metrics collection. - * Establishes interval attempts to push measured server metrics to the prometheus PushGateway - * on creation. - * @param {Object} user - Instance of User.js server class, which contains config settings. - */ -function ServerMetrics() { - MetricCollector.call(this); - this.socket = null; - // Randomly generated id to differentiate between metrics from this server and others. - this.serverId = crypto.randomBytes(8).toString('hex'); - this.serverMetrics = this.initMetricTools(metricConfig.serverMetrics); - this.clientNames = null; - this.enabled = false; - // Produce the prefix string for all metrics. - // NOTE: If grist-rt is used instead of grist-raw for some metrics, this must be changed. - let versionStr = version.version.replace(/\W/g, '-'); - let channelStr = version.channel.replace(/\W/g, '-'); - this._prefix = `grist-raw.instance.${channelStr}.${versionStr}`; - - globalServerMetrics = this; - - // This will not send metrics when they are disabled since there is a check in pushMetrics. - shutdown.addCleanupHandler(null, () => this.attemptPush()); -} -_.extend(ServerMetrics.prototype, MetricCollector.prototype); - -/** - * Checks the given preferences object from the user configuration and starts pushing metrics - * to carbon if metrics are enabled. Otherwise, ends the socket connection if there is one. - */ -ServerMetrics.prototype.handlePreferences = function(config) { - config = config || {}; - this.enabled = config.enableMetrics; - Promise.resolve(this.enabled && this._connectSocket()) - .then(() => { - if (this.enabled) { - this._push = setTimeout(() => this.attemptPush(), metricConfig.SERVER_PUSH_INTERVAL); - } else if (this.socket) { - this.socket.end(); - } - }); -}; - -ServerMetrics.prototype.disable = function() { - this.enabled = false; - if (this._push) { - clearTimeout(this._push); - this._push = null; - } - if (this._collect) { - clearTimeout(this._collect); - this._collect = null; - } -} - - -/** - * Returns a promise for a socket connection to the Carbon metrics collection server. - * The promise will not fail because of connection errors, rather it will be continuously - * re-evaluated until it connects. The retry rate is specified in metricConfig.js - */ -ServerMetrics.prototype._connectSocket = function() { - if (!this.enabled) { return Promise.resolve(); } - var socket = null; - log.info('Attempting connection to Carbon metrics server'); - return new Promise((resolve, reject) => { - socket = net.connect({host: host, port: port}, () => { - log.info('Connected to Carbon metrics server'); - this.socket = socket; - resolve(); - }); - socket.setEncoding('utf8'); - socket.on('error', err => { - log.warn('Carbon metrics connection error: %s', err); - if (this.socket) { - this.socket.end(); - this.socket = null; - } - reject(err); - }); - }) - .catch(() => { - return Promise.delay(metricConfig.CONN_RETRY) - .then(() => this._connectSocket()); - }); -}; - -// Returns a map from metric names (as entered in metricConfig.js) to their metricTools. -ServerMetrics.prototype.getMetrics = function() { - return this.serverMetrics; -}; - -// Pushes ready server and client metrics to the aggregator -ServerMetrics.prototype.pushMetrics = function(metrics) { - if (this.enabled) { - return this._request(metrics.join("")) - .finally(() => { - this._push = setTimeout(() => this.attemptPush(), metricConfig.SERVER_PUSH_INTERVAL); - }); - } -}; - -ServerMetrics.prototype._request = function(text) { - return new Promise(resolve => { - if (!this.enabled) { - resolve(); - return; - } - this.socket.write(text, 'utf8', () => { - log.info('Pushed metrics to Carbon'); - resolve(); - }); - }) - .catch(() => { - return this._connectSocket() - .then(() => this._request(text)); - }); -}; - -/** - * Function exposed to comm interface to provide server with client list of metrics. - * Used so that ServerMetrics can associate indices to client metric names. - * @param {Array} metricNames - A list of client metric names in the order in which values will be sent. - */ -ServerMetrics.prototype.registerClientMetrics = function(client, metricNames) { - this.clientNames = metricNames; -}; - -/** - * Function exposed to comm interface to allow client metrics to be pushed to this file, - * so that they may in turn be pushed to Carbon with the server metrics. - * @param {Array} data - A list of client buckets as defined in ClientMetrics.js's createBucket - */ -ServerMetrics.prototype.pushClientMetrics = function(client, data) { - // Merge ready client bucket metrics into ready server buckets. - if (!this.clientNames) { - throw new Error("Client metrics must be registered"); - } - data.forEach(clientBucket => { - // Label the bucket with the client id so that clients' metrics do not replace one another - let clientData = clientBucket.values.map((val, i) => { - return this._stringifyMetric(this.clientNames[i], client.clientId, val, clientBucket.startTime); - }).join(""); - this.queueBucket(clientData); - }); -}; - -ServerMetrics.prototype.get = function(name) { - this.prepareCompletedBuckets(Date.now()); - return this.serverMetrics[name]; -}; - -/** - * Creates string bucket with metrics in carbon's text format. - * For details, see phriction documentation: https://phab.getgrist.com/w/metrics/ - */ -ServerMetrics.prototype.createBucket = function(bucketStart) { - var data = []; - var bucketEnd = bucketStart + metricConfig.BUCKET_SIZE; - this.forEachBucketMetric(bucketEnd, tool => { - if (tool.getValue(bucketEnd) !== null) { - data.push(this._stringifyMetric(tool.getName(), this.serverId, tool.getValue(bucketEnd), bucketStart)); - } - }); - return data.join(""); -}; - -// Helper to stringify individual metrics for carbon's text format. -ServerMetrics.prototype._stringifyMetric = function(name, id, val, startTime) { - // Server/client id is added to name for differentiating inputs to aggregator - return `${this._prefix}.${name}.${id} ${val} ${startTime/1000}\n`; -}; - -/** - * Static get method to retreive server metric recording tools. - * IMPORTANT: Usage involves the side effect of updating completed buckets and - * adding them to a ready object. get() results should not be assigned to variables and - * reused, rather get() should be called each time a metric is needed. - */ -ServerMetrics.get = function(name) { - if (!globalServerMetrics) { - throw new Error('Must create ServerMetrics instance to access server metrics.'); - } - return globalServerMetrics.get(name); -}; - -module.exports = ServerMetrics; diff --git a/app/server/mergedServerMain.ts b/app/server/mergedServerMain.ts index 18b5a399..089717a4 100644 --- a/app/server/mergedServerMain.ts +++ b/app/server/mergedServerMain.ts @@ -34,7 +34,6 @@ interface ServerOptions extends FlexServerOptions { logToConsole?: boolean; // If set, messages logged to console (default: false) // (but if options are not given at all in call to main, // logToConsole is set to true) - metrics?: boolean; // If set, metric collector is enabled (default: true) s3?: boolean; // If set, documents saved to s3 (default is to check environment // variables, which get set in various ways in dev/test entry points) } @@ -131,10 +130,6 @@ export async function main(port: number, serverTypes: ServerType[], await server.addDoc(); } - if (options.metrics === false && includeDocs) { - server.disableMetrics(); - } - server.finalize(); server.summary();