From 9924132af8bc525e5d47be375af04cb3fba0266e Mon Sep 17 00:00:00 2001 From: Celso Inacio Date: Wed, 21 Feb 2024 14:32:15 -0300 Subject: [PATCH 1/2] fix: code according lint rules --- lib/domain/ErrorTracker.js | 13 +++-- lib/domain/Pipeline.js | 28 +++++----- lib/http/SchemaResponse.js | 5 +- lib/http/routers/FunctionsRouter.js | 53 +++++++------------ lib/http/routes.js | 2 +- lib/support/config.js | 2 +- lib/support/opentelemetry.js | 43 ++++++++------- .../unit/http/routers/FunctionsRouter.test.js | 2 +- 8 files changed, 69 insertions(+), 79 deletions(-) diff --git a/lib/domain/ErrorTracker.js b/lib/domain/ErrorTracker.js index 12d34ba..77288be 100644 --- a/lib/domain/ErrorTracker.js +++ b/lib/domain/ErrorTracker.js @@ -44,7 +44,7 @@ function getSandboxModules() { function logSentryPublishError(ctx, publishErr, layer = '') { const { event_id, tags } = ctx; const { codeHash } = tags; - const { reason, statusCode, message, code, requestTime} = publishErr; + const { reason, statusCode, message, code, requestTime } = publishErr; const prettyPublishError = { reason, @@ -56,10 +56,9 @@ function logSentryPublishError(ctx, publishErr, layer = '') { requestTime, codeHash, }; - - - if(prettyPublishError.hostname.private_key) - delete prettyPublishError.hostname['private_key'] + if (prettyPublishError.hostname.private_key) { + delete prettyPublishError.hostname.private_key; + } log.error('Failed to publish error into %s sentry', layer, prettyPublishError); } @@ -70,10 +69,10 @@ function notifySentryGlobal(ctx) { return; } ctx.event_id = raven.generateEventId(); - const timeStart = Date.now() + const timeStart = Date.now(); raven.send(ctx, (publishErr) => { if (publishErr) { - publishErr.requestTime = Date.now() - timeStart + publishErr.requestTime = Date.now() - timeStart; logSentryPublishError(ctx, publishErr, 'global'); return; } diff --git a/lib/domain/Pipeline.js b/lib/domain/Pipeline.js index 24168b5..bdba41e 100644 --- a/lib/domain/Pipeline.js +++ b/lib/domain/Pipeline.js @@ -48,7 +48,7 @@ class Pipeline { try { const runScriptMetric = new Metric('run-script'); const result = await this.sandbox.runScript(step.script, this.req, options); - runScriptMetric.observeFunctionRunScript({ namespace, id }); + runScriptMetric.observeFunctionRunScript({ namespace: step.namespace, id: step.id }); const spent = metric.observeFunctionRun({ namespace: step.namespace, @@ -65,11 +65,11 @@ class Pipeline { method: this.req.method, request_uri: this.req.url, status: result.status, - request_id: this.req.headers["x-request-id"] || "", + request_id: this.req.headers['x-request-id'] || '', request_time: spent, namespace: step.namespace, function_id: step.id, - }) + }); const previousResult = this.previousResult; this.previousResult = result; @@ -104,15 +104,17 @@ class Pipeline { status, }); - log.info({ - method: this.req.method, - request_uri: this.req.url, - status: status, - request_id: this.req.headers["x-request-id"] || "", - request_time: spent, - namespace: step.namespace, - function_id: step.id, - }) + if (err.constructor.name === 'SandboxInternalException') { + log.info({ + method: this.req.method, + request_uri: this.req.url, + status, + request_id: this.req.headers['x-request-id'] || '', + request_time: spent, + namespace: step.namespace, + function_id: step.id, + }); + } logStorage.console.error(`Failed to run function: ${err.message}, ${spent} seconds`); logStorage.console.error(err.stack); @@ -139,7 +141,7 @@ class Pipeline { err.details = { message: `Got error when execute pipeline function ${step.namespace}/${step.id}`, - } + }; throw err; } diff --git a/lib/http/SchemaResponse.js b/lib/http/SchemaResponse.js index fcd5fbf..32ce2fb 100644 --- a/lib/http/SchemaResponse.js +++ b/lib/http/SchemaResponse.js @@ -10,8 +10,9 @@ class SchemaResponse { removeNotAllowedFields(data) { const { env } = data; - if(env){ - const fieldsToRemove = Object.keys(env).filter(field => this.notAllowedFields.includes(field)); + if (env) { + const fieldsToRemove = Object.keys(env) + .filter(field => this.notAllowedFields.includes(field)); for (const field of fieldsToRemove) { delete data.env[field]; } diff --git a/lib/http/routers/FunctionsRouter.js b/lib/http/routers/FunctionsRouter.js index f45b8cb..50aa21a 100644 --- a/lib/http/routers/FunctionsRouter.js +++ b/lib/http/routers/FunctionsRouter.js @@ -17,7 +17,7 @@ const SchemaResponse = require('../SchemaResponse'); const router = new Router(); const { bodyParserLimit } = require('../../support/config'); const { reportError } = require('../../support/tracing'); -const { RecordOtelError } = require('../../support/opentelemetry') +const { RecordOtelError } = require('../../support/opentelemetry'); function codeFileName(namespace, codeId) { return `${namespace}/${codeId}.js`; @@ -200,7 +200,7 @@ router.delete('/:namespace/:id', async (req, res, next) => { }); -router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), async (req, res, next) => { +router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), async (req, res) => { const { namespace, id } = req.params; const memoryStorage = req.app.get('memoryStorage'); const sandbox = req.app.get('sandbox'); @@ -234,20 +234,20 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a } catch (err) { reportError(span, err); - const status = err.statusCode || 500 + const status = err.statusCode || 500; res.status(status).json({ error: err.message }); - if(err.constructor.name === "SandboxInternalException"){ + if (err.constructor.name === 'SandboxInternalException') { log.error({ method: req.method, request_uri: req.url, - status: status, - request_id: req.headers["x-request-id"] || "", + status, + request_id: req.headers['x-request-id'] || '', message: err.message, - namespace: namespace, + namespace, function_id: id, - stack: err.stack - }) + stack: err.stack, + }); } return; @@ -285,19 +285,18 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a res.status(status).json({ error: err.message }); const spent = metric.observeFunctionRun({ namespace, id, status }); - - if(err.constructor.name === "SandboxInternalException"){ + if (err.constructor.name === 'SandboxInternalException') { log.error({ method: req.method, request_uri: req.url, - status: status, - request_id: req.headers["x-request-id"] || "", + status, + request_id: req.headers['x-request-id'] || '', message: err.message, request_time: spent, - namespace: namespace, + namespace, function_id: id, - stack: err.stack - }) + stack: err.stack, + }); } const logResult = logStorage.flush({ @@ -316,7 +315,7 @@ router.all('/:namespace/:id/run', bodyParser.json({ limit: bodyParserLimit }), a code: code.code, }); errTracker.notify(err); - RecordOtelError(err) + RecordOtelError(err); // eslint-disable-line } }); @@ -375,26 +374,12 @@ router.put('/pipeline', bodyParser.json({ limit: bodyParserLimit }), async (req, res.json(result.body); } catch (err) { reportError(span, err); - RecordOtelError(err) + RecordOtelError(err); // eslint-disable-line const status = err.statusCode || 500; - if(err.constructor.name === "SandboxInternalException"){ - log.error({ - method: req.method, - request_uri: req.url, - status: status, - spent: spent, - request_id: req.headers["x-request-id"] || "", - message: err.message, - namespace: step.namespace, - function_id: step.id, - stack: err.stack - }) - } - metric.observePipelineRun(status); - const errDetails = { details: err.details ? err.details : {} } - res.status(status).json({ error: err.message, ...errDetails }); + const errDetails = { details: err.details ? err.details : {} }; + res.status(status).json({ error: err.message, ...errDetails }); } next(); }); diff --git a/lib/http/routes.js b/lib/http/routes.js index da8fae5..f0255c7 100644 --- a/lib/http/routes.js +++ b/lib/http/routes.js @@ -33,7 +33,7 @@ morgan.token('response-sectime', (req, res) => { return secs.toFixed(3); }); const app = express(); -const traceEngine = config.trace.engine +const traceEngine = config.trace.engine; app.use(morgan(config.log.morganFormat)); app.use(expressOpentracing.default({ tracer })); diff --git a/lib/support/config.js b/lib/support/config.js index e63e5b7..4a99424 100644 --- a/lib/support/config.js +++ b/lib/support/config.js @@ -11,7 +11,7 @@ const DEFAULT_GLOBAL_MODULES = [ ]; module.exports = { - host: process.env.HOSTNAME || "localhost", + host: process.env.HOSTNAME || 'localhost', port: ConfigDiscovery.getInt('PORT', 8100), metricsPort: ConfigDiscovery.getInt('METRICS_PORT', 8101), useNodeCluster: ConfigDiscovery.getBool('USE_NODE_CLUSTER', true), diff --git a/lib/support/opentelemetry.js b/lib/support/opentelemetry.js index 55ccd03..c1b2789 100644 --- a/lib/support/opentelemetry.js +++ b/lib/support/opentelemetry.js @@ -4,26 +4,27 @@ const { ExpressInstrumentation } = require('@opentelemetry/instrumentation-expre const { registerInstrumentations } = require('@opentelemetry/instrumentation'); const { CollectorTraceExporter } = require('@opentelemetry/exporter-collector-grpc'); const { NodeTracerProvider } = require('@opentelemetry/node'); -const { IORedisInstrumentation } = require('@opentelemetry/instrumentation-ioredis') -const config = require('./config') +const { IORedisInstrumentation } = require('@opentelemetry/instrumentation-ioredis'); +const config = require('./config'); const opentelemetry = require('@opentelemetry/api'); + const OtelConfig = config.trace.otel; -const HOST = config.host +const HOST = config.host; const collectorOptions = { - serviceName: OtelConfig.service, - url: `${OtelConfig.collector.host}:${OtelConfig.collector.port}`, - concurrencyLimit: 10 -} + serviceName: OtelConfig.service, + url: `${OtelConfig.collector.host}:${OtelConfig.collector.port}`, + concurrencyLimit: 10, +}; const provider = new NodeTracerProvider(); provider.resource = provider.resource.merge({ attributes: { - "service.instance.id": HOST - } -}) + 'service.instance.id': HOST, + }, +}); const exporter = new CollectorTraceExporter(collectorOptions); provider.addSpanProcessor(new BatchSpanProcessor(exporter, { @@ -39,11 +40,11 @@ registerInstrumentations({ span.updateName(`${attrs['http.method']} ${attrs['http.target']}`); span.setAttribute('functions.route', attrs['http.route']); span.setAttribute('functions.url', attrs['http.url']); - }, + }, ignoreOutgoingUrls: [/.*\/agent_listener/, /.*\/sampling/], - ignoreIncomingPaths: [/.*\/healthcheck/, /.*\/metrics/, /.*\/sampling/] + ignoreIncomingPaths: [/.*\/healthcheck/, /.*\/metrics/, /.*\/sampling/], }), - new ExpressInstrumentation() + new ExpressInstrumentation(), ], }); @@ -51,9 +52,11 @@ provider.register(); const tracer = opentelemetry.trace.getTracer(OtelConfig.service); function StartSpanMiddleware(req, res, next) { - const span = tracer.startSpan(req.path, { attributes: { - "http.request_id": req.headers["x-request-id"] - }}); + const span = tracer.startSpan(req.path, { + attributes: { + 'http.request_id': req.headers['x-request-id'], + }, + }); req.otelSpan = span; next(); } @@ -65,10 +68,10 @@ function FinalizeSpanMiddleware(req, res, next) { } function RecordOtelError(err) { - const span = tracer.startSpan('Exception Throwed') - span.recordException(err.stack) - span.setStatus({code: opentelemetry.SpanStatusCode.ERROR }) - span.end() + const span = tracer.startSpan('Exception Throwed'); + span.recordException(err.stack); + span.setStatus({ code: opentelemetry.SpanStatusCode.ERROR }); + span.end(); } module.exports = { StartSpanMiddleware, FinalizeSpanMiddleware, RecordOtelError }; diff --git a/test/unit/http/routers/FunctionsRouter.test.js b/test/unit/http/routers/FunctionsRouter.test.js index 92b6fcb..c022625 100644 --- a/test/unit/http/routers/FunctionsRouter.test.js +++ b/test/unit/http/routers/FunctionsRouter.test.js @@ -323,7 +323,7 @@ describe('PUT /functions/pipeline', () => { request(routes) .put('/functions/pipeline?steps[0]=backstage/not-found') .expect(404, { - error: 'Code \'backstage/not-found\' is not found', details: {} + error: 'Code \'backstage/not-found\' is not found', details: {}, }, done); }); }); From e768c477c928fab219f20893ad3f6f61186a1a0b Mon Sep 17 00:00:00 2001 From: Celso Inacio Date: Mon, 26 Feb 2024 14:05:49 -0300 Subject: [PATCH 2/2] fix: tests --- test/fakes/FakeStorage.js | 4 ++++ test/unit/domain/Pipeline.test.js | 3 +++ test/unit/http/routers/FunctionsRouter.test.js | 6 +++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/test/fakes/FakeStorage.js b/test/fakes/FakeStorage.js index 34920dd..8010dd0 100644 --- a/test/fakes/FakeStorage.js +++ b/test/fakes/FakeStorage.js @@ -55,6 +55,10 @@ class FakeStorage extends Storage { return { hash: 'my-hash-123', code: 'function main() {}', + env: { + MY_VAR: 'MY_VALUE', + CLIENT_SECRET: 'FOOBAR', + }, }; } diff --git a/test/unit/domain/Pipeline.test.js b/test/unit/domain/Pipeline.test.js index 35efe8c..85c515a 100644 --- a/test/unit/domain/Pipeline.test.js +++ b/test/unit/domain/Pipeline.test.js @@ -23,6 +23,9 @@ describe('Pipeline', () => { beforeEach(() => { req = { body: {}, + headers: { + 'x-request-id': 'foobar', + }, }; sandbox = new Sandbox({}); step200 = { diff --git a/test/unit/http/routers/FunctionsRouter.test.js b/test/unit/http/routers/FunctionsRouter.test.js index c022625..9cf2e87 100644 --- a/test/unit/http/routers/FunctionsRouter.test.js +++ b/test/unit/http/routers/FunctionsRouter.test.js @@ -382,15 +382,15 @@ describe('PUT /functions/:namespace/:id/env/:env', () => { }); }); - describe('when get called, not return secret variable', () => { - it('should create an enviroment variable', (done) => { + describe('when get called', () => { + it('should not return a secret variable', (done) => { request(routes) .get('/functions/backstage/correct') .set('content-type', 'application/json') .send() .expect((res) => { expect(res.body.env).to.be.eql({ - MY_VAR: 'MY VALUE', + MY_VAR: 'MY_VALUE', }); }) .expect(200, done);