Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: add sentry dsn rotation logic #466

Merged
merged 10 commits into from Oct 7, 2023
57 changes: 41 additions & 16 deletions src/Config/CliBuilder.ts
@@ -1,8 +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 { runWithAsyncContext, setContext } from '@sentry/node';

export interface CliBuilderOptions {
info: CliInfo;
Expand Down Expand Up @@ -86,20 +86,11 @@ export class CliBuilder {
);

return commands
.reduce((acc: Argv, item: CommandModule) => {
const handler = item.handler.bind(item);

item.handler = (args) =>
runWithAsyncContext(() => {
setContext('args', args);

handler(args);
});

acc.command(item);

return acc;
}, cli)
.reduce(
(acc: Argv, item: CommandModule) =>
acc.command(this.wrapWithSentry(item)),
cli
)
.recommendCommands()
.demandCommand(1)
.strict(true)
Expand All @@ -109,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;
}
}
135 changes: 135 additions & 0 deletions src/Config/SystemConfigReader.ts
@@ -0,0 +1,135 @@
import { logger } from '../Utils';
import request, { RequestPromiseAPI } from 'request-promise';
import { readFile, writeFile } from 'fs';
import { join } from 'path';
import { homedir } from 'os';
import { promisify } from 'util';

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 readonly client: RequestPromiseAPI;

constructor(baseUrl: string) {
this.client = request.defaults({
baseUrl,
timeout: 1500,
json: true
});
}

public async read(): Promise<SystemConfig> {
await this.rotateIfNecessary();
const configFile = await this.getConfigFile();

return {
sentryDsn: process.env['SENTRY_DSN'],
...configFile.data
};
}

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();

if (!this.needsRotation(configFile)) {
logger.debug(
'Rotation is not needed, last updated on: %s ms',
configFile.updatedAt
);

return;
}

logger.debug(
'Rotating system config last updated on: %s ms',
configFile.updatedAt
);

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()
});
Comment on lines +79 to +82
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this is redundant. Should we update the configuration as though the rotation was successful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, just because if we don't do that, it will try to perform rotation on every bright-cli call causing at most 1.5 seconds delay.

Since rotation logic is not crucial for the cli for now, I think it's ok in case of failure to repeat it later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you wish. However, I'm still uncertain about how it is supposed to work for long-running processes like the repeater which is intended to work for months.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have a separate PR for the background rotation

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsndr please open a ticket in Jira

}
}

private defaultConfigFile(): SystemConfigFile {
return {
data: {},
updatedAt: new Date()
};
}

private async getConfigFile() {
const defaultConfigFile = this.defaultConfigFile();

try {
logger.debug('Loading system config file');

const file = await promisify(readFile)(this.path);
const fileConfig = JSON.parse(file.toString()) as SystemConfigFile;

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');

return defaultConfigFile;
}
}

private async updateConfigFile(configFile: SystemConfigFile) {
logger.debug('Updating system config file');

try {
await promisify(writeFile)(this.path, JSON.stringify(configFile));
} catch (e) {
logger.debug('Error during updating system config file', e);
}
}

private async fetchNewConfig(): Promise<SystemConfig | undefined> {
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);
}
}
}
21 changes: 2 additions & 19 deletions src/Utils/Sentry.ts
@@ -1,20 +1,3 @@
import { init } from '@sentry/node';
import { init, setContext, runWithAsyncContext } 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;
}
});
}
export default { init, setContext, runWithAsyncContext };
2 changes: 1 addition & 1 deletion src/Utils/index.ts
Expand Up @@ -3,4 +3,4 @@ export * from './Helpers';
export * from './Logger';
export * from './Backoff';
export * from './Traceroute';
export * from './Sentry';
export { default as Sentry } from './Sentry';
3 changes: 0 additions & 3 deletions src/index.ts
Expand Up @@ -16,9 +16,6 @@ import {
Integration
} from './Commands';
import { CliBuilder, container } from './Config';
import { sentry } from './Utils';

sentry();

container.resolve(CliBuilder).build({
commands: [
Expand Down