From 6e4c8a6639df96023119766905e6af14d7cad717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 10:23:11 +0200 Subject: [PATCH 01/27] Follow Node.js callback pattern --- lib/api/middlewares/user.js | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/api/middlewares/user.js b/lib/api/middlewares/user.js index cd3fc85c..b64203d1 100644 --- a/lib/api/middlewares/user.js +++ b/lib/api/middlewares/user.js @@ -8,11 +8,12 @@ module.exports = function user (metadataBackend) { return function userMiddleware (req, res, next) { res.locals.user = getUserNameFromRequest(req, cdbRequest); - getUserId(metadataBackend, res.locals.user, function (userId) { - if (userId) { - res.locals.userId = userId; + metadataBackend.getUserId(res.locals.user, (err, userId) => { + if (err || !userId) { + return next(); } - return next(); + + res.locals.userId = userId; }); }; }; @@ -20,12 +21,3 @@ module.exports = function user (metadataBackend) { function getUserNameFromRequest (req, cdbRequest) { return cdbRequest.userByReq(req); } - -function getUserId (metadataBackend, userName, callback) { - metadataBackend.getUserId(userName, function (err, userId) { - if (err) { - return callback(); - } - return callback(userId); - }); -} From c31e3d6e3ff0efd7e7f29fb69d1ddf305a2df656 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 10:58:37 +0200 Subject: [PATCH 02/27] Consistent interface when returning no event for eventa data in metrics --- lib/api/middlewares/pubsub-metrics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index e12708b5..c397dc2f 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -25,7 +25,7 @@ function getEventData (req, res) { const eventGroupId = normalizedField(req.get('Carto-Event-Group-Id')); if (!event || !eventSource) { - return [undefined, undefined]; + return { event: undefined, attributes: undefined }; } const attributes = { From 3cec6b5a900e99575a41d18f32cc1726d73e9721 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 11:06:09 +0200 Subject: [PATCH 03/27] Missing callback --- lib/api/middlewares/user.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/api/middlewares/user.js b/lib/api/middlewares/user.js index b64203d1..57c9b45d 100644 --- a/lib/api/middlewares/user.js +++ b/lib/api/middlewares/user.js @@ -14,6 +14,7 @@ module.exports = function user (metadataBackend) { } res.locals.userId = userId; + return next(); }); }; }; From 42dc2915ea6e5bf61c54457c2bc694f29e7144b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 11:41:37 +0200 Subject: [PATCH 04/27] Send pubsub metrics once the response has finished --- lib/api/middlewares/pubsub-metrics.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index c397dc2f..4dbdee00 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -5,15 +5,19 @@ const MAX_LENGTH = 100; function pubSubMetrics (pubSubMetricsBackend) { if (!pubSubMetricsBackend.isEnabled()) { - return function pubSubMetricsDisabledMiddleware (req, res, next) { next(); }; + return function pubSubMetricsDisabledMiddleware (req, res, next) { + next(); + }; } return function pubSubMetricsMiddleware (req, res, next) { - const data = getEventData(req, res); + res.on('finish', () => { + const { event, attributes } = getEventData(req, res); - if (data.event) { - pubSubMetricsBackend.sendEvent(data.event, data.attributes); - } + if (event) { + pubSubMetricsBackend.sendEvent(event, attributes); + } + }); return next(); }; From 7d6a64d3835e49695b26e0080bfe0437abab79a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 11:59:36 +0200 Subject: [PATCH 05/27] Do not expose functions just to be able to mock them while testing --- lib/backends/pubsub-metrics.js | 24 +++--------------------- test/integration/pubsub-metrics-test.js | 2 +- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/lib/backends/pubsub-metrics.js b/lib/backends/pubsub-metrics.js index bc0fb0b7..fb410e52 100644 --- a/lib/backends/pubsub-metrics.js +++ b/lib/backends/pubsub-metrics.js @@ -2,34 +2,18 @@ const { PubSub } = require('@google-cloud/pubsub'); -/** - * PubSubMetricsBackend - */ -class PubSubMetricsBackend { +module.exports = class PubSubMetricsBackend { static build () { if (!global.environment.pubSubMetrics || !global.environment.pubSubMetrics.enabled) { return new PubSubMetricsBackend(undefined, false); } - const pubsub = PubSubMetricsBackend.createPubSub(); + const { project_id: projectId, credentials: keyFilename } = global.environment.pubSubMetrics; + const pubsub = new PubSub({ projectId, keyFilename }); return new PubSubMetricsBackend(pubsub, true); } - static createPubSub () { - const projectId = global.environment.pubSubMetrics.project_id; - const credentials = global.environment.pubSubMetrics.credentials; - const config = {}; - - if (projectId) { - config.projectId = projectId; - } - if (credentials) { - config.keyFilename = credentials; - } - return new PubSub(config); - } - constructor (pubSub, enabled) { this.pubsub = pubSub; this.enabled = enabled; @@ -62,5 +46,3 @@ class PubSubMetricsBackend { }); } } - -module.exports = PubSubMetricsBackend; diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/pubsub-metrics-test.js index 6bb8110d..8e215f91 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/pubsub-metrics-test.js @@ -55,7 +55,7 @@ const fakePubSub = { topic: () => fakeTopic }; -describe('pubsub metrics middleware', function () { +describe.skip('pubsub metrics middleware', function () { let redisClient; let testClient; let clock; From 6a2333be648cccae044a87edf67df0fd7c25cfe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 12:13:54 +0200 Subject: [PATCH 06/27] Topic name's lifetime is longer than pubsub backend, we can keep it as property. --- lib/backends/pubsub-metrics.js | 20 +++++++------------- test/unit/backends/pubsub-metrics-test.js | 2 +- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/lib/backends/pubsub-metrics.js b/lib/backends/pubsub-metrics.js index fb410e52..f6565a9b 100644 --- a/lib/backends/pubsub-metrics.js +++ b/lib/backends/pubsub-metrics.js @@ -8,14 +8,15 @@ module.exports = class PubSubMetricsBackend { return new PubSubMetricsBackend(undefined, false); } - const { project_id: projectId, credentials: keyFilename } = global.environment.pubSubMetrics; + const { project_id: projectId, credentials: keyFilename, topic } = global.environment.pubSubMetrics; const pubsub = new PubSub({ projectId, keyFilename }); - return new PubSubMetricsBackend(pubsub, true); + return new PubSubMetricsBackend(pubsub, topic, true); } - constructor (pubSub, enabled) { + constructor (pubSub, topic, enabled) { this.pubsub = pubSub; + this.topic = topic; this.enabled = enabled; } @@ -23,26 +24,19 @@ module.exports = class PubSubMetricsBackend { return this.enabled; } - _getTopic () { - const topicName = global.environment.pubSubMetrics.topic; - - return this.pubsub.topic(topicName); - } - sendEvent (event, attributes) { if (!this.enabled) { return; } const data = Buffer.from(event); - const topic = this._getTopic(); - topic.publish(data, attributes) + this.pubsub.topic(this.topic).publish(data, attributes) .then(() => { - console.log(`PubSubTracker: event '${event}' published to '${topic.name}'`); + console.log(`PubSubTracker: event '${event}' published to '${this.topic}'`); }) .catch((error) => { console.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`); }); } -} +}; diff --git a/test/unit/backends/pubsub-metrics-test.js b/test/unit/backends/pubsub-metrics-test.js index c08a93e5..3660718b 100644 --- a/test/unit/backends/pubsub-metrics-test.js +++ b/test/unit/backends/pubsub-metrics-test.js @@ -23,7 +23,7 @@ const eventAttributes = { event_version: '1' }; -describe('pubsub metrics backend', function () { +describe.skip('pubsub metrics backend', function () { it('should not send event if not enabled', function () { const pubSubMetricsService = new PubSubMetricsBackend(fakePubSub, false); From e90c1965989767232b9b0431e72ec9cb7990df79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 12:46:27 +0200 Subject: [PATCH 07/27] Simplified metrics middleware and backend --- lib/api/api-router.js | 7 ++++-- lib/api/middlewares/pubsub-metrics.js | 6 ++--- lib/backends/pubsub-metrics.js | 33 ++++++--------------------- lib/server-options.js | 3 ++- lib/server.js | 1 + 5 files changed, 18 insertions(+), 32 deletions(-) diff --git a/lib/api/api-router.js b/lib/api/api-router.js index 7bf7da88..699fc0a9 100644 --- a/lib/api/api-router.js +++ b/lib/api/api-router.js @@ -187,7 +187,7 @@ module.exports = class ApiRouter { this.mapRouter = new MapRouter({ collaborators }); this.templateRouter = new TemplateRouter({ collaborators }); this.metadataBackend = metadataBackend; - this.pubSubMetricsBackend = PubSubMetricsBackend.build(); + this.pubSubMetricsBackend = new PubSubMetricsBackend(this.serverOptions.pubSubMetrics); } route (app, routes) { @@ -220,7 +220,10 @@ module.exports = class ApiRouter { apiRouter.use(sendResponse()); apiRouter.use(syntaxError()); apiRouter.use(errorMiddleware()); - apiRouter.use(pubSubMetrics(this.pubSubMetricsBackend)); + apiRouter.use(pubSubMetrics({ + enabled: this.serverOptions.pubSubMetrics.enabled, + metricsBackend: this.pubSubMetricsBackend + })); paths.forEach(path => app.use(path, apiRouter)); }); diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index 4dbdee00..bae6171a 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -3,8 +3,8 @@ const EVENT_VERSION = '1'; const MAX_LENGTH = 100; -function pubSubMetrics (pubSubMetricsBackend) { - if (!pubSubMetricsBackend.isEnabled()) { +function pubSubMetrics ({ enabled, metricsBackend }) { + if (!enabled) { return function pubSubMetricsDisabledMiddleware (req, res, next) { next(); }; @@ -15,7 +15,7 @@ function pubSubMetrics (pubSubMetricsBackend) { const { event, attributes } = getEventData(req, res); if (event) { - pubSubMetricsBackend.sendEvent(event, attributes); + metricsBackend.send(event, attributes); } }); diff --git a/lib/backends/pubsub-metrics.js b/lib/backends/pubsub-metrics.js index f6565a9b..9cf0ee2d 100644 --- a/lib/backends/pubsub-metrics.js +++ b/lib/backends/pubsub-metrics.js @@ -3,37 +3,18 @@ const { PubSub } = require('@google-cloud/pubsub'); module.exports = class PubSubMetricsBackend { - static build () { - if (!global.environment.pubSubMetrics || !global.environment.pubSubMetrics.enabled) { - return new PubSubMetricsBackend(undefined, false); - } - - const { project_id: projectId, credentials: keyFilename, topic } = global.environment.pubSubMetrics; - const pubsub = new PubSub({ projectId, keyFilename }); - - return new PubSubMetricsBackend(pubsub, topic, true); + constructor (options = {}) { + const { project_id: projectId, credentials: keyFilename, topic } = options; + this._pubsub = new PubSub({ projectId, keyFilename }); + this._topicName = topic; } - constructor (pubSub, topic, enabled) { - this.pubsub = pubSub; - this.topic = topic; - this.enabled = enabled; - } - - isEnabled () { - return this.enabled; - } - - sendEvent (event, attributes) { - if (!this.enabled) { - return; - } - + send (event, attributes) { const data = Buffer.from(event); - this.pubsub.topic(this.topic).publish(data, attributes) + this._pubsub.topic(this._topicName).publish(data, attributes) .then(() => { - console.log(`PubSubTracker: event '${event}' published to '${this.topic}'`); + console.log(`PubSubTracker: event '${event}' published to '${this._topicName}'`); }) .catch((error) => { console.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`); diff --git a/lib/server-options.js b/lib/server-options.js index c607d98b..8b3ced76 100644 --- a/lib/server-options.js +++ b/lib/server-options.js @@ -134,5 +134,6 @@ module.exports = { fastly: global.environment.fastly || {}, cache_enabled: global.environment.cache_enabled, log_format: global.environment.log_format, - useProfiler: global.environment.useProfiler + useProfiler: global.environment.useProfiler, + pubSubMetrics: Object.assign({ enabled: false }, global.environment.pubSubMetrics) }; diff --git a/lib/server.js b/lib/server.js index be030631..671045c9 100644 --- a/lib/server.js +++ b/lib/server.js @@ -21,6 +21,7 @@ module.exports = function createServer (serverOptions) { app.disable('etag'); app.set('json replacer', jsonReplacer()); + // FIXME: do not pass 'global.environment' as 'serverOptions' should keep defaults from 'global.environment' const apiRouter = new ApiRouter({ serverOptions, environmentOptions: global.environment }); apiRouter.route(app, serverOptions.routes.api); From 1bbde4f5e3c1f36c360a25840d4dba0e09c7bd6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 13:27:05 +0200 Subject: [PATCH 08/27] Let to the caller to choose how to handle the call to a method --- lib/api/middlewares/pubsub-metrics.js | 6 ++++-- lib/backends/pubsub-metrics.js | 10 ++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index bae6171a..ae0195d4 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -3,7 +3,7 @@ const EVENT_VERSION = '1'; const MAX_LENGTH = 100; -function pubSubMetrics ({ enabled, metricsBackend }) { +function pubSubMetrics ({ enabled, metricsBackend, logger }) { if (!enabled) { return function pubSubMetricsDisabledMiddleware (req, res, next) { next(); @@ -15,7 +15,9 @@ function pubSubMetrics ({ enabled, metricsBackend }) { const { event, attributes } = getEventData(req, res); if (event) { - metricsBackend.send(event, attributes); + metricsBackend.send(event, attributes) + .then(() => logger.debug(`PubSubTracker: event '${event}' published succesfully`)) + .catch((error) => logger.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`)); } }); diff --git a/lib/backends/pubsub-metrics.js b/lib/backends/pubsub-metrics.js index 9cf0ee2d..e4b37643 100644 --- a/lib/backends/pubsub-metrics.js +++ b/lib/backends/pubsub-metrics.js @@ -5,19 +5,13 @@ const { PubSub } = require('@google-cloud/pubsub'); module.exports = class PubSubMetricsBackend { constructor (options = {}) { const { project_id: projectId, credentials: keyFilename, topic } = options; + this._pubsub = new PubSub({ projectId, keyFilename }); this._topicName = topic; } send (event, attributes) { const data = Buffer.from(event); - - this._pubsub.topic(this._topicName).publish(data, attributes) - .then(() => { - console.log(`PubSubTracker: event '${event}' published to '${this._topicName}'`); - }) - .catch((error) => { - console.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`); - }); + return this._pubsub.topic(this._topicName).publish(data, attributes); } }; From fe9610abe96ee81c9f09467ba75d9607ec22e536 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 13:35:07 +0200 Subject: [PATCH 09/27] Missing logger argument --- lib/api/api-router.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/api/api-router.js b/lib/api/api-router.js index 699fc0a9..3923a2bb 100644 --- a/lib/api/api-router.js +++ b/lib/api/api-router.js @@ -222,7 +222,8 @@ module.exports = class ApiRouter { apiRouter.use(errorMiddleware()); apiRouter.use(pubSubMetrics({ enabled: this.serverOptions.pubSubMetrics.enabled, - metricsBackend: this.pubSubMetricsBackend + metricsBackend: this.pubSubMetricsBackend, + logger: global.logger })); paths.forEach(path => app.use(path, apiRouter)); From c5cb2ea4cb5bbde18c244a713ac39d9bc0f39827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 13:35:19 +0200 Subject: [PATCH 10/27] Add FIXME comment --- lib/api/middlewares/pubsub-metrics.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index ae0195d4..8ef538a8 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -64,6 +64,7 @@ function normalizedField (field) { return field.toString().trim().substr(0, MAX_LENGTH); } +// FIXME: 'X-Tiler-Profiler' might not be accurate enough function getResponseTime (res) { const profiler = res.get('X-Tiler-Profiler'); let stats; From 89e349146dc175bd1a32fe65d6832b2198f51de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 18:02:06 +0200 Subject: [PATCH 11/27] Fix tests and stop using sinon as a dev dependency --- package-lock.json | 154 ---------------- package.json | 1 - test/integration/pubsub-metrics-test.js | 206 ++++++++++------------ test/support/test-client.js | 7 +- test/unit/backends/pubsub-metrics-test.js | 40 ----- 5 files changed, 102 insertions(+), 306 deletions(-) delete mode 100644 test/unit/backends/pubsub-metrics-test.js diff --git a/package-lock.json b/package-lock.json index 80c09e41..b776d336 100644 --- a/package-lock.json +++ b/package-lock.json @@ -331,64 +331,6 @@ "resolved": "https://registry.npmjs.org/@protobufjs/utf8/-/utf8-1.1.0.tgz", "integrity": "sha1-p3c2C1s5oaLlEG+OhY8v0tBgxXA=" }, - "@sinonjs/commons": { - "version": "1.7.1", - "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.7.1.tgz", - "integrity": "sha512-Debi3Baff1Qu1Unc3mjJ96MgpbwTn43S1+9yJ0llWygPwDNu2aaWBD6yc9y/Z8XDRNhx7U+u2UDg2OGQXkclUQ==", - "dev": true, - "requires": { - "type-detect": "4.0.8" - } - }, - "@sinonjs/fake-timers": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-6.0.0.tgz", - "integrity": "sha512-atR1J/jRXvQAb47gfzSK8zavXy7BcpnYq21ALon0U99etu99vsir0trzIO3wpeLtW+LLVY6X7EkfVTbjGSH8Ww==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.7.0" - } - }, - "@sinonjs/formatio": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-5.0.0.tgz", - "integrity": "sha512-ejFRrFNMaTAmhg9u1lYKJQxDocowta6KQKFnBE7XtZb/AAPlLkWQQSaqwlGYnDWQ6paXzyM1vbMhLAujSFiVPw==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1", - "@sinonjs/samsam": "^4.2.0" - }, - "dependencies": { - "@sinonjs/samsam": { - "version": "4.2.2", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-4.2.2.tgz", - "integrity": "sha512-z9o4LZUzSD9Hl22zV38aXNykgFeVj8acqfFabCY6FY83n/6s/XwNJyYYldz6/9lBJanpno9h+oL6HTISkviweA==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.6.0", - "lodash.get": "^4.4.2", - "type-detect": "^4.0.8" - } - } - } - }, - "@sinonjs/samsam": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-5.0.1.tgz", - "integrity": "sha512-iSZdE68szyFvV8ReYve6t4gAA1rLVwGyyhWBg9qrz8VAn1FH141gdg0NJcMrAJ069rD2XM2KQzY8ZNDgmTfBQA==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.6.0", - "lodash.get": "^4.4.2", - "type-detect": "^4.0.8" - } - }, - "@sinonjs/text-encoding": { - "version": "0.7.1", - "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.1.tgz", - "integrity": "sha512-+iTbntw2IZPb/anVDbypzfQa+ay64MW0Zo8aJ8gZPWMMK6/OubMVb6lUPMagqjOPnmtauXnFCACVl3O7ogjeqQ==", - "dev": true - }, "@types/duplexify": { "version": "3.6.0", "resolved": "https://registry.npmjs.org/@types/duplexify/-/duplexify-3.6.0.tgz", @@ -4078,12 +4020,6 @@ "verror": "1.10.0" } }, - "just-extend": { - "version": "4.0.2", - "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.0.2.tgz", - "integrity": "sha512-FrLwOgm+iXrPV+5zDU6Jqu4gCRXbWEQg2O3SKONsWE4w7AXFRkryS53bpWdaL9cNol+AmR3AEYz6kn+o0fCPnw==", - "dev": true - }, "jwa": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/jwa/-/jwa-2.0.0.tgz", @@ -4175,12 +4111,6 @@ "integrity": "sha1-+wMJF/hqMTTlvJvsDWngAT3f7bI=", "dev": true }, - "lodash.get": { - "version": "4.4.2", - "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz", - "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=", - "dev": true - }, "lodash.has": { "version": "4.5.2", "resolved": "https://registry.npmjs.org/lodash.has/-/lodash.has-4.5.2.tgz", @@ -4549,52 +4479,6 @@ "integrity": "sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==", "dev": true }, - "nise": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/nise/-/nise-4.0.1.tgz", - "integrity": "sha512-10PKL272rqg80o2RsWcTT6X9cDYqJ4kXqPTf8yCXPc9hbphZSDmbiG5FqUNeR5nouKCQMM24ld45kgYnBdx2rw==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.7.0", - "@sinonjs/fake-timers": "^6.0.0", - "@sinonjs/formatio": "^4.0.1", - "@sinonjs/text-encoding": "^0.7.1", - "just-extend": "^4.0.2", - "path-to-regexp": "^1.7.0" - }, - "dependencies": { - "@sinonjs/formatio": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/@sinonjs/formatio/-/formatio-4.0.1.tgz", - "integrity": "sha512-asIdlLFrla/WZybhm0C8eEzaDNNrzymiTqHMeJl6zPW2881l3uuVRpm0QlRQEjqYWv6CcKMGYME3LbrLJsORBw==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1", - "@sinonjs/samsam": "^4.2.0" - } - }, - "@sinonjs/samsam": { - "version": "4.2.2", - "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-4.2.2.tgz", - "integrity": "sha512-z9o4LZUzSD9Hl22zV38aXNykgFeVj8acqfFabCY6FY83n/6s/XwNJyYYldz6/9lBJanpno9h+oL6HTISkviweA==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.6.0", - "lodash.get": "^4.4.2", - "type-detect": "^4.0.8" - } - }, - "path-to-regexp": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz", - "integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==", - "dev": true, - "requires": { - "isarray": "0.0.1" - } - } - } - }, "nock": { "version": "9.2.6", "resolved": "https://registry.npmjs.org/nock/-/nock-9.2.6.tgz", @@ -5811,44 +5695,6 @@ "resolved": "https://registry.npmjs.org/simple-statistics/-/simple-statistics-0.9.2.tgz", "integrity": "sha1-PjXLEDCPx2ljqk7nJS5qbqENKOQ=" }, - "sinon": { - "version": "9.0.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.0.0.tgz", - "integrity": "sha512-c4bREcvuK5VuEGyMW/Oim9I3Rq49Vzb0aMdxouFaA44QCFpilc5LJOugrX+mkrvikbqCimxuK+4cnHVNnLR41g==", - "dev": true, - "requires": { - "@sinonjs/commons": "^1.7.0", - "@sinonjs/fake-timers": "^6.0.0", - "@sinonjs/formatio": "^5.0.0", - "@sinonjs/samsam": "^5.0.1", - "diff": "^4.0.2", - "nise": "^4.0.1", - "supports-color": "^7.1.0" - }, - "dependencies": { - "diff": { - "version": "4.0.2", - "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", - "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", - "dev": true - }, - "has-flag": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-4.0.0.tgz", - "integrity": "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ==", - "dev": true - }, - "supports-color": { - "version": "7.1.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-7.1.0.tgz", - "integrity": "sha512-oRSIpR8pxT1Wr2FquTNnGet79b3BWljqOuoW/h4oBhxJ/HUbX5nX6JSruTkvXDCFMwDPvsaTTbvMLKZWSy0R5g==", - "dev": true, - "requires": { - "has-flag": "^4.0.0" - } - } - } - }, "slice-ansi": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/slice-ansi/-/slice-ansi-2.1.0.tgz", diff --git a/package.json b/package.json index 3ee5f8c1..f062d445 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,6 @@ "nock": "9.2.6", "nyc": "^14.1.1", "redis": "2.8.0", - "sinon": "^9.0.0", "step": "1.0.0", "strftime": "0.10.0" }, diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/pubsub-metrics-test.js index 8e215f91..aea595c0 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/pubsub-metrics-test.js @@ -1,166 +1,156 @@ 'use strict'; -const sinon = require('sinon'); const assert = require('assert'); -const redis = require('redis'); const TestClient = require('../support/test-client'); const PubSubMetricsBackend = require('../../lib/backends/pubsub-metrics'); -const metricsHeaders = { - 'Carto-Event': 'test-event', - 'Carto-Event-Source': 'test', - 'Carto-Event-Group-Id': '1' -}; - -const tooLongField = ' If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean ' + - 'this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; - -const badHeaders = { - 'Carto-Event': tooLongField, - 'Carto-Event-Source': 'test', - 'Carto-Event-Group-Id': 1 -}; - const mapConfig = { - version: '1.7.0', + version: '1.8.0', layers: [ { options: { - sql: 'select * FROM test_table_localhost_regular1', + sql: TestClient.SQL.ONE_POINT, cartocss: TestClient.CARTOCSS.POINTS, cartocss_version: '2.3.0' } } ] }; +const apikey = 1234; -function buildEventAttributes (statusCode) { - return { - event_source: 'test', - user_id: '1', - event_group_id: '1', - response_code: statusCode.toString(), - source_domain: 'localhost', - event_time: new Date().toISOString(), - event_version: '1' - }; -} - -const fakeTopic = { - name: 'test-topic', - publish: sinon.stub().returns(Promise.resolve()) -}; - -const fakePubSub = { - topic: () => fakeTopic -}; - -describe.skip('pubsub metrics middleware', function () { - let redisClient; - let testClient; - let clock; - - before(function () { - redisClient = redis.createClient(global.environment.redis.port); - clock = sinon.useFakeTimers(); - sinon.stub(PubSubMetricsBackend, 'createPubSub').returns(fakePubSub); +describe('pubsub metrics middleware', function () { + beforeEach(function () { + this.originalPubSubMetricsBackendSendMethod = PubSubMetricsBackend.prototype.send; + this.pubSubMetricsBackendSendMethodCalled = false; + PubSubMetricsBackend.prototype.send = (event, attributes) => { + this.pubSubMetricsBackendSendMethodCalled = true; + this.pubSubMetricsBackendSendMethodCalledWith = { event, attributes }; + return Promise.resolve(); + }; }); - after(function () { - clock.restore(); - PubSubMetricsBackend.createPubSub.restore(); - global.environment.pubSubMetrics.enabled = false; - }); - - afterEach(function (done) { - fakeTopic.publish.resetHistory(); - - redisClient.SELECT(0, () => { - redisClient.del('user:localhost:mapviews:global'); - - redisClient.SELECT(5, () => { - redisClient.del('user:localhost:mapviews:global'); - done(); - }); - }); + afterEach(function () { + PubSubMetricsBackend.prototype.send = this.originalPubSubMetricsBackendSendMethod; }); it('should not send event if not enabled', function (done) { - global.environment.pubSubMetrics.enabled = false; - testClient = new TestClient(mapConfig, 1234, metricsHeaders); + const extraHeaders = { + 'Carto-Event': 'test-event', + 'Carto-Event-Source': 'test', + 'Carto-Event-Group-Id': '1' + }; + const overrideServerOptions = { pubSubMetrics: { enabled: false } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); testClient.getLayergroup((err, body) => { if (err) { return done(err); } - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.notCalled); - return done(); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(!this.pubSubMetricsBackendSendMethodCalled); + + return testClient.drain(done); }); }); it('should not send event if headers not present', function (done) { - global.environment.pubSubMetrics.enabled = true; - testClient = new TestClient(mapConfig, 1234); + const extraHeaders = {}; + const overrideServerOptions = { pubSubMetrics: { enabled: false } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); testClient.getLayergroup((err, body) => { if (err) { return done(err); } - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.notCalled); - return done(); - }); - }); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - it('should normalized headers type and length', function (done) { - global.environment.pubSubMetrics.enabled = true; - const eventAttributes = buildEventAttributes(200); - const maxLength = 100; - const eventName = tooLongField.trim().substr(0, maxLength); - - testClient = new TestClient(mapConfig, 1234, badHeaders); - - testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } - - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.calledOnceWith(Buffer.from(eventName), eventAttributes)); - return done(); + return testClient.drain(done); }); }); it('should send event for map requests', function (done) { - global.environment.pubSubMetrics.enabled = true; - const eventAttributes = buildEventAttributes(200); - testClient = new TestClient(mapConfig, 1234, metricsHeaders); + const expectedEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); testClient.getLayergroup((err, body) => { if (err) { return done(err); } - assert.strictEqual(typeof body.metadata, 'object'); - assert(fakeTopic.publish.calledOnceWith(Buffer.from('test-event'), eventAttributes)); - return done(); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + + return testClient.drain(done); + }); + }); + + it('should normalized headers type and length', function (done) { + const eventLong = 'If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; + const expectedEvent = eventLong.trim().substr(0, 100); + const expectedEventGroupId = '1'; + const expectedEventSource = 'test'; + const extraHeaders = { + 'Carto-Event': eventLong, + 'Carto-Event-Source': 'test', + 'Carto-Event-Group-Id': 1 + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + + return testClient.drain(done); }); }); it('should send event when error', function (done) { - global.environment.pubSubMetrics.enabled = true; - const eventAttributes = buildEventAttributes(400); - eventAttributes.user_id = undefined; + const expectedEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const emptyMapConfig = {}; + const testClient = new TestClient(emptyMapConfig, apikey, extraHeaders, overrideServerOptions); + const params = { response: { status: 400 } }; - testClient = new TestClient({}, 1234, metricsHeaders); + testClient.getLayergroup(params, (err, body) => { + if (err) { + return done(err); + } - testClient.getLayergroup(() => { - assert(fakeTopic.publish.calledOnceWith(Buffer.from('test-event'), eventAttributes)); - assert(fakeTopic.publish.calledOnce); - return done(); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + + return testClient.drain(done); }); }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index f0798dad..cdb59eaf 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -23,13 +23,14 @@ const MAPNIK_SUPPORTED_FORMATS = { mvt: true }; -function TestClient (config, apiKey, extraHeaders) { +function TestClient (config, apiKey, extraHeaders = {}, overrideServerOptions = {}) { this.mapConfig = isMapConfig(config) ? config : null; this.template = isTemplate(config) ? config : null; this.apiKey = apiKey; - this.extraHeaders = extraHeaders || {}; + this.extraHeaders = extraHeaders; this.keysToDelete = {}; - this.server = new CartodbWindshaft(serverOptions); + this.serverOptions = Object.assign({}, serverOptions, overrideServerOptions); + this.server = new CartodbWindshaft(this.serverOptions); } module.exports = TestClient; diff --git a/test/unit/backends/pubsub-metrics-test.js b/test/unit/backends/pubsub-metrics-test.js deleted file mode 100644 index 3660718b..00000000 --- a/test/unit/backends/pubsub-metrics-test.js +++ /dev/null @@ -1,40 +0,0 @@ -'use strict'; - -const sinon = require('sinon'); -const assert = require('assert'); -const PubSubMetricsBackend = require('../../../lib/backends/pubsub-metrics'); - -const fakeTopic = { - name: 'test-topic', - publish: sinon.stub().returns(Promise.resolve()) -}; - -const fakePubSub = { - topic: () => fakeTopic -}; - -const eventAttributes = { - event_source: 'test', - user_id: '123', - event_group_id: '1', - response_code: '200', - source_domain: 'localhost', - event_time: new Date().toISOString(), - event_version: '1' -}; - -describe.skip('pubsub metrics backend', function () { - it('should not send event if not enabled', function () { - const pubSubMetricsService = new PubSubMetricsBackend(fakePubSub, false); - - pubSubMetricsService.sendEvent('test-event', eventAttributes); - assert(fakeTopic.publish.notCalled); - }); - - it('should send event if enabled', function () { - const pubSubMetricsService = new PubSubMetricsBackend(fakePubSub, true); - - pubSubMetricsService.sendEvent('test-event', eventAttributes); - assert(fakeTopic.publish.calledOnceWith(Buffer.from('test-event'), eventAttributes)); - }); -}); From 7f5ed58a7919e6b46c4ab56a389fba762788a5fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 27 Apr 2020 18:40:28 +0200 Subject: [PATCH 12/27] Add test --- test/integration/pubsub-metrics-test.js | 119 +++++++++++++++++++++++- 1 file changed, 117 insertions(+), 2 deletions(-) diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/pubsub-metrics-test.js index aea595c0..553d157f 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/pubsub-metrics-test.js @@ -3,7 +3,7 @@ const assert = require('assert'); const TestClient = require('../support/test-client'); const PubSubMetricsBackend = require('../../lib/backends/pubsub-metrics'); - +const apikey = 1234; const mapConfig = { version: '1.8.0', layers: [ @@ -16,7 +16,23 @@ const mapConfig = { } ] }; -const apikey = 1234; +const template = { + version: '0.0.1', + name: 'metrics-template', + layergroup: { + version: '1.8.0', + layers: [ + { + type: 'cartodb', + options: { + sql: TestClient.SQL.ONE_POINT, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] + } +}; describe('pubsub metrics middleware', function () { beforeEach(function () { @@ -75,6 +91,7 @@ describe('pubsub metrics middleware', function () { const expectedEvent = 'event-test'; const expectedEventSource = 'event-source-test'; const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; const extraHeaders = { 'Carto-Event': expectedEvent, 'Carto-Event-Source': expectedEventSource, @@ -93,6 +110,7 @@ describe('pubsub metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); return testClient.drain(done); }); @@ -103,6 +121,7 @@ describe('pubsub metrics middleware', function () { const expectedEvent = eventLong.trim().substr(0, 100); const expectedEventGroupId = '1'; const expectedEventSource = 'test'; + const expectedResponseCode = '200'; const extraHeaders = { 'Carto-Event': eventLong, 'Carto-Event-Source': 'test', @@ -121,6 +140,7 @@ describe('pubsub metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); return testClient.drain(done); }); @@ -130,6 +150,7 @@ describe('pubsub metrics middleware', function () { const expectedEvent = 'event-test'; const expectedEventSource = 'event-source-test'; const expectedEventGroupId = '1'; + const expectedResponseCode = '400'; const extraHeaders = { 'Carto-Event': expectedEvent, 'Carto-Event-Source': expectedEventSource, @@ -149,6 +170,100 @@ describe('pubsub metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + + return testClient.drain(done); + }); + }); + + it('should send event for tile requests', function (done) { + const expectedEvent = 'event-tile-test'; + const expectedEventSource = 'event-source-tile-test'; + const expectedEventGroupId = '12345'; + const expectedResponseCode = '200'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + testClient.getTile(0, 0, 0, (err, res, tile) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + + return testClient.drain(done); + }); + }); + + it('should send event for errored tile requests', function (done) { + const expectedEvent = 'event-tile-test'; + const expectedEventSource = 'event-source-tile-test'; + const expectedEventGroupId = '12345'; + const expectedResponseCode = '400'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + testClient.getTile(0, 0, 2, params, (err, res, tile) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + + return testClient.drain(done); + }); + }); + + it('should send event for named map tile requests', function (done) { + const expectedEvent = 'event-named-map-tile-test'; + const expectedEventSource = 'event-source-named-map-tile-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + testClient.getTile(0, 0, 0, (err, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); return testClient.drain(done); }); From c88a14bf43f801145e0cf1963fc8b5dff8ee8cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 28 Apr 2020 19:17:00 +0200 Subject: [PATCH 13/27] Send metrics for map instantiations (named, anonymous and static) with the new format. --- lib/api/api-router.js | 15 +- lib/api/map/anonymous-map-controller.js | 25 ++- lib/api/map/map-router.js | 12 +- lib/api/map/preview-template-controller.js | 23 +- lib/api/middlewares/pubsub-metrics.js | 105 ++++++--- lib/api/template/named-template-controller.js | 25 ++- lib/api/template/template-router.js | 8 +- test/integration/pubsub-metrics-test.js | 200 +++++++++++++++--- test/support/test-client.js | 184 +++++++++++++--- 9 files changed, 492 insertions(+), 105 deletions(-) diff --git a/lib/api/api-router.js b/lib/api/api-router.js index 3923a2bb..479d25ed 100644 --- a/lib/api/api-router.js +++ b/lib/api/api-router.js @@ -57,7 +57,6 @@ const user = require('./middlewares/user'); const sendResponse = require('./middlewares/send-response'); const syntaxError = require('./middlewares/syntax-error'); const errorMiddleware = require('./middlewares/error-middleware'); -const pubSubMetrics = require('./middlewares/pubsub-metrics'); const MapRouter = require('./map/map-router'); const TemplateRouter = require('./template/template-router'); @@ -161,7 +160,10 @@ module.exports = class ApiRouter { }); namedMapProviderCacheReporter.start(); + const metricsBackend = new PubSubMetricsBackend(serverOptions.pubSubMetrics); + const collaborators = { + config: serverOptions, analysisStatusBackend, attributesBackend, dataviewBackend, @@ -181,13 +183,13 @@ module.exports = class ApiRouter { layergroupMetadata, namedMapProviderCache, tablesExtentBackend, - clusterBackend + clusterBackend, + metricsBackend }; + this.metadataBackend = metadataBackend; this.mapRouter = new MapRouter({ collaborators }); this.templateRouter = new TemplateRouter({ collaborators }); - this.metadataBackend = metadataBackend; - this.pubSubMetricsBackend = new PubSubMetricsBackend(this.serverOptions.pubSubMetrics); } route (app, routes) { @@ -220,11 +222,6 @@ module.exports = class ApiRouter { apiRouter.use(sendResponse()); apiRouter.use(syntaxError()); apiRouter.use(errorMiddleware()); - apiRouter.use(pubSubMetrics({ - enabled: this.serverOptions.pubSubMetrics.enabled, - metricsBackend: this.pubSubMetricsBackend, - logger: global.logger - })); paths.forEach(path => app.use(path, apiRouter)); }); diff --git a/lib/api/map/anonymous-map-controller.js b/lib/api/map/anonymous-map-controller.js index 508e1cd8..356ef338 100644 --- a/lib/api/map/anonymous-map-controller.js +++ b/lib/api/map/anonymous-map-controller.js @@ -23,6 +23,7 @@ const mapError = require('../middlewares/map-error'); const CreateLayergroupMapConfigProvider = require('../../models/mapconfig/provider/create-layergroup-provider'); const rateLimit = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const metrics = require('../middlewares/pubsub-metrics'); module.exports = class AnonymousMapController { /** @@ -39,6 +40,7 @@ module.exports = class AnonymousMapController { * @constructor */ constructor ( + config, pgConnection, templateMaps, mapBackend, @@ -49,8 +51,10 @@ module.exports = class AnonymousMapController { mapConfigAdapter, statsBackend, authBackend, - layergroupMetadata + layergroupMetadata, + metricsBackend ) { + this.config = config; this.pgConnection = pgConnection; this.templateMaps = templateMaps; this.mapBackend = mapBackend; @@ -62,6 +66,7 @@ module.exports = class AnonymousMapController { this.statsBackend = statsBackend; this.authBackend = authBackend; this.layergroupMetadata = layergroupMetadata; + this.metricsBackend = metricsBackend; } route (mapRouter) { @@ -76,8 +81,26 @@ module.exports = class AnonymousMapController { const includeQuery = true; const label = 'ANONYMOUS LAYERGROUP'; const addContext = true; + const metricsTags = { + event: 'map_view', + attributes: { map_type: 'anonymous' }, + from: { + req: { + query: { client: 'client' } + }, + res: { + body: { layergroupid: 'map_id' } + } + } + }; return [ + metrics({ + enabled: this.config.pubSubMetrics.enabled, + metricsBackend: this.metricsBackend, + logger: global.logger, + tags: metricsTags + }), credentials(), authorize(this.authBackend), dbConnSetup(this.pgConnection), diff --git a/lib/api/map/map-router.js b/lib/api/map/map-router.js index c5070627..2ebccb54 100644 --- a/lib/api/map/map-router.js +++ b/lib/api/map/map-router.js @@ -15,6 +15,7 @@ const ClusteredFeaturesLayergroupController = require('./clustered-features-laye module.exports = class MapRouter { constructor ({ collaborators }) { const { + config, analysisStatusBackend, attributesBackend, dataviewBackend, @@ -34,7 +35,8 @@ module.exports = class MapRouter { layergroupMetadata, namedMapProviderCache, tablesExtentBackend, - clusterBackend + clusterBackend, + metricsBackend } = collaborators; this.analysisLayergroupController = new AnalysisLayergroupController( @@ -85,6 +87,7 @@ module.exports = class MapRouter { ); this.anonymousMapController = new AnonymousMapController( + config, pgConnection, templateMaps, mapBackend, @@ -95,10 +98,12 @@ module.exports = class MapRouter { mapConfigAdapter, statsBackend, authBackend, - layergroupMetadata + layergroupMetadata, + metricsBackend ); this.previewTemplateController = new PreviewTemplateController( + config, namedMapProviderCache, previewBackend, surrogateKeysCache, @@ -106,7 +111,8 @@ module.exports = class MapRouter { metadataBackend, pgConnection, authBackend, - userLimitsBackend + userLimitsBackend, + metricsBackend ); this.analysesController = new AnalysesCatalogController( diff --git a/lib/api/map/preview-template-controller.js b/lib/api/map/preview-template-controller.js index 989bb8e1..832dfacb 100644 --- a/lib/api/map/preview-template-controller.js +++ b/lib/api/map/preview-template-controller.js @@ -12,6 +12,7 @@ const lastModifiedHeader = require('../middlewares/last-modified-header'); const checkStaticImageFormat = require('../middlewares/check-static-image-format'); const rateLimit = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const metrics = require('../middlewares/pubsub-metrics'); const DEFAULT_ZOOM_CENTER = { zoom: 1, @@ -27,6 +28,7 @@ function numMapper (n) { module.exports = class PreviewTemplateController { constructor ( + config, namedMapProviderCache, previewBackend, surrogateKeysCache, @@ -34,8 +36,10 @@ module.exports = class PreviewTemplateController { metadataBackend, pgConnection, authBackend, - userLimitsBackend + userLimitsBackend, + metricsBackend ) { + this.config = config; this.namedMapProviderCache = namedMapProviderCache; this.previewBackend = previewBackend; this.surrogateKeysCache = surrogateKeysCache; @@ -44,6 +48,7 @@ module.exports = class PreviewTemplateController { this.pgConnection = pgConnection; this.authBackend = authBackend; this.userLimitsBackend = userLimitsBackend; + this.metricsBackend = metricsBackend; } route (mapRouter) { @@ -51,7 +56,23 @@ module.exports = class PreviewTemplateController { } middlewares () { + const metricsTags = { + event: 'map_view', + attributes: { map_type: 'static' }, + from: { + req: { + query: { client: 'client' } + } + } + }; + return [ + metrics({ + enabled: this.config.pubSubMetrics.enabled, + metricsBackend: this.metricsBackend, + logger: global.logger, + tags: metricsTags + }), credentials(), authorize(this.authBackend), dbConnSetup(this.pgConnection), diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index 8ef538a8..ecd77644 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -3,55 +3,58 @@ const EVENT_VERSION = '1'; const MAX_LENGTH = 100; -function pubSubMetrics ({ enabled, metricsBackend, logger }) { +module.exports = function pubSubMetrics ({ enabled, metricsBackend, logger, tags }) { if (!enabled) { return function pubSubMetricsDisabledMiddleware (req, res, next) { next(); }; } + if (!tags || !tags.event) { + throw new Error('Missing required "event" parameter to report metrics'); + } + return function pubSubMetricsMiddleware (req, res, next) { res.on('finish', () => { - const { event, attributes } = getEventData(req, res); + const { event, attributes } = getEventData(req, res, tags); - if (event) { - metricsBackend.send(event, attributes) - .then(() => logger.debug(`PubSubTracker: event '${event}' published succesfully`)) - .catch((error) => logger.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`)); - } + metricsBackend.send(event, attributes) + .then(() => logger.debug(`PubSubTracker: event '${event}' published succesfully`)) + .catch((error) => logger.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`)); }); return next(); }; -} +}; -function getEventData (req, res) { - const event = normalizedField(req.get('Carto-Event')); - const eventSource = normalizedField(req.get('Carto-Event-Source')); - const eventGroupId = normalizedField(req.get('Carto-Event-Group-Id')); +function getEventData (req, res, tags) { + const event = tags.event; + const extra = {}; + if (tags.from) { + if (tags.from.req) { + Object.assign(extra, getFromReq(req, tags.from.req)); + } - if (!event || !eventSource) { - return { event: undefined, attributes: undefined }; + if (tags.from.res) { + Object.assign(extra, getFromRes(res, tags.from.res)); + } } - const attributes = { - event_source: eventSource, - user_id: res.locals.userId, - response_code: res.statusCode.toString(), - source_domain: req.hostname, + const attributes = Object.assign({}, { + metrics_event: normalizedField(req.get('Carto-Event')), + event_source: normalizedField(req.get('Carto-Event-Source')), + event_group_id: normalizedField(req.get('Carto-Event-Group-Id')), event_time: new Date().toISOString(), + user_id: res.locals.userId, + user_agent: req.get('User-Agent'), + response_code: res.statusCode.toString(), + response_time: getResponseTime(res), + source_domain: req.hostname, event_version: EVENT_VERSION - }; + }, tags.attributes, extra); - if (eventGroupId) { - attributes.event_group_id = eventGroupId; - } - - const responseTime = getResponseTime(res); - - if (responseTime) { - attributes.response_time = responseTime.toString(); - } + // remove undefined properties + Object.keys(attributes).forEach(key => attributes[key] === undefined && delete attributes[key]); return { event, attributes }; } @@ -75,7 +78,47 @@ function getResponseTime (res) { return undefined; } - return stats.total; + return stats.total.toString(); } -module.exports = pubSubMetrics; +function getFromReq (req, { query = {}, body = {}, params = {}, headers = {} } = {}) { + const extra = {}; + + for (const [queryParam, eventName] of Object.entries(query)) { + extra[eventName] = req.query[queryParam]; + } + + for (const [bodyParam, eventName] of Object.entries(body)) { + extra[eventName] = req.body[bodyParam]; + } + + for (const [pathParam, eventName] of Object.entries(params)) { + extra[eventName] = req.params[pathParam]; + } + + for (const [header, eventName] of Object.entries(headers)) { + extra[eventName] = req.get(header); + } + + return extra; +} + +function getFromRes (res, { body = {}, headers = {}, locals = {} } = {}) { + const extra = {}; + + if (res.body) { + for (const [bodyParam, eventName] of Object.entries(body)) { + extra[eventName] = res.body[bodyParam]; + } + } + + for (const [header, eventName] of Object.entries(headers)) { + extra[eventName] = res.get(header); + } + + for (const [localParam, eventName] of Object.entries(locals)) { + extra[eventName] = res.locals[localParam]; + } + + return extra; +} diff --git a/lib/api/template/named-template-controller.js b/lib/api/template/named-template-controller.js index b7b3a865..e8891455 100644 --- a/lib/api/template/named-template-controller.js +++ b/lib/api/template/named-template-controller.js @@ -21,6 +21,7 @@ const NamedMapMapConfigProvider = require('../../models/mapconfig/provider/named const CreateLayergroupMapConfigProvider = require('../../models/mapconfig/provider/create-layergroup-provider'); const rateLimit = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; +const metrics = require('../middlewares/pubsub-metrics'); module.exports = class NamedMapController { /** @@ -38,6 +39,7 @@ module.exports = class NamedMapController { * @constructor */ constructor ( + config, pgConnection, templateMaps, mapBackend, @@ -48,8 +50,10 @@ module.exports = class NamedMapController { mapConfigAdapter, statsBackend, authBackend, - layergroupMetadata + layergroupMetadata, + metricsBackend ) { + this.config = config; this.pgConnection = pgConnection; this.templateMaps = templateMaps; this.mapBackend = mapBackend; @@ -61,6 +65,7 @@ module.exports = class NamedMapController { this.statsBackend = statsBackend; this.authBackend = authBackend; this.layergroupMetadata = layergroupMetadata; + this.metricsBackend = metricsBackend; } route (templateRouter) { @@ -74,8 +79,26 @@ module.exports = class NamedMapController { const includeQuery = false; const label = 'NAMED MAP LAYERGROUP'; const addContext = false; + const metricsTags = { + event: 'map_view', + attributes: { map_type: 'named' }, + from: { + req: { + query: { client: 'client' } + }, + res: { + body: { layergroupid: 'map_id' } + } + } + }; return [ + metrics({ + enabled: this.config.pubSubMetrics.enabled, + metricsBackend: this.metricsBackend, + logger: global.logger, + tags: metricsTags + }), credentials(), authorize(this.authBackend), dbConnSetup(this.pgConnection), diff --git a/lib/api/template/template-router.js b/lib/api/template/template-router.js index 81d7973f..7b254f9c 100644 --- a/lib/api/template/template-router.js +++ b/lib/api/template/template-router.js @@ -9,6 +9,7 @@ const TileTemplateController = require('./tile-template-controller'); module.exports = class TemplateRouter { constructor ({ collaborators }) { const { + config, pgConnection, templateMaps, mapBackend, @@ -21,10 +22,12 @@ module.exports = class TemplateRouter { authBackend, layergroupMetadata, namedMapProviderCache, - tileBackend + tileBackend, + metricsBackend } = collaborators; this.namedMapController = new NamedMapController( + config, pgConnection, templateMaps, mapBackend, @@ -35,7 +38,8 @@ module.exports = class TemplateRouter { mapConfigAdapter, statsBackend, authBackend, - layergroupMetadata + layergroupMetadata, + metricsBackend ); this.tileTemplateController = new TileTemplateController( diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/pubsub-metrics-test.js index 553d157f..1ded5193 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/pubsub-metrics-test.js @@ -16,22 +16,25 @@ const mapConfig = { } ] }; -const template = { - version: '0.0.1', - name: 'metrics-template', - layergroup: { - version: '1.8.0', - layers: [ - { - type: 'cartodb', - options: { - sql: TestClient.SQL.ONE_POINT, - cartocss: TestClient.CARTOCSS.POINTS, - cartocss_version: '2.3.0' + +function templateBuilder ({ name }) { + return { + version: '0.0.1', + name: `metrics-template-${name}`, + layergroup: { + version: '1.8.0', + layers: [ + { + type: 'cartodb', + options: { + sql: TestClient.SQL.ONE_POINT, + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } } - } - ] - } + ] + } + }; }; describe('pubsub metrics middleware', function () { @@ -88,12 +91,14 @@ describe('pubsub metrics middleware', function () { }); it('should send event for map requests', function (done) { - const expectedEvent = 'event-test'; + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; const expectedEventSource = 'event-source-test'; const expectedEventGroupId = '1'; const expectedResponseCode = '200'; + const expectedMapType = 'anonymous'; const extraHeaders = { - 'Carto-Event': expectedEvent, + 'Carto-Event': expectedMetricsEvent, 'Carto-Event-Source': expectedEventSource, 'Carto-Event-Group-Id': expectedEventGroupId }; @@ -108,20 +113,24 @@ describe('pubsub metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); return testClient.drain(done); }); }); it('should normalized headers type and length', function (done) { + const expectedEvent = 'map_view'; const eventLong = 'If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; - const expectedEvent = eventLong.trim().substr(0, 100); + const expectedMetricsEvent = eventLong.trim().substr(0, 100); const expectedEventGroupId = '1'; const expectedEventSource = 'test'; const expectedResponseCode = '200'; + const expectedMapType = 'anonymous'; const extraHeaders = { 'Carto-Event': eventLong, 'Carto-Event-Source': 'test', @@ -138,27 +147,41 @@ describe('pubsub metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); return testClient.drain(done); }); }); it('should send event when error', function (done) { - const expectedEvent = 'event-test'; + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; const expectedEventSource = 'event-source-test'; const expectedEventGroupId = '1'; const expectedResponseCode = '400'; + const expectedMapType = 'anonymous'; const extraHeaders = { - 'Carto-Event': expectedEvent, + 'Carto-Event': expectedMetricsEvent, 'Carto-Event-Source': expectedEventSource, 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const emptyMapConfig = {}; - const testClient = new TestClient(emptyMapConfig, apikey, extraHeaders, overrideServerOptions); + const mapConfigMissingCartoCSS = { + version: '1.8.0', + layers: [ + { + options: { + sql: TestClient.SQL.ONE_POINT, + cartocss: TestClient.CARTOCSS.POINTS + } + } + ] + }; + const testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); const params = { response: { status: 400 } }; testClient.getLayergroup(params, (err, body) => { @@ -168,15 +191,17 @@ describe('pubsub metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); return testClient.drain(done); }); }); - it('should send event for tile requests', function (done) { + it.skip('should send event for tile requests', function (done) { const expectedEvent = 'event-tile-test'; const expectedEventSource = 'event-source-tile-test'; const expectedEventGroupId = '12345'; @@ -204,7 +229,7 @@ describe('pubsub metrics middleware', function () { }); }); - it('should send event for errored tile requests', function (done) { + it.skip('should send event for errored tile requests', function (done) { const expectedEvent = 'event-tile-test'; const expectedEventSource = 'event-source-tile-test'; const expectedEventGroupId = '12345'; @@ -241,7 +266,98 @@ describe('pubsub metrics middleware', function () { }); }); - it('should send event for named map tile requests', function (done) { + it('should send event for named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const expectedMapType = 'named'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'map' }); + const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + + return testClient.drain(done); + }); + }); + + it('should send event for errored named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '400'; + const expectedMapType = 'named'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const templateMissingCartoCSS = { + version: '0.0.1', + name: 'metrics-template', + layergroup: { + version: '1.8.0', + layers: [ + { + type: 'cartodb', + options: { + sql: TestClient.SQL.ONE_POINT, + cartocss: TestClient.CARTOCSS.POINTS + } + } + ] + } + }; + + const testClient = new TestClient(templateMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + testClient.getLayergroup(params, (err, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + + return testClient.drain(done); + }); + }); + + it.skip('should send event for named map tile requests', function (done) { const expectedEvent = 'event-named-map-tile-test'; const expectedEventSource = 'event-source-named-map-tile-test'; const expectedEventGroupId = '1'; @@ -252,6 +368,7 @@ describe('pubsub metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'tile' }); const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); testClient.getTile(0, 0, 0, (err, body) => { @@ -268,4 +385,37 @@ describe('pubsub metrics middleware', function () { return testClient.drain(done); }); }); + + it('should send event for static named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const expectedMapType = 'static'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'preview' }); + const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + testClient.getPreview(640, 480, {}, (err, res, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + + return testClient.drain(done); + }); + }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index cdb59eaf..0639d402 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -942,16 +942,8 @@ TestClient.prototype.getLayergroup = function (params, callback) { params = {}; } - if (!params.response) { - params.response = { - status: 200, - headers: { - 'Content-Type': 'application/json; charset=utf-8' - } - }; - } - - var url = '/api/v1/map'; + let url = '/api/v1/map'; + const urlNamed = url + '/named'; const headers = Object.assign({ host: 'localhost', 'Content-Type': 'application/json' }, self.extraHeaders); const queryParams = {}; @@ -968,30 +960,109 @@ TestClient.prototype.getLayergroup = function (params, callback) { url += '?' + qs.stringify(queryParams); } - assert.response(self.server, - { - url: url, - method: 'POST', - headers, - data: JSON.stringify(self.mapConfig) - }, - params.response, - function (res, err) { - var parsedBody; - // If there is a response, we are still interested in catching the created keys - // to be able to delete them on the .drain() method. - if (res) { - parsedBody = JSON.parse(res.body); - if (parsedBody.layergroupid) { - self.keysToDelete['map_cfg|' + LayergroupToken.parse(parsedBody.layergroupid).token] = 0; - self.keysToDelete['user:localhost:mapviews:global'] = 5; - } - } - if (err) { - return callback(err); + var layergroupId; + + if (params.layergroupid) { + layergroupId = params.layergroupid; + } + + step( + function createTemplate () { + var next = this; + + if (!self.template) { + return next(); } - return callback(null, parsedBody); + if (!self.apiKey) { + return next(new Error('apiKey param is mandatory to create a new template')); + } + + params.placeholders = params.placeholders || {}; + + assert.response(self.server, + { + url: urlNamed + '?' + qs.stringify({ api_key: self.apiKey }), + method: 'POST', + headers, + data: JSON.stringify(self.template) + }, + { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }, + function (res, err) { + if (err) { + return next(err); + } + return next(null, JSON.parse(res.body).template_id); + } + ); + }, + function createLayergroup (err, templateId) { + var next = this; + + if (err) { + return next(err); + } + + if (layergroupId) { + return next(null, layergroupId); + } + + const data = templateId ? params.placeholders : self.mapConfig; + + if (!params.response) { + params.response = { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + }; + } + + const queryParams = {}; + + if (self.apiKey) { + queryParams.api_key = self.apiKey; + } + + if (params.aggregation !== undefined) { + queryParams.aggregation = params.aggregation; + } + + const path = templateId + ? urlNamed + '/' + templateId + '?' + qs.stringify(queryParams) + : url; + + assert.response(self.server, + { + url: path, + method: 'POST', + headers, + data: JSON.stringify(data) + }, + params.response, + function (res, err) { + var parsedBody; + // If there is a response, we are still interested in catching the created keys + // to be able to delete them on the .drain() method. + if (res) { + parsedBody = JSON.parse(res.body); + if (parsedBody.layergroupid) { + self.keysToDelete['map_cfg|' + LayergroupToken.parse(parsedBody.layergroupid).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; + } + } + if (err) { + return callback(err); + } + + return callback(null, parsedBody); + } + ); } ); }; @@ -1687,3 +1758,52 @@ TestClient.prototype.getTemplate = function (params, callback) { return callback(err, res, body); }); }; + +TestClient.prototype.getPreview = function (width, height, params = {}, callback) { + this.createTemplate({}, (err, res, template) => { + if (err) { + return callback(err); + } + + params = Object.assign({ api_key: this.apiKey }, params); + const url = `/api/v1/map/static/named/${template.template_id}/${width}/${height}.png?${qs.stringify(params)}`; + const headers = Object.assign({ host: 'localhost' }, this.extraHeaders); + + const requestOptions = { + url: url, + method: 'GET', + headers, + encoding: 'binary' + }; + + const expectedResponse = Object.assign({ + status: 200, + headers: { + 'Content-Type': 'image/png' + } + }, params.response || {}); + + assert.response(this.server, requestOptions, expectedResponse, (res, err) => { + if (err) { + return callback(err); + } + + this.keysToDelete['user:localhost:mapviews:global'] = 5; + + let body; + switch (res.headers['content-type']) { + case 'image/png': + body = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary')); + break; + case 'application/json; charset=utf-8': + body = JSON.parse(res.body); + break; + default: + body = res.body; + break; + } + + return callback(null, res, body); + }); + }); +}; From 4e3ef963744a37f023b70a95d1cf98be6f0cba34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 10:28:10 +0200 Subject: [PATCH 14/27] Add test to chek we still send events when errored map static tile --- lib/api/middlewares/pubsub-metrics.js | 2 +- test/integration/pubsub-metrics-test.js | 41 +++++++++++++++++++++++++ test/support/test-client.js | 3 +- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/pubsub-metrics.js index ecd77644..5e9b1df2 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/pubsub-metrics.js @@ -78,7 +78,7 @@ function getResponseTime (res) { return undefined; } - return stats.total.toString(); + return stats && stats.total ? stats.total.toString() : undefined; } function getFromReq (req, { query = {}, body = {}, params = {}, headers = {} } = {}) { diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/pubsub-metrics-test.js index 1ded5193..4a4c7272 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/pubsub-metrics-test.js @@ -418,4 +418,45 @@ describe('pubsub metrics middleware', function () { return testClient.drain(done); }); }); + + it('should send event for errored static named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '400'; + const expectedMapType = 'static'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'preview-errored' }); + const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + const widthTooLarge = 8193; + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + + return testClient.drain(done); + }); + }); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index 0639d402..542855f9 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1788,11 +1788,10 @@ TestClient.prototype.getPreview = function (width, height, params = {}, callback return callback(err); } - this.keysToDelete['user:localhost:mapviews:global'] = 5; - let body; switch (res.headers['content-type']) { case 'image/png': + this.keysToDelete['user:localhost:mapviews:global'] = 5; body = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary')); break; case 'application/json; charset=utf-8': From 70f0b6ea508376e6d4a5c9f0801244d7f5eef3dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 10:40:45 +0200 Subject: [PATCH 15/27] Avoid to use "pubsub" for the name of modules, middlewares, variables, etc.. --- lib/api/api-router.js | 2 +- lib/api/map/anonymous-map-controller.js | 2 +- lib/api/map/preview-template-controller.js | 2 +- lib/api/middlewares/{pubsub-metrics.js => metrics.js} | 10 +++++----- lib/api/template/named-template-controller.js | 2 +- lib/backends/{pubsub-metrics.js => metrics.js} | 6 +++--- .../{pubsub-metrics-test.js => metrics-test.js} | 10 +++++----- 7 files changed, 17 insertions(+), 17 deletions(-) rename lib/api/middlewares/{pubsub-metrics.js => metrics.js} (87%) rename lib/backends/{pubsub-metrics.js => metrics.js} (61%) rename test/integration/{pubsub-metrics-test.js => metrics-test.js} (98%) diff --git a/lib/api/api-router.js b/lib/api/api-router.js index 479d25ed..ab2bf332 100644 --- a/lib/api/api-router.js +++ b/lib/api/api-router.js @@ -21,7 +21,7 @@ const OverviewsMetadataBackend = require('../backends/overviews-metadata'); const FilterStatsApi = require('../backends/filter-stats'); const TablesExtentBackend = require('../backends/tables-extent'); const ClusterBackend = require('../backends/cluster'); -const PubSubMetricsBackend = require('../backends/pubsub-metrics'); +const PubSubMetricsBackend = require('../backends/metrics'); const LayergroupAffectedTablesCache = require('../cache/layergroup-affected-tables'); const SurrogateKeysCache = require('../cache/surrogate-keys-cache'); diff --git a/lib/api/map/anonymous-map-controller.js b/lib/api/map/anonymous-map-controller.js index 356ef338..135da65f 100644 --- a/lib/api/map/anonymous-map-controller.js +++ b/lib/api/map/anonymous-map-controller.js @@ -23,7 +23,7 @@ const mapError = require('../middlewares/map-error'); const CreateLayergroupMapConfigProvider = require('../../models/mapconfig/provider/create-layergroup-provider'); const rateLimit = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; -const metrics = require('../middlewares/pubsub-metrics'); +const metrics = require('../middlewares/metrics'); module.exports = class AnonymousMapController { /** diff --git a/lib/api/map/preview-template-controller.js b/lib/api/map/preview-template-controller.js index 832dfacb..d9a64707 100644 --- a/lib/api/map/preview-template-controller.js +++ b/lib/api/map/preview-template-controller.js @@ -12,7 +12,7 @@ const lastModifiedHeader = require('../middlewares/last-modified-header'); const checkStaticImageFormat = require('../middlewares/check-static-image-format'); const rateLimit = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; -const metrics = require('../middlewares/pubsub-metrics'); +const metrics = require('../middlewares/metrics'); const DEFAULT_ZOOM_CENTER = { zoom: 1, diff --git a/lib/api/middlewares/pubsub-metrics.js b/lib/api/middlewares/metrics.js similarity index 87% rename from lib/api/middlewares/pubsub-metrics.js rename to lib/api/middlewares/metrics.js index 5e9b1df2..82924f47 100644 --- a/lib/api/middlewares/pubsub-metrics.js +++ b/lib/api/middlewares/metrics.js @@ -3,9 +3,9 @@ const EVENT_VERSION = '1'; const MAX_LENGTH = 100; -module.exports = function pubSubMetrics ({ enabled, metricsBackend, logger, tags }) { +module.exports = function metrics ({ enabled, metricsBackend, logger, tags }) { if (!enabled) { - return function pubSubMetricsDisabledMiddleware (req, res, next) { + return function metricsDisabledMiddleware (req, res, next) { next(); }; } @@ -14,13 +14,13 @@ module.exports = function pubSubMetrics ({ enabled, metricsBackend, logger, tags throw new Error('Missing required "event" parameter to report metrics'); } - return function pubSubMetricsMiddleware (req, res, next) { + return function metricsMiddleware (req, res, next) { res.on('finish', () => { const { event, attributes } = getEventData(req, res, tags); metricsBackend.send(event, attributes) - .then(() => logger.debug(`PubSubTracker: event '${event}' published succesfully`)) - .catch((error) => logger.error(`ERROR: pubsub middleware failed to publish event '${event}': ${error.message}`)); + .then(() => logger.debug(`Event "${event}" published succesfully`)) + .catch((error) => logger.error(`Failed to publish event "${event}": ${error.message}`)); }); return next(); diff --git a/lib/api/template/named-template-controller.js b/lib/api/template/named-template-controller.js index e8891455..b1fd7348 100644 --- a/lib/api/template/named-template-controller.js +++ b/lib/api/template/named-template-controller.js @@ -21,7 +21,7 @@ const NamedMapMapConfigProvider = require('../../models/mapconfig/provider/named const CreateLayergroupMapConfigProvider = require('../../models/mapconfig/provider/create-layergroup-provider'); const rateLimit = require('../middlewares/rate-limit'); const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimit; -const metrics = require('../middlewares/pubsub-metrics'); +const metrics = require('../middlewares/metrics'); module.exports = class NamedMapController { /** diff --git a/lib/backends/pubsub-metrics.js b/lib/backends/metrics.js similarity index 61% rename from lib/backends/pubsub-metrics.js rename to lib/backends/metrics.js index e4b37643..ba91b4e6 100644 --- a/lib/backends/pubsub-metrics.js +++ b/lib/backends/metrics.js @@ -2,16 +2,16 @@ const { PubSub } = require('@google-cloud/pubsub'); -module.exports = class PubSubMetricsBackend { +module.exports = class MetricsBackend { constructor (options = {}) { const { project_id: projectId, credentials: keyFilename, topic } = options; - this._pubsub = new PubSub({ projectId, keyFilename }); + this._metricsClient = new PubSub({ projectId, keyFilename }); this._topicName = topic; } send (event, attributes) { const data = Buffer.from(event); - return this._pubsub.topic(this._topicName).publish(data, attributes); + return this._metricsClient.topic(this._topicName).publish(data, attributes); } }; diff --git a/test/integration/pubsub-metrics-test.js b/test/integration/metrics-test.js similarity index 98% rename from test/integration/pubsub-metrics-test.js rename to test/integration/metrics-test.js index 4a4c7272..147224aa 100644 --- a/test/integration/pubsub-metrics-test.js +++ b/test/integration/metrics-test.js @@ -2,7 +2,7 @@ const assert = require('assert'); const TestClient = require('../support/test-client'); -const PubSubMetricsBackend = require('../../lib/backends/pubsub-metrics'); +const MetricsBackend = require('../../lib/backends/metrics'); const apikey = 1234; const mapConfig = { version: '1.8.0', @@ -37,11 +37,11 @@ function templateBuilder ({ name }) { }; }; -describe('pubsub metrics middleware', function () { +describe('metrics middleware', function () { beforeEach(function () { - this.originalPubSubMetricsBackendSendMethod = PubSubMetricsBackend.prototype.send; + this.originalMetricsBackendSendMethod = MetricsBackend.prototype.send; this.pubSubMetricsBackendSendMethodCalled = false; - PubSubMetricsBackend.prototype.send = (event, attributes) => { + MetricsBackend.prototype.send = (event, attributes) => { this.pubSubMetricsBackendSendMethodCalled = true; this.pubSubMetricsBackendSendMethodCalledWith = { event, attributes }; return Promise.resolve(); @@ -49,7 +49,7 @@ describe('pubsub metrics middleware', function () { }); afterEach(function () { - PubSubMetricsBackend.prototype.send = this.originalPubSubMetricsBackendSendMethod; + MetricsBackend.prototype.send = this.originalMetricsBackendSendMethod; }); it('should not send event if not enabled', function (done) { From 798d01077654ae9f9bcd1f21f27d88d02532adf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 14:32:08 +0200 Subject: [PATCH 16/27] Ensure "map_id" and "cache_buster" as part of the event --- lib/api/map/anonymous-map-controller.js | 5 +- lib/api/middlewares/last-modified-header.js | 2 + .../last-updated-time-layergroup.js | 2 + lib/api/middlewares/metrics.js | 28 ++++- lib/api/template/named-template-controller.js | 3 - .../mapconfig/provider/named-map-provider.js | 4 + test/index.js | 2 + test/integration/metrics-test.js | 119 ++++++++++++------ 8 files changed, 120 insertions(+), 45 deletions(-) diff --git a/lib/api/map/anonymous-map-controller.js b/lib/api/map/anonymous-map-controller.js index 135da65f..46ed876a 100644 --- a/lib/api/map/anonymous-map-controller.js +++ b/lib/api/map/anonymous-map-controller.js @@ -87,9 +87,6 @@ module.exports = class AnonymousMapController { from: { req: { query: { client: 'client' } - }, - res: { - body: { layergroupid: 'map_id' } } } }; @@ -230,6 +227,7 @@ function createLayergroup (mapBackend, userLimitsBackend, pgConnection, affected ); res.locals.mapConfig = mapConfig; + res.locals.mapConfigProvider = mapConfigProvider; res.locals.analysesResults = context.analysesResults; const mapParams = { dbuser, dbname, dbpassword, dbhost, dbport }; @@ -243,7 +241,6 @@ function createLayergroup (mapBackend, userLimitsBackend, pgConnection, affected res.statusCode = 200; res.body = layergroup; - res.locals.mapConfigProvider = mapConfigProvider; next(); }); diff --git a/lib/api/middlewares/last-modified-header.js b/lib/api/middlewares/last-modified-header.js index 4f00c0d2..4afd283f 100644 --- a/lib/api/middlewares/last-modified-header.js +++ b/lib/api/middlewares/last-modified-header.js @@ -36,6 +36,8 @@ module.exports = function setLastModifiedHeader () { res.set('Last-Modified', lastModifiedDate.toUTCString()); + res.locals.cache_buster = lastUpdatedAt; + next(); }); }; diff --git a/lib/api/middlewares/last-updated-time-layergroup.js b/lib/api/middlewares/last-updated-time-layergroup.js index a1c4593a..1c4a3cab 100644 --- a/lib/api/middlewares/last-updated-time-layergroup.js +++ b/lib/api/middlewares/last-updated-time-layergroup.js @@ -22,6 +22,8 @@ module.exports = function setLastUpdatedTimeToLayergroup () { layergroup.layergroupid = layergroup.layergroupid + ':' + lastUpdateTime; layergroup.last_updated = new Date(lastUpdateTime).toISOString(); + res.locals.cache_buster = lastUpdateTime; + next(); }); }; diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index 82924f47..898c2069 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -3,7 +3,7 @@ const EVENT_VERSION = '1'; const MAX_LENGTH = 100; -module.exports = function metrics ({ enabled, metricsBackend, logger, tags }) { +module.exports = function metrics ({ enabled, tags, metricsBackend, logger }) { if (!enabled) { return function metricsDisabledMiddleware (req, res, next) { next(); @@ -47,6 +47,8 @@ function getEventData (req, res, tags) { event_time: new Date().toISOString(), user_id: res.locals.userId, user_agent: req.get('User-Agent'), + map_id: getLayergroupid({ res }), + cache_buster: getCacheBuster({ res }), response_code: res.statusCode.toString(), response_time: getResponseTime(res), source_domain: req.hostname, @@ -67,6 +69,30 @@ function normalizedField (field) { return field.toString().trim().substr(0, MAX_LENGTH); } +function getLayergroupid ({ res }) { + if (res.locals.token) { + return res.locals.token; + } + + if (res.locals.mapConfig) { + return res.locals.mapConfig.id(); + } + + if (res.locals.mapConfigProvider && res.locals.mapConfigProvider.mapConfig) { + return res.locals.mapConfigProvider.mapConfig.id(); + } +} + +function getCacheBuster ({ res }) { + if (res.locals.cache_buster !== undefined) { + return `${res.locals.cache_buster}`; + } + + if (res.locals.mapConfigProvider) { + return `${res.locals.mapConfigProvider.getCacheBuster()}`; + } +} + // FIXME: 'X-Tiler-Profiler' might not be accurate enough function getResponseTime (res) { const profiler = res.get('X-Tiler-Profiler'); diff --git a/lib/api/template/named-template-controller.js b/lib/api/template/named-template-controller.js index b1fd7348..88270b89 100644 --- a/lib/api/template/named-template-controller.js +++ b/lib/api/template/named-template-controller.js @@ -85,9 +85,6 @@ module.exports = class NamedMapController { from: { req: { query: { client: 'client' } - }, - res: { - body: { layergroupid: 'map_id' } } } }; diff --git a/lib/models/mapconfig/provider/named-map-provider.js b/lib/models/mapconfig/provider/named-map-provider.js index d0823f89..0818eef2 100644 --- a/lib/models/mapconfig/provider/named-map-provider.js +++ b/lib/models/mapconfig/provider/named-map-provider.js @@ -40,6 +40,10 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { this.authToken = authToken; this.params = params; + // FIXME: why is this different thah that: + // this.cacheBuster = params.cache_buster || 0; + // test: "should fail to use template from named map provider after template deletion" + // check named-map-provider-cache invalidation this.cacheBuster = Date.now(); // use template after call to mapConfig diff --git a/test/index.js b/test/index.js index 84f43989..d1e27973 100644 --- a/test/index.js +++ b/test/index.js @@ -77,6 +77,8 @@ async function populateDatabase () { async function populateRedis () { const commands = ` + FLUSHALL + HMSET rails:users:localhost \ id ${TEST_USER_ID} \ database_name "${TEST_DB}" \ diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 147224aa..56ff4f7f 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -3,6 +3,7 @@ const assert = require('assert'); const TestClient = require('../support/test-client'); const MetricsBackend = require('../../lib/backends/metrics'); +const LayergroupToken = require('../../lib/models/layergroup-token'); const apikey = 1234; const mapConfig = { version: '1.8.0', @@ -48,8 +49,9 @@ describe('metrics middleware', function () { }; }); - afterEach(function () { + afterEach(function (done) { MetricsBackend.prototype.send = this.originalMetricsBackendSendMethod; + return this.testClient.drain(done); }); it('should not send event if not enabled', function (done) { @@ -59,9 +61,10 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': '1' }; const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } @@ -69,16 +72,17 @@ describe('metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - return testClient.drain(done); + return done(); }); }); it('should not send event if headers not present', function (done) { const extraHeaders = {}; const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } @@ -86,7 +90,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - return testClient.drain(done); + return done(); }); }); @@ -103,14 +107,18 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); @@ -118,8 +126,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - return testClient.drain(done); + return done(); }); }); @@ -137,14 +147,18 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': 1 }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); @@ -152,8 +166,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - return testClient.drain(done); + return done(); }); }); @@ -181,10 +197,12 @@ describe('metrics middleware', function () { } ] }; - const testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + + this.testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + const params = { response: { status: 400 } }; - testClient.getLayergroup(params, (err, body) => { + this.testClient.getLayergroup(params, (err, body) => { if (err) { return done(err); } @@ -196,8 +214,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -212,9 +232,10 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - testClient.getTile(0, 0, 0, (err, res, tile) => { + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getTile(0, 0, 0, (err, res, tile) => { if (err) { return done(err); } @@ -224,8 +245,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -240,7 +263,8 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); const params = { response: { @@ -251,7 +275,7 @@ describe('metrics middleware', function () { } }; - testClient.getTile(0, 0, 2, params, (err, res, tile) => { + this.testClient.getTile(0, 0, 2, params, (err, res, tile) => { if (err) { return done(err); } @@ -261,8 +285,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -280,14 +306,18 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'map' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - testClient.getLayergroup((err, body) => { + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { if (err) { return done(err); } assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); @@ -295,8 +325,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - return testClient.drain(done); + return done(); }); }); @@ -330,7 +362,7 @@ describe('metrics middleware', function () { } }; - const testClient = new TestClient(templateMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(templateMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); const params = { response: { @@ -340,7 +372,8 @@ describe('metrics middleware', function () { } } }; - testClient.getLayergroup(params, (err, body) => { + + this.testClient.getLayergroup(params, (err, body) => { if (err) { return done(err); } @@ -352,8 +385,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -369,9 +404,10 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'tile' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - testClient.getTile(0, 0, 0, (err, body) => { + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getTile(0, 0, 0, (err, body) => { if (err) { return done(err); } @@ -381,8 +417,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -400,9 +438,10 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'preview' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - testClient.getPreview(640, 480, {}, (err, res, body) => { + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getPreview(640, 480, {}, (err, res, body) => { if (err) { return done(err); } @@ -414,8 +453,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); @@ -433,7 +474,9 @@ describe('metrics middleware', function () { }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; const template = templateBuilder({ name: 'preview-errored' }); - const testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + const widthTooLarge = 8193; const params = { response: { @@ -444,7 +487,7 @@ describe('metrics middleware', function () { } }; - testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { + this.testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { if (err) { return done(err); } @@ -455,8 +498,10 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - return testClient.drain(done); + return done(); }); }); }); From c91d78fe51c03fd4f85e1a4d8eac0c24133f040f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 16:44:14 +0200 Subject: [PATCH 17/27] Also export template hash --- lib/models/layergroup-token.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/models/layergroup-token.js b/lib/models/layergroup-token.js index 1e4a6951..5332ce0d 100644 --- a/lib/models/layergroup-token.js +++ b/lib/models/layergroup-token.js @@ -4,7 +4,7 @@ * @param {String} token might match the following pattern: {user}@{tpl_id}@{token}:{cache_buster} */ function parse (token) { - var signer, cacheBuster; + var signer, cacheBuster, templateHash; var tokenSplit = token.split(':'); @@ -17,7 +17,7 @@ function parse (token) { if (tokenSplit.length > 1) { signer = tokenSplit.shift(); if (tokenSplit.length > 1) { - /* var template_hash = */tokenSplit.shift(); // unused + templateHash = tokenSplit.shift(); } token = tokenSplit.shift(); } @@ -25,7 +25,9 @@ function parse (token) { return { token: token, signer: signer, - cacheBuster: cacheBuster + cacheBuster: cacheBuster, + templateHash: templateHash }; } + module.exports.parse = parse; From dbc5d65d908194cae3ee62a580f13cc608879d40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 17:26:33 +0200 Subject: [PATCH 18/27] Send template_hash as part of the metrics event --- lib/api/middlewares/layergroup-id-header.js | 3 ++- lib/api/middlewares/layergroup-token.js | 4 ++++ lib/api/middlewares/metrics.js | 5 +++++ test/integration/metrics-test.js | 10 +++++++++- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/api/middlewares/layergroup-id-header.js b/lib/api/middlewares/layergroup-id-header.js index 94470972..edc452ed 100644 --- a/lib/api/middlewares/layergroup-id-header.js +++ b/lib/api/middlewares/layergroup-id-header.js @@ -6,8 +6,9 @@ module.exports = function setLayergroupIdHeader (templateMaps, useTemplateHash) const layergroup = res.body; if (useTemplateHash) { - var templateHash = templateMaps.fingerPrint(template).substring(0, 8); + const templateHash = templateMaps.fingerPrint(template).substring(0, 8); layergroup.layergroupid = `${user}@${templateHash}@${layergroup.layergroupid}`; + res.locals.templateHash = templateHash; } res.set('X-Layergroup-Id', layergroup.layergroupid); diff --git a/lib/api/middlewares/layergroup-token.js b/lib/api/middlewares/layergroup-token.js index 88e9295d..7d8003bb 100644 --- a/lib/api/middlewares/layergroup-token.js +++ b/lib/api/middlewares/layergroup-token.js @@ -13,6 +13,10 @@ module.exports = function layergroupToken () { res.locals.token = layergroupToken.token; res.locals.cache_buster = layergroupToken.cacheBuster; + if (layergroupToken.templateHash) { + res.locals.templateHash = layergroupToken.templateHash; + } + if (layergroupToken.signer) { res.locals.signer = layergroupToken.signer; diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index 898c2069..c0095913 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -49,6 +49,7 @@ function getEventData (req, res, tags) { user_agent: req.get('User-Agent'), map_id: getLayergroupid({ res }), cache_buster: getCacheBuster({ res }), + template_hash: getTemplateHash({ res }), response_code: res.statusCode.toString(), response_time: getResponseTime(res), source_domain: req.hostname, @@ -93,6 +94,10 @@ function getCacheBuster ({ res }) { } } +function getTemplateHash ({ res }) { + return res.locals.templateHash; +} + // FIXME: 'X-Tiler-Profiler' might not be accurate enough function getResponseTime (res) { const profiler = res.get('X-Tiler-Profiler'); diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 56ff4f7f..72354263 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -316,7 +316,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof body.layergroupid, 'string'); - const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + const { token, cacheBuster, templateHash } = LayergroupToken.parse(body.layergroupid); assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); @@ -327,6 +327,7 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, templateHash); return done(); }); @@ -387,6 +388,8 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + // TODO: uncomment this + // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); return done(); }); @@ -419,6 +422,7 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); return done(); }); @@ -455,6 +459,8 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + // TODO: uncomment this + // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); return done(); }); @@ -500,6 +506,8 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + // TODO: uncomment this + // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); return done(); }); From 7e31b956bfcd38d1c121997a218022463d3e7be5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 18:25:01 +0200 Subject: [PATCH 19/27] Send stat_tag metric when available --- lib/api/middlewares/metrics.js | 12 ++++++++++++ test/integration/metrics-test.js | 13 ++++++++++--- test/support/test-client.js | 6 ++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index c0095913..33f81670 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -50,6 +50,7 @@ function getEventData (req, res, tags) { map_id: getLayergroupid({ res }), cache_buster: getCacheBuster({ res }), template_hash: getTemplateHash({ res }), + stat_tag: getStatTag({ res }), response_code: res.statusCode.toString(), response_time: getResponseTime(res), source_domain: req.hostname, @@ -98,6 +99,17 @@ function getTemplateHash ({ res }) { return res.locals.templateHash; } +function getStatTag ({ res }) { + if (res.locals.mapConfig) { + return res.locals.mapConfig.obj().stat_tag; + } + + // FIXME: don't expect that mapConfig is already set + if (res.locals.mapConfigProvider && res.locals.mapConfigProvider.mapConfig) { + return res.locals.mapConfigProvider.mapConfig.obj().stat_tag; + } +} + // FIXME: 'X-Tiler-Profiler' might not be accurate enough function getResponseTime (res) { const profiler = res.get('X-Tiler-Profiler'); diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 72354263..789a10d1 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -23,6 +23,7 @@ function templateBuilder ({ name }) { version: '0.0.1', name: `metrics-template-${name}`, layergroup: { + stat_tag: `stat-tag-${name}`, version: '1.8.0', layers: [ { @@ -328,6 +329,7 @@ describe('metrics middleware', function () { assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, templateHash); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); return done(); }); @@ -346,10 +348,11 @@ describe('metrics middleware', function () { 'Carto-Event-Group-Id': expectedEventGroupId }; const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const templateMissingCartoCSS = { + const templateMissingCartoCSSVersion = { version: '0.0.1', - name: 'metrics-template', + name: 'metrics-template-missing-cartocss-version', layergroup: { + stat_tag: 'stat-tag-missing-cartocss-version', version: '1.8.0', layers: [ { @@ -363,7 +366,7 @@ describe('metrics middleware', function () { } }; - this.testClient = new TestClient(templateMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(templateMissingCartoCSSVersion, apikey, extraHeaders, overrideServerOptions); const params = { response: { @@ -390,6 +393,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); // TODO: uncomment this // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, templateMissingCartoCSSVersion.layergroup.stat_tag); return done(); }); @@ -423,6 +427,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); return done(); }); @@ -461,6 +466,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); // TODO: uncomment this // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); return done(); }); @@ -508,6 +514,7 @@ describe('metrics middleware', function () { assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); // TODO: uncomment this // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); return done(); }); diff --git a/test/support/test-client.js b/test/support/test-client.js index 542855f9..36183a0d 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -1055,6 +1055,9 @@ TestClient.prototype.getLayergroup = function (params, callback) { self.keysToDelete['map_cfg|' + LayergroupToken.parse(parsedBody.layergroupid).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; } + if (res.statusCode === 200 && self.template && self.template.layergroup && self.template.layergroup.stat_tag) { + self.keysToDelete[`user:localhost:mapviews:stat_tag:${self.template.layergroup.stat_tag}`] = 5; + } } if (err) { return callback(err); @@ -1792,6 +1795,9 @@ TestClient.prototype.getPreview = function (width, height, params = {}, callback switch (res.headers['content-type']) { case 'image/png': this.keysToDelete['user:localhost:mapviews:global'] = 5; + if (this.template.layergroup && this.template.layergroup.stat_tag) { + this.keysToDelete[`user:localhost:mapviews:stat_tag:${this.template.layergroup.stat_tag}`] = 5; + } body = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary')); break; case 'application/json; charset=utf-8': From d5348dd9d4b5289cc9c75c76bc48d2aabad706f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 29 Apr 2020 18:48:10 +0200 Subject: [PATCH 20/27] Rename fields from headers of metrics --- lib/api/middlewares/metrics.js | 4 ++-- test/integration/metrics-test.js | 34 ++++++++++++++++---------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index 33f81670..c4b3d570 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -41,9 +41,9 @@ function getEventData (req, res, tags) { } const attributes = Object.assign({}, { - metrics_event: normalizedField(req.get('Carto-Event')), + client_event: normalizedField(req.get('Carto-Event')), + client_event_group_id: normalizedField(req.get('Carto-Event-Group-Id')), event_source: normalizedField(req.get('Carto-Event-Source')), - event_group_id: normalizedField(req.get('Carto-Event-Group-Id')), event_time: new Date().toISOString(), user_id: res.locals.userId, user_agent: req.get('User-Agent'), diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 789a10d1..13cc7f5f 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -122,9 +122,9 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); @@ -162,9 +162,9 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); @@ -210,9 +210,9 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); @@ -244,7 +244,7 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); @@ -284,7 +284,7 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); @@ -321,9 +321,9 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); @@ -384,9 +384,9 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); @@ -422,7 +422,7 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); @@ -457,9 +457,9 @@ describe('metrics middleware', function () { assert.ok(this.pubSubMetricsBackendSendMethodCalled); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); @@ -505,9 +505,9 @@ describe('metrics middleware', function () { } assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.metrics_event, expectedMetricsEvent); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_group_id, expectedEventGroupId); + assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); From 8d73571f5bc7413a5a1accd8ef4c6819ae9578b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 30 Apr 2020 12:30:31 +0200 Subject: [PATCH 21/27] Simplify assertions --- test/integration/metrics-test.js | 201 ++++++++++++++++++------------- 1 file changed, 116 insertions(+), 85 deletions(-) diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 13cc7f5f..53640792 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -39,7 +39,7 @@ function templateBuilder ({ name }) { }; }; -describe('metrics middleware', function () { +describe('metrics', function () { beforeEach(function () { this.originalMetricsBackendSendMethod = MetricsBackend.prototype.send; this.pubSubMetricsBackendSendMethodCalled = false; @@ -121,14 +121,17 @@ describe('metrics middleware', function () { const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(attributes.map_id, token); + assert.strictEqual(attributes.cache_buster, cacheBuster); return done(); }); @@ -161,14 +164,17 @@ describe('metrics middleware', function () { const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(attributes.map_id, token); + assert.strictEqual(attributes.cache_buster, cacheBuster); return done(); }); @@ -209,14 +215,17 @@ describe('metrics middleware', function () { } assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); return done(); }); @@ -242,12 +251,15 @@ describe('metrics middleware', function () { } assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); return done(); }); @@ -282,12 +294,15 @@ describe('metrics middleware', function () { } assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); return done(); }); @@ -320,16 +335,19 @@ describe('metrics middleware', function () { const { token, cacheBuster, templateHash } = LayergroupToken.parse(body.layergroupid); assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, token); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, cacheBuster); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, templateHash); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(attributes.map_id, token); + assert.strictEqual(attributes.cache_buster, cacheBuster); + assert.strictEqual(attributes.template_hash, templateHash); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); return done(); }); @@ -383,17 +401,20 @@ describe('metrics middleware', function () { } assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); // TODO: uncomment this - // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, templateMissingCartoCSSVersion.layergroup.stat_tag); + // assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, templateMissingCartoCSSVersion.layergroup.stat_tag); return done(); }); @@ -420,14 +441,17 @@ describe('metrics middleware', function () { } assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); return done(); }); @@ -456,17 +480,20 @@ describe('metrics middleware', function () { } assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); // TODO: uncomment this - // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); + // assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); return done(); }); @@ -504,17 +531,21 @@ describe('metrics middleware', function () { return done(err); } - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.event, expectedEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event, expectedMetricsEvent); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.event_source, expectedEventSource); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.response_code, expectedResponseCode); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_type, expectedMapType); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.map_id, 'string'); - assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.cache_buster, 'string'); + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); // TODO: uncomment this - // assert.strictEqual(typeof this.pubSubMetricsBackendSendMethodCalledWith.attributes.template_hash, 'string'); - assert.strictEqual(this.pubSubMetricsBackendSendMethodCalledWith.attributes.stat_tag, template.layergroup.stat_tag); + // assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); return done(); }); From a196a26ab403f03f556897006e7cf6e1db2feeb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 30 Apr 2020 13:09:12 +0200 Subject: [PATCH 22/27] Get templateHash for static tile request and errored named map instantiations --- lib/api/middlewares/metrics.js | 14 +++++++++++++- lib/backends/template-maps.js | 11 +++++++---- .../mapconfig/provider/named-map-provider.js | 10 +++++++++- test/integration/metrics-test.js | 9 +++------ 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index c4b3d570..b51ddba8 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -96,7 +96,19 @@ function getCacheBuster ({ res }) { } function getTemplateHash ({ res }) { - return res.locals.templateHash; + if (res.locals.templateHash) { + return res.locals.templateHash; + } + + if (res.locals.mapConfigProvider && res.locals.mapConfigProvider.getTemplateHash) { + let templateHash; + + try { + templateHash = res.locals.mapConfigProvider.getTemplateHash(); + } catch (e) {} + + return templateHash; + } } function getStatTag ({ res }) { diff --git a/lib/backends/template-maps.js b/lib/backends/template-maps.js index 9d09eb9d..f8ff641e 100644 --- a/lib/backends/template-maps.js +++ b/lib/backends/template-maps.js @@ -511,12 +511,15 @@ TemplateMaps.prototype.instance = function (template, params) { // Return a fingerPrint of the object TemplateMaps.prototype.fingerPrint = function (template) { - return crypto.createHash('md5') - .update(JSON.stringify(template)) - .digest('hex') - ; + return fingerPrint(template); }; +function fingerPrint (template) { + return crypto.createHash('md5').update(JSON.stringify(template)).digest('hex'); +} + +module.exports.fingerPrint = fingerPrint; + module.exports.templateName = function templateName (templateId) { var templateIdTokens = templateId.split('@'); var name = templateIdTokens[0]; diff --git a/lib/models/mapconfig/provider/named-map-provider.js b/lib/models/mapconfig/provider/named-map-provider.js index 0818eef2..a131ce18 100644 --- a/lib/models/mapconfig/provider/named-map-provider.js +++ b/lib/models/mapconfig/provider/named-map-provider.js @@ -4,7 +4,7 @@ const BaseMapConfigProvider = require('./base-mapconfig-adapter'); const crypto = require('crypto'); const dot = require('dot'); const MapConfig = require('windshaft').model.MapConfig; -const templateName = require('../../../backends/template-maps').templateName; +const { templateName, fingerPrint: templateFingerPrint } = require('../../../backends/template-maps'); // Configure bases for cache keys suitable for string interpolation const baseKey = '{{=it.dbname}}:{{=it.user}}:{{=it.templateName}}'; @@ -258,6 +258,14 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { getTemplateName () { return this.templateName; } + + getTemplateHash () { + if (!this.template) { + throw new Error('Missing template, call "getTemplate()" method first'); + } + + return templateFingerPrint(this.template); + } }; function createConfigHash (config) { diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 53640792..52d345a7 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -412,8 +412,7 @@ describe('metrics', function () { assert.strictEqual(attributes.map_type, expectedMapType); assert.strictEqual(typeof attributes.map_id, 'string'); assert.strictEqual(typeof attributes.cache_buster, 'string'); - // TODO: uncomment this - // assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); assert.strictEqual(attributes.stat_tag, templateMissingCartoCSSVersion.layergroup.stat_tag); return done(); @@ -491,8 +490,7 @@ describe('metrics', function () { assert.strictEqual(attributes.map_type, expectedMapType); assert.strictEqual(typeof attributes.map_id, 'string'); assert.strictEqual(typeof attributes.cache_buster, 'string'); - // TODO: uncomment this - // assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); return done(); @@ -543,8 +541,7 @@ describe('metrics', function () { assert.strictEqual(attributes.map_type, expectedMapType); assert.strictEqual(typeof attributes.map_id, 'string'); assert.strictEqual(typeof attributes.cache_buster, 'string'); - // TODO: uncomment this - // assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); return done(); From 8c38ecf808848105e525eb28f9da520a02447c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 30 Apr 2020 13:24:41 +0200 Subject: [PATCH 23/27] Missing substring --- lib/api/middlewares/metrics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index b51ddba8..084c353f 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -104,7 +104,7 @@ function getTemplateHash ({ res }) { let templateHash; try { - templateHash = res.locals.mapConfigProvider.getTemplateHash(); + templateHash = res.locals.mapConfigProvider.getTemplateHash().substring(0, 8); } catch (e) {} return templateHash; From 3cf17c8baba9e58e5f3298b764a24e063a05fe4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 1 May 2020 10:40:56 +0200 Subject: [PATCH 24/27] typo --- .../mapconfig/provider/named-map-provider.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/models/mapconfig/provider/named-map-provider.js b/lib/models/mapconfig/provider/named-map-provider.js index a131ce18..63516853 100644 --- a/lib/models/mapconfig/provider/named-map-provider.js +++ b/lib/models/mapconfig/provider/named-map-provider.js @@ -40,7 +40,7 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { this.authToken = authToken; this.params = params; - // FIXME: why is this different thah that: + // FIXME: why is this different than that: // this.cacheBuster = params.cache_buster || 0; // test: "should fail to use template from named map provider after template deletion" // check named-map-provider-cache invalidation @@ -98,17 +98,16 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider { const { user, rendererParams } = this; - this.mapConfigAdapter.getMapConfig( - user, requestMapConfig, rendererParams, context, (err, mapConfig, stats = {}) => { - if (err) { - return callback(err); - } + this.mapConfigAdapter.getMapConfig(user, requestMapConfig, rendererParams, context, (err, mapConfig, stats = {}) => { + if (err) { + return callback(err); + } - this.mapConfig = (mapConfig === null) ? null : new MapConfig(mapConfig, context.datasource); - this.analysesResults = context.analysesResults || []; + this.mapConfig = (mapConfig === null) ? null : new MapConfig(mapConfig, context.datasource); + this.analysesResults = context.analysesResults || []; - return callback(null, this.mapConfig, this.rendererParams, this.context, stats); - }); + return callback(null, this.mapConfig, this.rendererParams, this.context, stats); + }); }); }); } From 24863b639386129822865eea408298114c3a8a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 1 May 2020 10:51:33 +0200 Subject: [PATCH 25/27] Update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 29e94124..14f69a0b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -20,6 +20,7 @@ Announcements: - Rename NamedMapProviderReporter by NamedMapProviderCacheReporter - Remove `bootstrapFonts` at process startup (now done in `windshaft@6.0.0`) - Stop checking the installed version of some dependencies while testing +- Send metrics about `map views` (#1162) Bug Fixes: - Parsing date column in numeric histograms (#1160) From 05e77b2aed96019164b0336f78b87e336f100134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 1 May 2020 11:43:37 +0200 Subject: [PATCH 26/27] Add test with mapconfig's query against a table to ensure cache buster metrics are sent with the right values. --- test/integration/metrics-test.js | 941 ++++++++++++++++--------------- 1 file changed, 487 insertions(+), 454 deletions(-) diff --git a/test/integration/metrics-test.js b/test/integration/metrics-test.js index 52d345a7..c1b75eec 100644 --- a/test/integration/metrics-test.js +++ b/test/integration/metrics-test.js @@ -18,12 +18,27 @@ const mapConfig = { ] }; +const mapConfigWithTable = { + version: '1.8.0', + layers: [ + { + options: { + sql: 'select * from test_table', + cartocss: TestClient.CARTOCSS.POINTS, + cartocss_version: '2.3.0' + } + } + ] +}; + function templateBuilder ({ name }) { + const templateName = `metrics-template-${name}-${Date.now()}`; + return { version: '0.0.1', - name: `metrics-template-${name}`, + name: templateName, layergroup: { - stat_tag: `stat-tag-${name}`, + stat_tag: `stat-tag-${templateName}`, version: '1.8.0', layers: [ { @@ -37,514 +52,532 @@ function templateBuilder ({ name }) { ] } }; -}; +} -describe('metrics', function () { - beforeEach(function () { - this.originalMetricsBackendSendMethod = MetricsBackend.prototype.send; - this.pubSubMetricsBackendSendMethodCalled = false; - MetricsBackend.prototype.send = (event, attributes) => { - this.pubSubMetricsBackendSendMethodCalled = true; - this.pubSubMetricsBackendSendMethodCalledWith = { event, attributes }; - return Promise.resolve(); - }; - }); +function templateMissingCartoCSSVersionBuilder () { + const templateName = `missing-cartocss-version-${Date.now()}`; - afterEach(function (done) { - MetricsBackend.prototype.send = this.originalMetricsBackendSendMethod; - return this.testClient.drain(done); - }); - - it('should not send event if not enabled', function (done) { - const extraHeaders = { - 'Carto-Event': 'test-event', - 'Carto-Event-Source': 'test', - 'Carto-Event-Group-Id': '1' - }; - const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - - this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } - - assert.strictEqual(typeof body.layergroupid, 'string'); - assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - - return done(); - }); - }); - - it('should not send event if headers not present', function (done) { - const extraHeaders = {}; - const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - - this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } - - assert.strictEqual(typeof body.layergroupid, 'string'); - assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - - return done(); - }); - }); - - it('should send event for map requests', function (done) { - const expectedEvent = 'map_view'; - const expectedMetricsEvent = 'event-test'; - const expectedEventSource = 'event-source-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '200'; - const expectedMapType = 'anonymous'; - const extraHeaders = { - 'Carto-Event': expectedMetricsEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - - this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } - - assert.strictEqual(typeof body.layergroupid, 'string'); - - const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(attributes.map_id, token); - assert.strictEqual(attributes.cache_buster, cacheBuster); - - return done(); - }); - }); - - it('should normalized headers type and length', function (done) { - const expectedEvent = 'map_view'; - const eventLong = 'If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; - const expectedMetricsEvent = eventLong.trim().substr(0, 100); - const expectedEventGroupId = '1'; - const expectedEventSource = 'test'; - const expectedResponseCode = '200'; - const expectedMapType = 'anonymous'; - const extraHeaders = { - 'Carto-Event': eventLong, - 'Carto-Event-Source': 'test', - 'Carto-Event-Group-Id': 1 - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - - this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } - - assert.strictEqual(typeof body.layergroupid, 'string'); - - const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(attributes.map_id, token); - assert.strictEqual(attributes.cache_buster, cacheBuster); - - return done(); - }); - }); - - it('should send event when error', function (done) { - const expectedEvent = 'map_view'; - const expectedMetricsEvent = 'event-test'; - const expectedEventSource = 'event-source-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '400'; - const expectedMapType = 'anonymous'; - const extraHeaders = { - 'Carto-Event': expectedMetricsEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const mapConfigMissingCartoCSS = { + return { + version: '0.0.1', + name: templateName, + layergroup: { + stat_tag: `stat-tag-${templateName}`, version: '1.8.0', layers: [ { + type: 'cartodb', options: { sql: TestClient.SQL.ONE_POINT, cartocss: TestClient.CARTOCSS.POINTS } } ] - }; + } + }; +} - this.testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); +const suites = [ + { + desc: 'map config with live query', + mapConfig + }, + { + desc: 'map config with query against table', + mapConfig: mapConfigWithTable + } +]; - const params = { response: { status: 400 } }; - - this.testClient.getLayergroup(params, (err, body) => { - if (err) { - return done(err); - } - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - - return done(); +suites.forEach(function ({ desc, mapConfig }) { + describe(`metrics: ${desc}`, function () { + beforeEach(function () { + this.originalMetricsBackendSendMethod = MetricsBackend.prototype.send; + this.pubSubMetricsBackendSendMethodCalled = false; + MetricsBackend.prototype.send = (event, attributes) => { + this.pubSubMetricsBackendSendMethodCalled = true; + this.pubSubMetricsBackendSendMethodCalledWith = { event, attributes }; + return Promise.resolve(); + }; }); - }); - it.skip('should send event for tile requests', function (done) { - const expectedEvent = 'event-tile-test'; - const expectedEventSource = 'event-source-tile-test'; - const expectedEventGroupId = '12345'; - const expectedResponseCode = '200'; - const extraHeaders = { - 'Carto-Event': expectedEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - - this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - - this.testClient.getTile(0, 0, 0, (err, res, tile) => { - if (err) { - return done(err); - } - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - - return done(); + afterEach(function (done) { + MetricsBackend.prototype.send = this.originalMetricsBackendSendMethod; + return this.testClient.drain(done); }); - }); - it.skip('should send event for errored tile requests', function (done) { - const expectedEvent = 'event-tile-test'; - const expectedEventSource = 'event-source-tile-test'; - const expectedEventGroupId = '12345'; - const expectedResponseCode = '400'; - const extraHeaders = { - 'Carto-Event': expectedEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + it('should not send event if not enabled', function (done) { + const extraHeaders = { + 'Carto-Event': 'test-event', + 'Carto-Event-Source': 'test', + 'Carto-Event-Group-Id': '1' + }; + const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - const params = { - response: { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); } - } - }; - this.testClient.getTile(0, 0, 2, params, (err, res, tile) => { - if (err) { - return done(err); - } + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - - return done(); + return done(); + }); }); - }); - it('should send event for named map requests', function (done) { - const expectedEvent = 'map_view'; - const expectedMetricsEvent = 'event-test'; - const expectedEventSource = 'event-source-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '200'; - const expectedMapType = 'named'; - const extraHeaders = { - 'Carto-Event': expectedMetricsEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const template = templateBuilder({ name: 'map' }); + it('should not send event if headers not present', function (done) { + const extraHeaders = {}; + const overrideServerOptions = { pubSubMetrics: { enabled: false } }; - this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - this.testClient.getLayergroup((err, body) => { - if (err) { - return done(err); - } + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } - assert.strictEqual(typeof body.layergroupid, 'string'); + assert.strictEqual(typeof body.layergroupid, 'string'); + assert.ok(!this.pubSubMetricsBackendSendMethodCalled); - const { token, cacheBuster, templateHash } = LayergroupToken.parse(body.layergroupid); - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(attributes.map_id, token); - assert.strictEqual(attributes.cache_buster, cacheBuster); - assert.strictEqual(attributes.template_hash, templateHash); - assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); - - return done(); + return done(); + }); }); - }); - it('should send event for errored named map requests', function (done) { - const expectedEvent = 'map_view'; - const expectedMetricsEvent = 'event-test'; - const expectedEventSource = 'event-source-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '400'; - const expectedMapType = 'named'; - const extraHeaders = { - 'Carto-Event': expectedMetricsEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const templateMissingCartoCSSVersion = { - version: '0.0.1', - name: 'metrics-template-missing-cartocss-version', - layergroup: { - stat_tag: 'stat-tag-missing-cartocss-version', + it('should send event for map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const expectedMapType = 'anonymous'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(attributes.map_id, token); + assert.strictEqual(attributes.cache_buster, cacheBuster); + + return done(); + }); + }); + + it('should normalized headers type and length', function (done) { + const expectedEvent = 'map_view'; + const eventLong = 'If you are sending a text this long in a header you kind of deserve the worst, honestly. I mean this is not a header, it is almost a novel, and you do not see any Novel cookie here, right?'; + const expectedMetricsEvent = eventLong.trim().substr(0, 100); + const expectedEventGroupId = '1'; + const expectedEventSource = 'test'; + const expectedResponseCode = '200'; + const expectedMapType = 'anonymous'; + const extraHeaders = { + 'Carto-Event': eventLong, + 'Carto-Event-Source': 'test', + 'Carto-Event-Group-Id': 1 + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster } = LayergroupToken.parse(body.layergroupid); + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(attributes.map_id, token); + assert.strictEqual(attributes.cache_buster, cacheBuster); + + return done(); + }); + }); + + it('should send event when error', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '400'; + const expectedMapType = 'anonymous'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const mapConfigMissingCartoCSS = { version: '1.8.0', layers: [ { - type: 'cartodb', options: { sql: TestClient.SQL.ONE_POINT, cartocss: TestClient.CARTOCSS.POINTS } } ] - } - }; + }; - this.testClient = new TestClient(templateMissingCartoCSSVersion, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(mapConfigMissingCartoCSS, apikey, extraHeaders, overrideServerOptions); - const params = { - response: { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' + const params = { response: { status: 400 } }; + + this.testClient.getLayergroup(params, (err, body) => { + if (err) { + return done(err); } - } - }; - this.testClient.getLayergroup(params, (err, body) => { - if (err) { - return done(err); - } + assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.ok(this.pubSubMetricsBackendSendMethodCalled); + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - assert.strictEqual(typeof attributes.template_hash, 'string'); - assert.strictEqual(attributes.stat_tag, templateMissingCartoCSSVersion.layergroup.stat_tag); - - return done(); + return done(); + }); }); - }); - it.skip('should send event for named map tile requests', function (done) { - const expectedEvent = 'event-named-map-tile-test'; - const expectedEventSource = 'event-source-named-map-tile-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '200'; - const extraHeaders = { - 'Carto-Event': expectedEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const template = templateBuilder({ name: 'tile' }); + it.skip('should send event for tile requests', function (done) { + const expectedEvent = 'event-tile-test'; + const expectedEventSource = 'event-source-tile-test'; + const expectedEventGroupId = '12345'; + const expectedResponseCode = '200'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); - this.testClient.getTile(0, 0, 0, (err, body) => { - if (err) { - return done(err); - } - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - assert.strictEqual(typeof attributes.template_hash, 'string'); - assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); - - return done(); - }); - }); - - it('should send event for static named map requests', function (done) { - const expectedEvent = 'map_view'; - const expectedMetricsEvent = 'event-test'; - const expectedEventSource = 'event-source-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '200'; - const expectedMapType = 'static'; - const extraHeaders = { - 'Carto-Event': expectedMetricsEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const template = templateBuilder({ name: 'preview' }); - - this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - - this.testClient.getPreview(640, 480, {}, (err, res, body) => { - if (err) { - return done(err); - } - - assert.ok(this.pubSubMetricsBackendSendMethodCalled); - - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - assert.strictEqual(typeof attributes.template_hash, 'string'); - assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); - - return done(); - }); - }); - - it('should send event for errored static named map requests', function (done) { - const expectedEvent = 'map_view'; - const expectedMetricsEvent = 'event-test'; - const expectedEventSource = 'event-source-test'; - const expectedEventGroupId = '1'; - const expectedResponseCode = '400'; - const expectedMapType = 'static'; - const extraHeaders = { - 'Carto-Event': expectedMetricsEvent, - 'Carto-Event-Source': expectedEventSource, - 'Carto-Event-Group-Id': expectedEventGroupId - }; - const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; - const template = templateBuilder({ name: 'preview-errored' }); - - this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); - - const widthTooLarge = 8193; - const params = { - response: { - status: 400, - headers: { - 'Content-Type': 'application/json; charset=utf-8' + this.testClient.getTile(0, 0, 0, (err, res, tile) => { + if (err) { + return done(err); } - } - }; - this.testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { - if (err) { - return done(err); - } + assert.ok(this.pubSubMetricsBackendSendMethodCalled); - assert.ok(this.pubSubMetricsBackendSendMethodCalled); + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; - const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); - assert.strictEqual(event, expectedEvent); - assert.strictEqual(attributes.client_event, expectedMetricsEvent); - assert.strictEqual(attributes.event_source, expectedEventSource); - assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); - assert.strictEqual(attributes.response_code, expectedResponseCode); - assert.strictEqual(attributes.map_type, expectedMapType); - assert.strictEqual(typeof attributes.map_id, 'string'); - assert.strictEqual(typeof attributes.cache_buster, 'string'); - assert.strictEqual(typeof attributes.template_hash, 'string'); - assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); + return done(); + }); + }); - return done(); + it.skip('should send event for errored tile requests', function (done) { + const expectedEvent = 'event-tile-test'; + const expectedEventSource = 'event-source-tile-test'; + const expectedEventGroupId = '12345'; + const expectedResponseCode = '400'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + + this.testClient = new TestClient(mapConfig, apikey, extraHeaders, overrideServerOptions); + + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getTile(0, 0, 2, params, (err, res, tile) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); + + return done(); + }); + }); + + it('should send event for named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const expectedMapType = 'named'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'map' }); + + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getLayergroup((err, body) => { + if (err) { + return done(err); + } + + assert.strictEqual(typeof body.layergroupid, 'string'); + + const { token, cacheBuster, templateHash } = LayergroupToken.parse(body.layergroupid); + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(attributes.map_id, token); + assert.strictEqual(attributes.cache_buster, cacheBuster); + assert.strictEqual(attributes.template_hash, templateHash); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); + + return done(); + }); + }); + + it('should send event for errored named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '400'; + const expectedMapType = 'named'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const templateMissingCartoCSSVersion = templateMissingCartoCSSVersionBuilder(); + this.testClient = new TestClient(templateMissingCartoCSSVersion, apikey, extraHeaders, overrideServerOptions); + + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getLayergroup(params, (err, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, templateMissingCartoCSSVersion.layergroup.stat_tag); + + return done(); + }); + }); + + it.skip('should send event for named map tile requests', function (done) { + const expectedEvent = 'event-named-map-tile-test'; + const expectedEventSource = 'event-source-named-map-tile-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const extraHeaders = { + 'Carto-Event': expectedEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'tile' }); + + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getTile(0, 0, 0, (err, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); + + return done(); + }); + }); + + it('should send event for static named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '200'; + const expectedMapType = 'static'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'preview' }); + + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + this.testClient.getPreview(640, 480, {}, (err, res, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); + + return done(); + }); + }); + + it('should send event for errored static named map requests', function (done) { + const expectedEvent = 'map_view'; + const expectedMetricsEvent = 'event-test'; + const expectedEventSource = 'event-source-test'; + const expectedEventGroupId = '1'; + const expectedResponseCode = '400'; + const expectedMapType = 'static'; + const extraHeaders = { + 'Carto-Event': expectedMetricsEvent, + 'Carto-Event-Source': expectedEventSource, + 'Carto-Event-Group-Id': expectedEventGroupId + }; + const overrideServerOptions = { pubSubMetrics: { enabled: true, topic: 'topic-test' } }; + const template = templateBuilder({ name: 'preview-errored' }); + + this.testClient = new TestClient(template, apikey, extraHeaders, overrideServerOptions); + + const widthTooLarge = 8193; + const params = { + response: { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8' + } + } + }; + + this.testClient.getPreview(widthTooLarge, 480, params, (err, res, body) => { + if (err) { + return done(err); + } + + assert.ok(this.pubSubMetricsBackendSendMethodCalled); + + const { event, attributes } = this.pubSubMetricsBackendSendMethodCalledWith; + + assert.strictEqual(event, expectedEvent); + assert.strictEqual(attributes.client_event, expectedMetricsEvent); + assert.strictEqual(attributes.event_source, expectedEventSource); + assert.strictEqual(attributes.client_event_group_id, expectedEventGroupId); + assert.strictEqual(attributes.response_code, expectedResponseCode); + assert.strictEqual(attributes.map_type, expectedMapType); + assert.strictEqual(typeof attributes.map_id, 'string'); + assert.strictEqual(typeof attributes.cache_buster, 'string'); + assert.strictEqual(typeof attributes.template_hash, 'string'); + assert.strictEqual(attributes.stat_tag, template.layergroup.stat_tag); + + return done(); + }); }); }); }); From 4dfc898587cb58ed05c553165c03429cf90e6470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 1 May 2020 13:25:03 +0200 Subject: [PATCH 27/27] Don't log when metrics where sent successfully --- lib/api/middlewares/metrics.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/api/middlewares/metrics.js b/lib/api/middlewares/metrics.js index 084c353f..e5c36a01 100644 --- a/lib/api/middlewares/metrics.js +++ b/lib/api/middlewares/metrics.js @@ -19,7 +19,6 @@ module.exports = function metrics ({ enabled, tags, metricsBackend, logger }) { const { event, attributes } = getEventData(req, res, tags); metricsBackend.send(event, attributes) - .then(() => logger.debug(`Event "${event}" published succesfully`)) .catch((error) => logger.error(`Failed to publish event "${event}": ${error.message}`)); });