From 8f962ff49532c5f8bb08a071e3bd8b1d1ebe7f72 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Wed, 4 Oct 2023 12:58:30 +0400 Subject: [PATCH 01/10] refactor: add sentry dsn rotation logic --- src/Config/CliBuilder.ts | 36 +++++++- src/Config/SystemConfigReader.ts | 153 +++++++++++++++++++++++++++++++ src/Utils/Sentry.ts | 20 ---- src/Utils/index.ts | 1 - src/index.ts | 3 - 5 files changed, 184 insertions(+), 29 deletions(-) create mode 100644 src/Config/SystemConfigReader.ts delete mode 100644 src/Utils/Sentry.ts diff --git a/src/Config/CliBuilder.ts b/src/Config/CliBuilder.ts index 2b970e8a..a9327fe5 100644 --- a/src/Config/CliBuilder.ts +++ b/src/Config/CliBuilder.ts @@ -1,8 +1,13 @@ import { CliConfig, ConfigReader } from './ConfigReader'; import { ClusterArgs, Helpers, logger, LogLevel } from '../Utils'; +import { SystemConfigReader } from './SystemConfigReader'; import { CliInfo } from './CliInfo'; import { Arguments, Argv, CommandModule } from 'yargs'; -import { runWithAsyncContext, setContext } from '@sentry/node'; +import { + init as initSentry, + runWithAsyncContext as runSentryWithAsyncContext, + setContext as setSentryContext +} from '@sentry/node'; export interface CliBuilderOptions { info: CliInfo; @@ -89,12 +94,33 @@ export class CliBuilder { .reduce((acc: Argv, item: CommandModule) => { const handler = item.handler.bind(item); - item.handler = (args) => - runWithAsyncContext(() => { - setContext('args', args); + item.handler = async (args: Arguments) => { + const systemConfigReader = new SystemConfigReader(args.api as string); + const systemConfig = await systemConfigReader.read(); - handler(args); + initSentry({ + attachStacktrace: true, + dsn: systemConfig.sentryDsn, + release: process.env.VERSION, + beforeSend: (event) => { + if (event.contexts.args) { + event.contexts.args = { + ...event.contexts.args, + t: event.contexts.args.t && '[Filtered]', + token: event.contexts.args.token && '[Filtered]' + }; + } + + return event; + } + }); + + return runSentryWithAsyncContext(() => { + setSentryContext('args', args); + + return handler(args); }); + }; acc.command(item); diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts new file mode 100644 index 00000000..11319c4e --- /dev/null +++ b/src/Config/SystemConfigReader.ts @@ -0,0 +1,153 @@ +import { logger } from '../Utils'; +import request, { RequestPromiseAPI } from 'request-promise'; +import { promises, constants } from 'fs'; +import { join } from 'path'; +import { homedir } from 'os'; + +const { readFile, writeFile, access } = promises; +const { F_OK } = constants; + +export interface SystemConfig { + sentryDsn?: string; +} + +interface SystemConfigFile { + data: SystemConfig; + updatedAt: Date; +} + +export class SystemConfigReader { + private readonly rotationInterval = 3600000; + private readonly path = join(homedir(), '.brightclirc'); + private client: RequestPromiseAPI; + + constructor(baseUrl: string) { + this.client = request.defaults({ + baseUrl, + timeout: 1500, + json: true + }); + } + + public async read(): Promise { + await this.rotateIfNecessary(); + const configFile = await this.getConfigFile(); + + return { + sentryDsn: process.env['SENTRY_DSN'], + ...configFile.data + }; + } + + private async rotateIfNecessary() { + if (process.env.NODE_ENV !== 'production') { + return; + } + + logger.debug('Trying to rotate system config'); + + const configFile = await this.getConfigFile(); + const lifeTime = Date.now() - configFile.updatedAt.getTime(); + + if (lifeTime < this.rotationInterval) { + logger.debug( + 'Rotation is not needed, system config is fresh: %s ms', + lifeTime + ); + + return; + } + + logger.debug( + "Rotating system config because it's too old: %s ms", + lifeTime + ); + + const newConfig = await this.fetchNewConfig(); + + if (newConfig) { + await this.updateConfigFile({ + data: newConfig, + updatedAt: new Date() + }); + } else { + logger.debug('Rotation failed'); + + await this.updateConfigFile({ + ...configFile, + updatedAt: new Date() + }); + } + } + + private defaultConfigFile(): SystemConfigFile { + return { + data: {}, + updatedAt: new Date() + }; + } + + private async getConfigFile() { + const defaultConfigFile = this.defaultConfigFile(); + + try { + logger.debug('Loading system config file'); + + const configFileExists = await this.configFileExists(); + + if (!configFileExists) { + logger.debug( + "System config file doesn't exist. Creating a new one with default values" + ); + + await this.updateConfigFile(defaultConfigFile); + } + + const file = await readFile(this.path); + const fileConfig = JSON.parse(file.toString()) as SystemConfigFile; + fileConfig.updatedAt = new Date(fileConfig.updatedAt); + + return fileConfig; + } catch (e) { + logger.debug('Error during loading system config file', e); + } + + logger.debug('Using default system config file'); + + return defaultConfigFile; + } + + private async updateConfigFile(configFile: SystemConfigFile) { + logger.debug('Updating system config file'); + + try { + await writeFile(this.path, JSON.stringify(configFile)); + } catch (e) { + logger.debug('Error during updating system config file', e); + } + } + + private async configFileExists(): Promise { + try { + await access(this.path, F_OK); + + return true; + } catch (e) { + // pass + } + + return false; + } + + private async fetchNewConfig(): Promise { + logger.debug('Fetching new system config'); + + try { + return await this.client.get({ + uri: '/api/v1/cli/config' + }); + } catch (e) { + logger.debug('Error during fetching new system config: ', e); + } + } +} diff --git a/src/Utils/Sentry.ts b/src/Utils/Sentry.ts deleted file mode 100644 index dfe0aced..00000000 --- a/src/Utils/Sentry.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { init } from '@sentry/node'; - -export function sentry() { - init({ - attachStacktrace: true, - dsn: process.env.SENTRY_DSN, - release: process.env.VERSION, - beforeSend: (event) => { - if (event.contexts.args) { - event.contexts.args = { - ...event.contexts.args, - t: event.contexts.args.t && '[Filtered]', - token: event.contexts.args.token && '[Filtered]' - }; - } - - return event; - } - }); -} diff --git a/src/Utils/index.ts b/src/Utils/index.ts index 201c3f9a..332df095 100644 --- a/src/Utils/index.ts +++ b/src/Utils/index.ts @@ -3,4 +3,3 @@ export * from './Helpers'; export * from './Logger'; export * from './Backoff'; export * from './Traceroute'; -export * from './Sentry'; diff --git a/src/index.ts b/src/index.ts index f21844a8..bba22d6d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,9 +16,6 @@ import { Integration } from './Commands'; import { CliBuilder, container } from './Config'; -import { sentry } from './Utils'; - -sentry(); container.resolve(CliBuilder).build({ commands: [ From eeed3de5aecd5a2f5d8e405a43c2f42e0bb51777 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 15:29:57 +0400 Subject: [PATCH 02/10] refactor: reexport `Sentry` from utils --- src/Config/CliBuilder.ts | 13 ++++--------- src/Utils/Sentry.ts | 3 +++ src/Utils/index.ts | 1 + 3 files changed, 8 insertions(+), 9 deletions(-) create mode 100644 src/Utils/Sentry.ts diff --git a/src/Config/CliBuilder.ts b/src/Config/CliBuilder.ts index a9327fe5..39bff5f0 100644 --- a/src/Config/CliBuilder.ts +++ b/src/Config/CliBuilder.ts @@ -1,13 +1,8 @@ import { CliConfig, ConfigReader } from './ConfigReader'; -import { ClusterArgs, Helpers, logger, LogLevel } from '../Utils'; +import { ClusterArgs, Helpers, logger, LogLevel, Sentry } from '../Utils'; import { SystemConfigReader } from './SystemConfigReader'; import { CliInfo } from './CliInfo'; import { Arguments, Argv, CommandModule } from 'yargs'; -import { - init as initSentry, - runWithAsyncContext as runSentryWithAsyncContext, - setContext as setSentryContext -} from '@sentry/node'; export interface CliBuilderOptions { info: CliInfo; @@ -98,7 +93,7 @@ export class CliBuilder { const systemConfigReader = new SystemConfigReader(args.api as string); const systemConfig = await systemConfigReader.read(); - initSentry({ + Sentry.init({ attachStacktrace: true, dsn: systemConfig.sentryDsn, release: process.env.VERSION, @@ -115,8 +110,8 @@ export class CliBuilder { } }); - return runSentryWithAsyncContext(() => { - setSentryContext('args', args); + return Sentry.runWithAsyncContext(() => { + Sentry.setContext('args', args); return handler(args); }); diff --git a/src/Utils/Sentry.ts b/src/Utils/Sentry.ts new file mode 100644 index 00000000..e3a504ad --- /dev/null +++ b/src/Utils/Sentry.ts @@ -0,0 +1,3 @@ +import { init, setContext, runWithAsyncContext } from '@sentry/node'; + +export default { init, setContext, runWithAsyncContext }; diff --git a/src/Utils/index.ts b/src/Utils/index.ts index 332df095..8b1cce64 100644 --- a/src/Utils/index.ts +++ b/src/Utils/index.ts @@ -3,3 +3,4 @@ export * from './Helpers'; export * from './Logger'; export * from './Backoff'; export * from './Traceroute'; +export { default as Sentry } from './Sentry'; From 63e05e606a5535e34f7a0a1f699b1d1c70762925 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 15:33:06 +0400 Subject: [PATCH 03/10] refactor: use shorthand syntax --- src/Config/CliBuilder.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config/CliBuilder.ts b/src/Config/CliBuilder.ts index 39bff5f0..cd4328e1 100644 --- a/src/Config/CliBuilder.ts +++ b/src/Config/CliBuilder.ts @@ -97,7 +97,7 @@ export class CliBuilder { attachStacktrace: true, dsn: systemConfig.sentryDsn, release: process.env.VERSION, - beforeSend: (event) => { + beforeSend(event) { if (event.contexts.args) { event.contexts.args = { ...event.contexts.args, From 648fd1d6c9c7c1bdc36ac7da508e6ef410280406 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 16:02:34 +0400 Subject: [PATCH 04/10] refactor: make `client` readonly --- src/Config/SystemConfigReader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts index 11319c4e..9073cc4e 100644 --- a/src/Config/SystemConfigReader.ts +++ b/src/Config/SystemConfigReader.ts @@ -19,7 +19,7 @@ interface SystemConfigFile { export class SystemConfigReader { private readonly rotationInterval = 3600000; private readonly path = join(homedir(), '.brightclirc'); - private client: RequestPromiseAPI; + private readonly client: RequestPromiseAPI; constructor(baseUrl: string) { this.client = request.defaults({ From 0f1a93e0452675895248c26ec06e1feeaff5bb0b Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 16:04:20 +0400 Subject: [PATCH 05/10] refactor: reorganize code --- src/Config/SystemConfigReader.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts index 9073cc4e..73b017da 100644 --- a/src/Config/SystemConfigReader.ts +++ b/src/Config/SystemConfigReader.ts @@ -110,11 +110,10 @@ export class SystemConfigReader { return fileConfig; } catch (e) { logger.debug('Error during loading system config file', e); - } - - logger.debug('Using default system config file'); + logger.debug('Using default system config file'); - return defaultConfigFile; + return defaultConfigFile; + } } private async updateConfigFile(configFile: SystemConfigFile) { @@ -133,10 +132,8 @@ export class SystemConfigReader { return true; } catch (e) { - // pass + return false; } - - return false; } private async fetchNewConfig(): Promise { From 0d053a7f299ad6f46b88b809bb95d6133f0474cb Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 16:06:05 +0400 Subject: [PATCH 06/10] refactor: remove mutation --- src/Config/SystemConfigReader.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts index 73b017da..7c3dd389 100644 --- a/src/Config/SystemConfigReader.ts +++ b/src/Config/SystemConfigReader.ts @@ -105,9 +105,11 @@ export class SystemConfigReader { const file = await readFile(this.path); const fileConfig = JSON.parse(file.toString()) as SystemConfigFile; - fileConfig.updatedAt = new Date(fileConfig.updatedAt); - return fileConfig; + return { + ...fileConfig, + updatedAt: new Date(fileConfig.updatedAt) + }; } catch (e) { logger.debug('Error during loading system config file', e); logger.debug('Using default system config file'); From 969a52045067fcd856602f2385a2fefd8107b4d2 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 16:14:14 +0400 Subject: [PATCH 07/10] refactor: extract rotation detection logic --- src/Config/SystemConfigReader.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts index 7c3dd389..c932618c 100644 --- a/src/Config/SystemConfigReader.ts +++ b/src/Config/SystemConfigReader.ts @@ -39,28 +39,33 @@ export class SystemConfigReader { }; } - private async rotateIfNecessary() { + private needsRotation(configFile: SystemConfigFile) { if (process.env.NODE_ENV !== 'production') { return; } + const lifeTime = Date.now() - configFile.updatedAt.getTime(); + + return lifeTime >= this.rotationInterval; + } + + private async rotateIfNecessary() { logger.debug('Trying to rotate system config'); const configFile = await this.getConfigFile(); - const lifeTime = Date.now() - configFile.updatedAt.getTime(); - if (lifeTime < this.rotationInterval) { + if (!this.needsRotation(configFile)) { logger.debug( - 'Rotation is not needed, system config is fresh: %s ms', - lifeTime + 'Rotation is not needed, last updated on: %s ms', + configFile.updatedAt ); return; } logger.debug( - "Rotating system config because it's too old: %s ms", - lifeTime + 'Rotating system config last updated on: %s ms', + configFile.updatedAt ); const newConfig = await this.fetchNewConfig(); From 2b64e7ade97f3f99dcf3de0e0de91a16c8e4a625 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Thu, 5 Oct 2023 16:25:06 +0400 Subject: [PATCH 08/10] refactor: remove config file existing checks --- src/Config/SystemConfigReader.ts | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts index c932618c..9e5b49a8 100644 --- a/src/Config/SystemConfigReader.ts +++ b/src/Config/SystemConfigReader.ts @@ -1,11 +1,10 @@ import { logger } from '../Utils'; import request, { RequestPromiseAPI } from 'request-promise'; -import { promises, constants } from 'fs'; +import { promises } from 'fs'; import { join } from 'path'; import { homedir } from 'os'; -const { readFile, writeFile, access } = promises; -const { F_OK } = constants; +const { readFile, writeFile } = promises; export interface SystemConfig { sentryDsn?: string; @@ -98,16 +97,6 @@ export class SystemConfigReader { try { logger.debug('Loading system config file'); - const configFileExists = await this.configFileExists(); - - if (!configFileExists) { - logger.debug( - "System config file doesn't exist. Creating a new one with default values" - ); - - await this.updateConfigFile(defaultConfigFile); - } - const file = await readFile(this.path); const fileConfig = JSON.parse(file.toString()) as SystemConfigFile; @@ -133,16 +122,6 @@ export class SystemConfigReader { } } - private async configFileExists(): Promise { - try { - await access(this.path, F_OK); - - return true; - } catch (e) { - return false; - } - } - private async fetchNewConfig(): Promise { logger.debug('Fetching new system config'); From ab4e472d5eb1ad18c97796d9aa8ffe70744394f0 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Fri, 6 Oct 2023 10:57:58 +0400 Subject: [PATCH 09/10] refactor: move sentry logic to `wrapWithSentry` --- src/Config/CliBuilder.ts | 74 +++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/src/Config/CliBuilder.ts b/src/Config/CliBuilder.ts index cd4328e1..cc2d5463 100644 --- a/src/Config/CliBuilder.ts +++ b/src/Config/CliBuilder.ts @@ -86,41 +86,11 @@ export class CliBuilder { ); return commands - .reduce((acc: Argv, item: CommandModule) => { - const handler = item.handler.bind(item); - - item.handler = async (args: Arguments) => { - const systemConfigReader = new SystemConfigReader(args.api as string); - const systemConfig = await systemConfigReader.read(); - - Sentry.init({ - attachStacktrace: true, - dsn: systemConfig.sentryDsn, - release: process.env.VERSION, - beforeSend(event) { - if (event.contexts.args) { - event.contexts.args = { - ...event.contexts.args, - t: event.contexts.args.t && '[Filtered]', - token: event.contexts.args.token && '[Filtered]' - }; - } - - return event; - } - }); - - return Sentry.runWithAsyncContext(() => { - Sentry.setContext('args', args); - - return handler(args); - }); - }; - - acc.command(item); - - return acc; - }, cli) + .reduce( + (acc: Argv, item: CommandModule) => + acc.command(this.wrapWithSentry(item)), + cli + ) .recommendCommands() .demandCommand(1) .strict(true) @@ -130,4 +100,38 @@ export class CliBuilder { .alias('h', 'help') .wrap(null); } + + private wrapWithSentry(command: CommandModule) { + const handler = command.handler.bind(command); + + command.handler = async (args: Arguments) => { + const systemConfigReader = new SystemConfigReader(args.api as string); + const systemConfig = await systemConfigReader.read(); + + Sentry.init({ + attachStacktrace: true, + dsn: systemConfig.sentryDsn, + release: process.env.VERSION, + beforeSend(event) { + if (event.contexts.args) { + event.contexts.args = { + ...event.contexts.args, + t: event.contexts.args.t && '[Filtered]', + token: event.contexts.args.token && '[Filtered]' + }; + } + + return event; + } + }); + + return Sentry.runWithAsyncContext(() => { + Sentry.setContext('args', args); + + return handler(args); + }); + }; + + return command; + } } From ee07c4d6ec661772745381595ac1ef20f6e46175 Mon Sep 17 00:00:00 2001 From: Alexander Rodionov Date: Fri, 6 Oct 2023 11:03:07 +0400 Subject: [PATCH 10/10] refactor: replace `fs.promises` with promisify --- src/Config/SystemConfigReader.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Config/SystemConfigReader.ts b/src/Config/SystemConfigReader.ts index 9e5b49a8..e6e2c689 100644 --- a/src/Config/SystemConfigReader.ts +++ b/src/Config/SystemConfigReader.ts @@ -1,10 +1,9 @@ import { logger } from '../Utils'; import request, { RequestPromiseAPI } from 'request-promise'; -import { promises } from 'fs'; +import { readFile, writeFile } from 'fs'; import { join } from 'path'; import { homedir } from 'os'; - -const { readFile, writeFile } = promises; +import { promisify } from 'util'; export interface SystemConfig { sentryDsn?: string; @@ -97,7 +96,7 @@ export class SystemConfigReader { try { logger.debug('Loading system config file'); - const file = await readFile(this.path); + const file = await promisify(readFile)(this.path); const fileConfig = JSON.parse(file.toString()) as SystemConfigFile; return { @@ -116,7 +115,7 @@ export class SystemConfigReader { logger.debug('Updating system config file'); try { - await writeFile(this.path, JSON.stringify(configFile)); + await promisify(writeFile)(this.path, JSON.stringify(configFile)); } catch (e) { logger.debug('Error during updating system config file', e); }