From 86894a86bcd64380c74202e5763f5c9b2fae02ab Mon Sep 17 00:00:00 2001 From: Perki Date: Tue, 10 Aug 2021 18:14:27 +0200 Subject: [PATCH 1/9] testing express patching --- components/api-server/src/expressApp.js | 35 +++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/components/api-server/src/expressApp.js b/components/api-server/src/expressApp.js index 79162deed..610c6c5e4 100644 --- a/components/api-server/src/expressApp.js +++ b/components/api-server/src/expressApp.js @@ -27,6 +27,10 @@ async function expressAppInit(logging) { const version = await getAPIVersion(); const config = await getConfig(); const app = express(); // register common middleware + + patchApp(app); + + const commonHeadersMiddleware = await middleware.commonHeaders(); const requestTraceMiddleware = middleware.requestTrace(app, logging); @@ -64,4 +68,35 @@ async function expressAppInit(logging) { return app; } +function patchApp(app) { + app.unpatchedUse = app.use; + app.use = function() { + const newArgs = []; + for (let i = 0; i < arguments.length; i++) { + newArgs.push(patchFunction2(arguments[i])); + } + console.log(arguments, newArgs); + return app.unpatchedUse(...newArgs); + } + +} + +function patchFunction(fn) { + // return fn; + if (fn.constructor.name === 'AsyncFunction') { + return async function() { return await fn(...arguments); } + } + return function() { return fn(...arguments); } +} + +function patchFunction2(fn) { + // return fn; + if (fn.constructor.name === 'AsyncFunction') { + return async function(req, res, next) { return await fn(req, res, next); } + } + return function(req, res, next) { return fn(req, res, next); } +} + + + module.exports = expressAppInit; \ No newline at end of file From 226c3b76faa050c757999515829abdf4d9f3dad8 Mon Sep 17 00:00:00 2001 From: Perki Date: Tue, 10 Aug 2021 18:50:35 +0200 Subject: [PATCH 2/9] keeping patch --- components/api-server/src/expressApp.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/api-server/src/expressApp.js b/components/api-server/src/expressApp.js index 610c6c5e4..69392dfd3 100644 --- a/components/api-server/src/expressApp.js +++ b/components/api-server/src/expressApp.js @@ -73,10 +73,10 @@ function patchApp(app) { app.use = function() { const newArgs = []; for (let i = 0; i < arguments.length; i++) { - newArgs.push(patchFunction2(arguments[i])); + newArgs.push(patchFunction(arguments[i])); } console.log(arguments, newArgs); - return app.unpatchedUse(...newArgs); + return app.unpatchedUse.apply(app, newArgs); } } @@ -84,9 +84,9 @@ function patchApp(app) { function patchFunction(fn) { // return fn; if (fn.constructor.name === 'AsyncFunction') { - return async function() { return await fn(...arguments); } + return async function() { return await fn.apply(this, arguments); } } - return function() { return fn(...arguments); } + return function() { return fn.apply(this, arguments); } } function patchFunction2(fn) { From 9ee688e5ebba7e55f1321208f01e5497576d8f43 Mon Sep 17 00:00:00 2001 From: Perki Date: Wed, 11 Aug 2021 09:09:51 +0200 Subject: [PATCH 3/9] Express Tracer v0 --- components/api-server/src/expressApp.js | 36 ++------------- components/tracing/src/expressTracer.js | 61 +++++++++++++++++++++++++ components/tracing/src/index.js | 4 +- 3 files changed, 66 insertions(+), 35 deletions(-) create mode 100644 components/tracing/src/expressTracer.js diff --git a/components/api-server/src/expressApp.js b/components/api-server/src/expressApp.js index 69392dfd3..c917da43a 100644 --- a/components/api-server/src/expressApp.js +++ b/components/api-server/src/expressApp.js @@ -18,6 +18,8 @@ const { getAPIVersion } = require('middleware/src/project_version'); const { getConfig } = require('@pryv/boiler'); +const { expressTracer } = require('tracing'); + // ------------------------------------------------------------ express app init // Creates and returns an express application with a standard set of middleware. @@ -28,8 +30,7 @@ async function expressAppInit(logging) { const config = await getConfig(); const app = express(); // register common middleware - patchApp(app); - + expressTracer(app); const commonHeadersMiddleware = await middleware.commonHeaders(); const requestTraceMiddleware = middleware.requestTrace(app, logging); @@ -68,35 +69,4 @@ async function expressAppInit(logging) { return app; } -function patchApp(app) { - app.unpatchedUse = app.use; - app.use = function() { - const newArgs = []; - for (let i = 0; i < arguments.length; i++) { - newArgs.push(patchFunction(arguments[i])); - } - console.log(arguments, newArgs); - return app.unpatchedUse.apply(app, newArgs); - } - -} - -function patchFunction(fn) { - // return fn; - if (fn.constructor.name === 'AsyncFunction') { - return async function() { return await fn.apply(this, arguments); } - } - return function() { return fn.apply(this, arguments); } -} - -function patchFunction2(fn) { - // return fn; - if (fn.constructor.name === 'AsyncFunction') { - return async function(req, res, next) { return await fn(req, res, next); } - } - return function(req, res, next) { return fn(req, res, next); } -} - - - module.exports = expressAppInit; \ No newline at end of file diff --git a/components/tracing/src/expressTracer.js b/components/tracing/src/expressTracer.js new file mode 100644 index 000000000..5490f3611 --- /dev/null +++ b/components/tracing/src/expressTracer.js @@ -0,0 +1,61 @@ + +const { getConfigUnsafe } = require('@pryv/boiler'); +const isTracingEnabled = getConfigUnsafe(true).get('trace:enable'); + +module.exports = function patchApp(app) { + if (! isTracingEnabled) return; + if (app.unpatchedUse != null) throw new Error('Already patched'); + app.unpatchedUse = app.use; + app.use = function () { + const newArgs = []; + for (let i = 0; i < arguments.length; i++) { + newArgs.push(patchFunction0(arguments[i])); + } + //console.log(arguments, newArgs); + return app.unpatchedUse(...newArgs); + } + + patch('get', app); + //patch('post', app); + +} + + + +function patch(key, app) { + app['legacy_' + key] = app[key]; + app[key] = function () { + const newArgs = [arguments[0]]; + for (let i = 1; i < arguments.length; i++) { + const fn = arguments[i]; + const spanName = 'e:' + key + ':' + arguments[0] + ':' + (fn.name || ('unnamed.' + i)); + newArgs.push(patchFunction(fn, spanName)); + } + return app['legacy_' + key](...newArgs); + } +} + +function patchFunction(fn, spanName) { + return async function (req, res, next) { + function nextCloseSpan(err) { + console.log('<<' + spanName); + next(err); + } + console.log('>>' + spanName); + return await fn(req, res, nextCloseSpan); + } +} + +function patchFunction0(fn) { + return fn; +} + +// kept for reference +function patchFunction2(fn) { + // return fn; + if (fn.constructor.name === 'AsyncFunction') { + return async () => { try { return await fn.apply(null, arguments); } catch (e) { console.log('XXXX', e) } } + } + return () => { try { return fn.apply(null, arguments); } catch (e) { console.log(e) } } +} + diff --git a/components/tracing/src/index.js b/components/tracing/src/index.js index 0df9e64d9..e051f3a32 100644 --- a/components/tracing/src/index.js +++ b/components/tracing/src/index.js @@ -11,7 +11,7 @@ const { Tracing, DummyTracing } = require('./Tracing'); const { getHookerTracer } = require('./HookedTracer'); - +const expressTracer = require('./expressTracer'); const dataBaseTracer = require('./databaseTracer'); const { getConfigUnsafe } = require('@pryv/boiler'); const isTracingEnabled = getConfigUnsafe(true).get('trace:enable'); @@ -19,8 +19,8 @@ const isTracingEnabled = getConfigUnsafe(true).get('trace:enable'); module.exports.DummyTracing = DummyTracing; +module.exports.expressTracer = expressTracer; module.exports.dataBaseTracer = dataBaseTracer; - module.exports.getHookerTracer = getHookerTracer; /** From a4f363cca8c7c95ae95c209383d840b65312ece5 Mon Sep 17 00:00:00 2001 From: Perki Date: Wed, 11 Aug 2021 09:27:01 +0200 Subject: [PATCH 4/9] Refactoring express tracing --- components/api-server/src/application.js | 8 -------- components/api-server/src/expressApp.js | 4 ++-- components/tracing/src/expressTracer.js | 15 ++++++++++----- components/tracing/src/index.js | 9 +++++++-- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/components/api-server/src/application.js b/components/api-server/src/application.js index d779ca822..01b60067e 100644 --- a/components/api-server/src/application.js +++ b/components/api-server/src/application.js @@ -48,7 +48,6 @@ const UserLocalDirectory = require('business').users.UserLocalDirectory; const { Extension, ExtensionLoader } = require('utils').extension; const { getAPIVersion } = require('middleware/src/project_version'); -const { tracingMiddleware } = require('tracing'); const { pubsub } = require('messages'); logger.debug('Loading app'); @@ -121,13 +120,6 @@ class Application { await this.createExpressApp(); const apiVersion: string = await getAPIVersion(); const hostname: string = require('os').hostname(); - this.expressApp.use(tracingMiddleware( - 'express1', - { - apiVersion, - hostname, - } - )) this.initiateRoutes(); this.expressApp.use(middleware.notFound); const errorsMiddleware = errorsMiddlewareMod(this.logging); diff --git a/components/api-server/src/expressApp.js b/components/api-server/src/expressApp.js index c917da43a..d342bdd04 100644 --- a/components/api-server/src/expressApp.js +++ b/components/api-server/src/expressApp.js @@ -18,7 +18,7 @@ const { getAPIVersion } = require('middleware/src/project_version'); const { getConfig } = require('@pryv/boiler'); -const { expressTracer } = require('tracing'); +const { initExpressTracer } = require('tracing'); // ------------------------------------------------------------ express app init @@ -30,7 +30,7 @@ async function expressAppInit(logging) { const config = await getConfig(); const app = express(); // register common middleware - expressTracer(app); + initExpressTracer(app); const commonHeadersMiddleware = await middleware.commonHeaders(); const requestTraceMiddleware = middleware.requestTrace(app, logging); diff --git a/components/tracing/src/expressTracer.js b/components/tracing/src/expressTracer.js index 5490f3611..5ddeac75e 100644 --- a/components/tracing/src/expressTracer.js +++ b/components/tracing/src/expressTracer.js @@ -1,10 +1,14 @@ +/** + * @license + * Copyright (C) 2012-2021 Pryv S.A. https://pryv.com - All Rights Reserved + * Unauthorized copying of this file, via any medium is strictly prohibited + * Proprietary and confidential + */ -const { getConfigUnsafe } = require('@pryv/boiler'); -const isTracingEnabled = getConfigUnsafe(true).get('trace:enable'); module.exports = function patchApp(app) { - if (! isTracingEnabled) return; if (app.unpatchedUse != null) throw new Error('Already patched'); + app.unpatchedUse = app.use; app.use = function () { const newArgs = []; @@ -16,8 +20,9 @@ module.exports = function patchApp(app) { } patch('get', app); - //patch('post', app); - + patch('post', app); + patch('put', app); + patch('delete', app); } diff --git a/components/tracing/src/index.js b/components/tracing/src/index.js index e051f3a32..911b3c79b 100644 --- a/components/tracing/src/index.js +++ b/components/tracing/src/index.js @@ -19,7 +19,7 @@ const isTracingEnabled = getConfigUnsafe(true).get('trace:enable'); module.exports.DummyTracing = DummyTracing; -module.exports.expressTracer = expressTracer; + module.exports.dataBaseTracer = dataBaseTracer; module.exports.getHookerTracer = getHookerTracer; @@ -41,7 +41,7 @@ module.exports.initRootSpan = initRootSpan; /** * Returns an ExpressJS middleware that starts a span and attaches the "tracing" object to the request parameter. */ -module.exports.tracingMiddleware = (name: string = 'express1', tags: ?{}): Function => { +function tracingMiddleware (name: string = 'express1', tags: ?{}): Function { return function (req: express$Request, res: express$Response, next: express$NextFunction): void { if (req.tracing != null) { console.log('XXXXX tracing already set', new Error()); return next();} const tracing = initRootSpan (name, tags); @@ -53,3 +53,8 @@ module.exports.tracingMiddleware = (name: string = 'express1', tags: ?{}): Funct next(); } } + +module.exports.initExpressTracer = function(app) { + app.use(tracingMiddleware()); // anyway .. initRootSpan will retrun a dummytracer is not enabled + if (isTracingEnabled) expressTracer(app); +} \ No newline at end of file From a506b0abf532c8bdcd8083ccd40c2a060ac99b2b Mon Sep 17 00:00:00 2001 From: Perki Date: Wed, 11 Aug 2021 09:38:08 +0200 Subject: [PATCH 5/9] tracing express with some missing close --- .../src/{expressTracer.js => expressPatch.js} | 12 ++++++++---- components/tracing/src/index.js | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) rename components/tracing/src/{expressTracer.js => expressPatch.js} (87%) diff --git a/components/tracing/src/expressTracer.js b/components/tracing/src/expressPatch.js similarity index 87% rename from components/tracing/src/expressTracer.js rename to components/tracing/src/expressPatch.js index 5ddeac75e..95ec2eb31 100644 --- a/components/tracing/src/expressTracer.js +++ b/components/tracing/src/expressPatch.js @@ -33,7 +33,7 @@ function patch(key, app) { const newArgs = [arguments[0]]; for (let i = 1; i < arguments.length; i++) { const fn = arguments[i]; - const spanName = 'e:' + key + ':' + arguments[0] + ':' + (fn.name || ('unnamed.' + i)); + const spanName = 'e:' + key + ':' + arguments[0] + ':' + (fn.name || ('unamed.' + i)); newArgs.push(patchFunction(fn, spanName)); } return app['legacy_' + key](...newArgs); @@ -43,11 +43,15 @@ function patch(key, app) { function patchFunction(fn, spanName) { return async function (req, res, next) { function nextCloseSpan(err) { - console.log('<<' + spanName); + req.tracing.finishSpan(spanName) next(err); } - console.log('>>' + spanName); - return await fn(req, res, nextCloseSpan); + req.tracing.startSpan(spanName, {}, 'express1'); + try { + return await fn(req, res, nextCloseSpan); + } catch (e) { + console.log(e); + } } } diff --git a/components/tracing/src/index.js b/components/tracing/src/index.js index 911b3c79b..91217d0f7 100644 --- a/components/tracing/src/index.js +++ b/components/tracing/src/index.js @@ -11,7 +11,7 @@ const { Tracing, DummyTracing } = require('./Tracing'); const { getHookerTracer } = require('./HookedTracer'); -const expressTracer = require('./expressTracer'); +const expressPatch = require('./expressPatch'); const dataBaseTracer = require('./databaseTracer'); const { getConfigUnsafe } = require('@pryv/boiler'); const isTracingEnabled = getConfigUnsafe(true).get('trace:enable'); @@ -56,5 +56,5 @@ function tracingMiddleware (name: string = 'express1', tags: ?{}): Function { module.exports.initExpressTracer = function(app) { app.use(tracingMiddleware()); // anyway .. initRootSpan will retrun a dummytracer is not enabled - if (isTracingEnabled) expressTracer(app); + if (isTracingEnabled) expressPatch(app); } \ No newline at end of file From 7aa6ef710fa2cad745c09c056ffd2438d6c6e9be Mon Sep 17 00:00:00 2001 From: Perki Date: Wed, 11 Aug 2021 09:53:58 +0200 Subject: [PATCH 6/9] adding error tracking --- components/tracing/src/expressPatch.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/components/tracing/src/expressPatch.js b/components/tracing/src/expressPatch.js index 95ec2eb31..37d4e7425 100644 --- a/components/tracing/src/expressPatch.js +++ b/components/tracing/src/expressPatch.js @@ -43,14 +43,16 @@ function patch(key, app) { function patchFunction(fn, spanName) { return async function (req, res, next) { function nextCloseSpan(err) { - req.tracing.finishSpan(spanName) + req.tracing.finishSpan(spanName); next(err); } req.tracing.startSpan(spanName, {}, 'express1'); try { return await fn(req, res, nextCloseSpan); } catch (e) { - console.log(e); + console.log('XXXX PatchError', e); + req.tracing.finishSpan(spanName); + throw e; } } } From 5f46a584ec18d2a55e09d2f71878fe11a3641d44 Mon Sep 17 00:00:00 2001 From: Perki Date: Wed, 11 Aug 2021 09:59:49 +0200 Subject: [PATCH 7/9] Naming some anonymous functions --- .../api-server/src/routes/auth/register.js | 4 ++-- components/api-server/src/routes/events.js | 16 ++++++++-------- components/api-server/src/routes/streams.js | 2 +- components/middleware/src/commonHeaders.js | 4 ++-- components/middleware/src/contentType.js | 2 +- components/middleware/src/initContext.js | 2 +- components/middleware/src/loadAccess.js | 2 +- components/middleware/src/notFound.js | 2 +- components/middleware/src/requestTrace.js | 2 +- components/middleware/src/subdomainToPath.js | 2 +- 10 files changed, 19 insertions(+), 19 deletions(-) diff --git a/components/api-server/src/routes/auth/register.js b/components/api-server/src/routes/auth/register.js index 297b0e879..8fe3c40ba 100644 --- a/components/api-server/src/routes/auth/register.js +++ b/components/api-server/src/routes/auth/register.js @@ -47,13 +47,13 @@ module.exports = function (expressApp: express$Application, app: Application) { expressApp.get(path.join(regPath, '/:username/check_username'), setMinimalMethodContext, setMethodId('auth.usernameCheck'), - (req: express$Request, res, next) => { + function apiCall(req: express$Request, res, next) { api.call(req.context, req.params, methodCallback(res, next, 200)); }); expressApp.get(path.join(regPath, '/:email/check_email'), setMinimalMethodContext, setMethodId('auth.emailCheck'), - (req: express$Request, res, next) => { + function apiCall(req: express$Request, res, next) { api.call(req.context, req.params, methodCallback(res, next, 200)); }); expressApp.post(path.join(regPath, '/username/check'), (req: express$Request, res, next) => { diff --git a/components/api-server/src/routes/events.js b/components/api-server/src/routes/events.js index a60b765c0..16e753835 100644 --- a/components/api-server/src/routes/events.js +++ b/components/api-server/src/routes/events.js @@ -37,7 +37,7 @@ module.exports = function(expressApp: express$Application, app: Application) { expressApp.get(Paths.Events + '/', setMethodId('events.get'), loadAccessMiddleware, - function (req: express$Request, res, next) { + function coerce(req: express$Request, res, next) { const params = _.extend({}, req.query); tryCoerceStringValues(params, { fromTime: 'number', @@ -57,7 +57,7 @@ module.exports = function(expressApp: express$Application, app: Application) { expressApp.get(Paths.Events + '/:id', setMethodId('events.getOne'), loadAccessMiddleware, - function (req: express$Request, res, next) { + function coerce(req: express$Request, res, next) { const params = _.extend({id: req.params.id}, req.query); tryCoerceStringValues(params, { includeHistory: 'boolean' @@ -127,7 +127,7 @@ module.exports = function(expressApp: express$Application, app: Application) { setMethodId('events.create'), loadAccessMiddleware, hasFileUpload, - function (req: express$Request, res, next) { + function apiCall(req: express$Request, res, next) { const params = req.body; if (req.files) { params.files = req.files; @@ -143,12 +143,12 @@ module.exports = function(expressApp: express$Application, app: Application) { expressApp.put(Paths.Events + '/:id', setMethodId('events.update'), loadAccessMiddleware, - function (req: express$Request, res, next) { + function apiCall(req: express$Request, res, next) { api.call(req.context, { id: req.params.id, update: req.body }, methodCallback(res, next, 200)); }); expressApp.post(Paths.Events + '/stop', - function (req: express$Request, res, next) { + function apiCall(req: express$Request, res, next) { return next(errors.goneResource()); }); @@ -157,7 +157,7 @@ module.exports = function(expressApp: express$Application, app: Application) { setMethodId('events.update'), loadAccessMiddleware, hasFileUpload, - function (req: express$Request, res, next) { + function apiCall(req: express$Request, res, next) { const params = { id: req.params.id, update: {} @@ -173,14 +173,14 @@ module.exports = function(expressApp: express$Application, app: Application) { expressApp.delete(Paths.Events + '/:id', setMethodId('events.delete'), loadAccessMiddleware, - function (req: express$Request, res, next) { + function apiCall(req: express$Request, res, next) { api.call(req.context, {id: req.params.id}, methodCallback(res, next, 200)); }); expressApp.delete(Paths.Events + '/:id/:fileId', setMethodId('events.deleteAttachment'), loadAccessMiddleware, - function (req: express$Request, res, next) { + function apiCall(req: express$Request, res, next) { api.call(req.context, {id: req.params.id, fileId: req.params.fileId}, methodCallback(res, next, 200)); }); diff --git a/components/api-server/src/routes/streams.js b/components/api-server/src/routes/streams.js index 0a5a58b18..d37e3cfce 100644 --- a/components/api-server/src/routes/streams.js +++ b/components/api-server/src/routes/streams.js @@ -24,7 +24,7 @@ module.exports = function (expressApp: express$Application, app: Application) { expressApp.get(Paths.Streams, loadAccessMiddleware, setMethodId('streams.get'), - function (req: express$Request, res, next) { + function coerce (req: express$Request, res, next) { const params = _.extend({}, req.query); tryCoerceStringValues(params, { includeDeletionsSince: 'number' diff --git a/components/middleware/src/commonHeaders.js b/components/middleware/src/commonHeaders.js index 2cab36a54..324803d56 100644 --- a/components/middleware/src/commonHeaders.js +++ b/components/middleware/src/commonHeaders.js @@ -11,9 +11,9 @@ const { getAPIVersion } = require('middleware/src/project_version'); // Middleware to handle OPTIONS requests and to add CORS headers to all other // requests. -module.exports = async function (): Promise { +module.exports = async function (): Promise { const version = await getAPIVersion(); - return function (req: express$Request, res: express$Response, next: express$NextFunction) { + return function commonHeaders(req: express$Request, res: express$Response, next: express$NextFunction) { // allow cross-domain requests (CORS) res.header('Access-Control-Allow-Origin', req.headers.origin || '*'); // * diff --git a/components/middleware/src/contentType.js b/components/middleware/src/contentType.js index f98c3abd2..b301fadf4 100644 --- a/components/middleware/src/contentType.js +++ b/components/middleware/src/contentType.js @@ -17,7 +17,7 @@ var errors = require('errors').factory; function checkContentType(/* arguments */) { var acceptedTypes = arguments, count = acceptedTypes.length; - return function (req, res, next) { + return function checkContentType(req, res, next) { if (count < 1) { return next(); } var contentType = req.headers['content-type']; diff --git a/components/middleware/src/initContext.js b/components/middleware/src/initContext.js index db45a7954..b8d865635 100644 --- a/components/middleware/src/initContext.js +++ b/components/middleware/src/initContext.js @@ -21,7 +21,7 @@ import type { StorageLayer } from 'storage'; module.exports = function initContext( storageLayer: StorageLayer, customAuthStepFn: ?CustomAuthFunction ) { - return function ( + return function initContext( req: express$Request, res: express$Response, next: express$NextFunction ) { const authorizationHeader = req.headers['authorization']; diff --git a/components/middleware/src/loadAccess.js b/components/middleware/src/loadAccess.js index d3c30e783..fc967ad81 100644 --- a/components/middleware/src/loadAccess.js +++ b/components/middleware/src/loadAccess.js @@ -13,7 +13,7 @@ import type { StorageLayer } from 'storage'; // Also, it adds the corresponding access id as a specific response header. // module.exports = function loadAccess(storageLayer: StorageLayer) { - return async function ( + return async function loadAccess( req: express$Request, res: express$Response, next: express$NextFunction ) { diff --git a/components/middleware/src/notFound.js b/components/middleware/src/notFound.js index 5b24a03eb..4158c0bdc 100644 --- a/components/middleware/src/notFound.js +++ b/components/middleware/src/notFound.js @@ -9,6 +9,6 @@ const errors = require('errors').factory; /** * '404' handling to override Express' defaults. Must be set after the routes in the init sequence. */ -module.exports = function (req, res, next) { +module.exports = function notFound(req, res, next) { return next(errors.unknownResource()); }; diff --git a/components/middleware/src/requestTrace.js b/components/middleware/src/requestTrace.js index 33f017ff4..4a53c9817 100644 --- a/components/middleware/src/requestTrace.js +++ b/components/middleware/src/requestTrace.js @@ -10,7 +10,7 @@ const morgan = require('morgan'); const { getLogger } = require('@pryv/boiler'); -module.exports = function (express: any) { +module.exports = function requestTrace(express: any) { const logger = getLogger('request-trace'); const morganLoggerStreamWrite = (msg: string) => logger.info(msg); diff --git a/components/middleware/src/subdomainToPath.js b/components/middleware/src/subdomainToPath.js index a6744ffd7..f22f2ab99 100644 --- a/components/middleware/src/subdomainToPath.js +++ b/components/middleware/src/subdomainToPath.js @@ -21,7 +21,7 @@ const { USERNAME_REGEXP_STR } = require('api-server/src/schema/helpers'); * @return {Function} */ module.exports = function (ignoredPaths: Array) { - return function (req: express$Request, res: express$Response, next: express$NextFunction) { + return function subdomainToPath (req: express$Request, res: express$Response, next: express$NextFunction) { if (isIgnoredPath(req.url)) { return next(); } if (! req.headers.host) { return next(errors.missingHeader('Host')); } From 1b2c145d7f59635877723049c8006b4af85250cb Mon Sep 17 00:00:00 2001 From: Perki Date: Thu, 23 Sep 2021 13:35:23 +0200 Subject: [PATCH 8/9] a bit of refactoring --- .../api-server/src/routes/methodCallback.js | 2 +- components/middleware/src/setMethodId.js | 4 +- components/tracing/src/expressPatch.js | 82 +++++++++---------- components/tracing/src/index.js | 7 +- 4 files changed, 47 insertions(+), 48 deletions(-) diff --git a/components/api-server/src/routes/methodCallback.js b/components/api-server/src/routes/methodCallback.js index dcfa49177..46d4b4ca7 100644 --- a/components/api-server/src/routes/methodCallback.js +++ b/components/api-server/src/routes/methodCallback.js @@ -18,7 +18,7 @@ import type Result from '../Result'; * @returns {Function} */ module.exports = function (res: express$Response, next: express$NextFunction, successCode: number) { - return function (err: ?Error, result: ?Result) { + return function methodCallBack(err: ?Error, result: ?Result) { if (err != null) { return next(err); diff --git a/components/middleware/src/setMethodId.js b/components/middleware/src/setMethodId.js index 239f8ffb5..7099b7f6a 100644 --- a/components/middleware/src/setMethodId.js +++ b/components/middleware/src/setMethodId.js @@ -15,9 +15,9 @@ module.exports = function (methodId: string) { req: express$Request, res: express$Response, next: express$NextFunction ) { if (req.context == null) { - const tracing = initRootSpan('express2'); + const tracing = initRootSpan('expressX'); req.context = { tracing: tracing}; - res.on('finish', () => { tracing.finishSpan('express2', 'e2:' + methodId)} ) + res.on('finish', () => { tracing.finishSpan('expressX', 'e2:' + methodId)} ) } req.context.methodId = methodId; diff --git a/components/tracing/src/expressPatch.js b/components/tracing/src/expressPatch.js index 37d4e7425..ce8b81154 100644 --- a/components/tracing/src/expressPatch.js +++ b/components/tracing/src/expressPatch.js @@ -6,7 +6,7 @@ */ -module.exports = function patchApp(app) { +module.exports = function patchApp(app, expressSpanName) { if (app.unpatchedUse != null) throw new Error('Already patched'); app.unpatchedUse = app.use; @@ -19,54 +19,52 @@ module.exports = function patchApp(app) { return app.unpatchedUse(...newArgs); } - patch('get', app); - patch('post', app); - patch('put', app); - patch('delete', app); -} + patch('get'); + patch('post'); + patch('put'); + patch('delete'); - - -function patch(key, app) { - app['legacy_' + key] = app[key]; - app[key] = function () { - const newArgs = [arguments[0]]; - for (let i = 1; i < arguments.length; i++) { - const fn = arguments[i]; - const spanName = 'e:' + key + ':' + arguments[0] + ':' + (fn.name || ('unamed.' + i)); - newArgs.push(patchFunction(fn, spanName)); + function patch(key) { + app['legacy_' + key] = app[key]; + app[key] = function () { + const newArgs = [arguments[0]]; + for (let i = 1; i < arguments.length; i++) { + const fn = arguments[i]; + const spanName = 'e:' + key + ':' + arguments[0] + ':' + (fn.name || ('unamed.' + i)); + newArgs.push(patchFunction(fn, spanName)); + } + return app['legacy_' + key](...newArgs); } - return app['legacy_' + key](...newArgs); } -} -function patchFunction(fn, spanName) { - return async function (req, res, next) { - function nextCloseSpan(err) { - req.tracing.finishSpan(spanName); - next(err); - } - req.tracing.startSpan(spanName, {}, 'express1'); - try { - return await fn(req, res, nextCloseSpan); - } catch (e) { - console.log('XXXX PatchError', e); - req.tracing.finishSpan(spanName); - throw e; + function patchFunction(fn, spanName) { + return async function (req, res, next) { + function nextCloseSpan(err) { + req.tracing.finishSpan(spanName); + next(err); + } + req.tracing.startSpan(spanName, {}, expressSpanName); + try { + return await fn(req, res, nextCloseSpan); + } catch (e) { + console.log('XXXX PatchError', e); + req.tracing.finishSpan(spanName); + throw e; + } } } -} -function patchFunction0(fn) { - return fn; -} + function patchFunction0(fn) { + return fn; + } -// kept for reference -function patchFunction2(fn) { - // return fn; - if (fn.constructor.name === 'AsyncFunction') { - return async () => { try { return await fn.apply(null, arguments); } catch (e) { console.log('XXXX', e) } } + // kept for reference + function patchFunction2(fn) { + // return fn; + if (fn.constructor.name === 'AsyncFunction') { + return async () => { try { return await fn.apply(null, arguments); } catch (e) { console.log('XXXX', e) } } + } + return () => { try { return fn.apply(null, arguments); } catch (e) { console.log(e) } } } - return () => { try { return fn.apply(null, arguments); } catch (e) { console.log(e) } } -} +} \ No newline at end of file diff --git a/components/tracing/src/index.js b/components/tracing/src/index.js index 91217d0f7..95f07c41a 100644 --- a/components/tracing/src/index.js +++ b/components/tracing/src/index.js @@ -41,7 +41,7 @@ module.exports.initRootSpan = initRootSpan; /** * Returns an ExpressJS middleware that starts a span and attaches the "tracing" object to the request parameter. */ -function tracingMiddleware (name: string = 'express1', tags: ?{}): Function { +function tracingMiddleware (name: string, tags: ?{}): Function { return function (req: express$Request, res: express$Response, next: express$NextFunction): void { if (req.tracing != null) { console.log('XXXXX tracing already set', new Error()); return next();} const tracing = initRootSpan (name, tags); @@ -55,6 +55,7 @@ function tracingMiddleware (name: string = 'express1', tags: ?{}): Function { } module.exports.initExpressTracer = function(app) { - app.use(tracingMiddleware()); // anyway .. initRootSpan will retrun a dummytracer is not enabled - if (isTracingEnabled) expressPatch(app); + const expressTraceName = 'express2'; + app.use(tracingMiddleware(expressTraceName)); // anyway .. initRootSpan will retrun a dummytracer is not enabled + if (isTracingEnabled) expressPatch(app, expressTraceName); } \ No newline at end of file From ce5e9418424a05b58b3db67565f2143e6ffbb86a Mon Sep 17 00:00:00 2001 From: Perki Date: Thu, 23 Sep 2021 14:16:43 +0200 Subject: [PATCH 9/9] a bit of refactoring --- components/tracing/src/expressPatch.js | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/components/tracing/src/expressPatch.js b/components/tracing/src/expressPatch.js index ce8b81154..a03087dbd 100644 --- a/components/tracing/src/expressPatch.js +++ b/components/tracing/src/expressPatch.js @@ -7,16 +7,17 @@ module.exports = function patchApp(app, expressSpanName) { - if (app.unpatchedUse != null) throw new Error('Already patched'); + if (app.legacy_use != null) throw new Error('Already patched'); - app.unpatchedUse = app.use; + app.legacy_use = app.use; app.use = function () { const newArgs = []; for (let i = 0; i < arguments.length; i++) { + // !!!! doing nothing for now.. all attemp to patch "use" failed newArgs.push(patchFunction0(arguments[i])); } //console.log(arguments, newArgs); - return app.unpatchedUse(...newArgs); + return app.legacy_use(...newArgs); } patch('get'); @@ -39,22 +40,17 @@ module.exports = function patchApp(app, expressSpanName) { function patchFunction(fn, spanName) { return async function (req, res, next) { - function nextCloseSpan(err) { - req.tracing.finishSpan(spanName); - next(err); - } req.tracing.startSpan(spanName, {}, expressSpanName); - try { - return await fn(req, res, nextCloseSpan); - } catch (e) { - console.log('XXXX PatchError', e); + return await fn(req, res, function nextCloseSpan(err) { req.tracing.finishSpan(spanName); - throw e; - } + next(err); + }); } } + // doing nothing function patchFunction0(fn) { + console.log('>>>> unpatched: ', fn.name); return fn; }