From 7b53b7c30a7a57f800d639273aaa87c85e294b2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 3 Jun 2020 19:51:56 +0200 Subject: [PATCH] Stop using profiling wrongly. Now it only saves custom events from backends (tile, map, attributes, etc..) and calculates the response time. Besides, removed tags to know whether overviews are being used. --- lib/api/map/anonymous-map-controller.js | 11 +-------- .../map/attributes-layergroup-controller.js | 2 -- ...lustered-features-layergroup-controller.js | 2 -- lib/api/map/preview-layergroup-controller.js | 2 -- lib/api/map/preview-template-controller.js | 5 ++-- lib/api/map/tile-layergroup-controller.js | 2 -- lib/api/middlewares/authorize.js | 2 -- .../middlewares/check-json-content-type.js | 2 -- lib/api/middlewares/db-conn-setup.js | 2 -- .../middlewares/increment-map-view-count.js | 2 -- lib/api/middlewares/init-profiler.js | 11 --------- lib/api/middlewares/lzma.js | 2 -- lib/api/middlewares/map-error.js | 1 - lib/api/middlewares/profiler.js | 4 ++++ lib/api/middlewares/send-response.js | 2 -- lib/api/template/admin-template-controller.js | 6 ----- lib/api/template/named-template-controller.js | 8 ------- lib/api/template/tile-template-controller.js | 3 +-- lib/backends/auth.js | 6 ----- lib/models/dataview/base.js | 17 ++----------- lib/models/dataview/overviews/aggregation.js | 2 +- lib/models/dataview/overviews/formula.js | 2 +- lib/models/dataview/overviews/histogram.js | 2 +- .../adapter/mapconfig-overviews-adapter.js | 24 +++++++------------ 24 files changed, 21 insertions(+), 101 deletions(-) delete mode 100644 lib/api/middlewares/init-profiler.js diff --git a/lib/api/map/anonymous-map-controller.js b/lib/api/map/anonymous-map-controller.js index 7236935b..ad0f4324 100644 --- a/lib/api/map/anonymous-map-controller.js +++ b/lib/api/map/anonymous-map-controller.js @@ -7,7 +7,6 @@ const cleanUpQueryParams = require('../middlewares/clean-up-query-params'); const credentials = require('../middlewares/credentials'); const dbConnSetup = require('../middlewares/db-conn-setup'); const authorize = require('../middlewares/authorize'); -const initProfiler = require('../middlewares/init-profiler'); const checkJsonContentType = require('../middlewares/check-json-content-type'); const incrementMapViewCount = require('../middlewares/increment-map-view-count'); const augmentLayergroupData = require('../middlewares/augment-layergroup-data'); @@ -76,7 +75,6 @@ module.exports = class AnonymousMapController { } middlewares () { - const isTemplateInstantiation = false; const useTemplateHash = false; const includeQuery = true; const label = 'ANONYMOUS LAYERGROUP'; @@ -102,7 +100,6 @@ module.exports = class AnonymousMapController { dbConnSetup(this.pgConnection), rateLimit(this.userLimitsBackend, RATE_LIMIT_ENDPOINTS_GROUPS.ANONYMOUS), cleanUpQueryParams(['aggregation']), - initProfiler(isTemplateInstantiation), checkJsonContentType(), checkCreateLayergroup(), prepareAdapterMapConfig(this.mapConfigAdapter), @@ -143,7 +140,6 @@ function checkCreateLayergroup () { } } - req.profiler.done('checkCreateLayergroup'); return next(); }; } @@ -179,12 +175,7 @@ function prepareAdapterMapConfig (mapConfigAdapter) { requestMapConfig, params, context, - (err, requestMapConfig, stats = { overviewsAddedToMapconfig: false }) => { - req.profiler.done('anonymous.getMapConfig'); - - stats.mapType = 'anonymous'; - req.profiler.add(stats); - + (err, requestMapConfig) => { if (err) { return next(err); } diff --git a/lib/api/map/attributes-layergroup-controller.js b/lib/api/map/attributes-layergroup-controller.js index f734d3d2..ad0f1bc8 100644 --- a/lib/api/map/attributes-layergroup-controller.js +++ b/lib/api/map/attributes-layergroup-controller.js @@ -61,8 +61,6 @@ module.exports = class AttributesLayergroupController { function getFeatureAttributes (attributesBackend) { return function getFeatureAttributesMiddleware (req, res, next) { - req.profiler.start('windshaft.maplayer_attribute'); - const { mapConfigProvider } = res.locals; const { token } = res.locals; const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; diff --git a/lib/api/map/clustered-features-layergroup-controller.js b/lib/api/map/clustered-features-layergroup-controller.js index 7b300ac7..611813c4 100644 --- a/lib/api/map/clustered-features-layergroup-controller.js +++ b/lib/api/map/clustered-features-layergroup-controller.js @@ -62,8 +62,6 @@ module.exports = class AggregatedFeaturesLayergroupController { function getClusteredFeatures (clusterBackend) { return function getFeatureAttributesMiddleware (req, res, next) { - req.profiler.start('windshaft.maplayer_cluster_features'); - const { mapConfigProvider } = res.locals; const { user, token } = res.locals; const { dbuser, dbname, dbpassword, dbhost, dbport } = res.locals; diff --git a/lib/api/map/preview-layergroup-controller.js b/lib/api/map/preview-layergroup-controller.js index 698d94ff..0d8edf9e 100644 --- a/lib/api/map/preview-layergroup-controller.js +++ b/lib/api/map/preview-layergroup-controller.js @@ -100,7 +100,6 @@ function getPreviewImageByCenter (previewBackend) { const options = { mapConfigProvider, format, width, height, zoom, center }; previewBackend.getImage(options, (err, image, stats = {}) => { - req.profiler.done(`render-${format}`); req.profiler.add(stats); if (err) { @@ -133,7 +132,6 @@ function getPreviewImageByBoundingBox (previewBackend) { const options = { mapConfigProvider, format, width, height, bbox }; previewBackend.getImage(options, (err, image, stats = {}) => { - req.profiler.done(`render-${format}`); req.profiler.add(stats); if (err) { diff --git a/lib/api/map/preview-template-controller.js b/lib/api/map/preview-template-controller.js index 49ee18c7..287c0122 100644 --- a/lib/api/map/preview-template-controller.js +++ b/lib/api/map/preview-template-controller.js @@ -292,7 +292,7 @@ function getImage ({ previewBackend, label }) { if (zoom !== undefined && center) { const options = { mapConfigProvider, format, width, height, zoom, center }; - return previewBackend.getImage(options, (err, image, stats) => { + return previewBackend.getImage(options, (err, image, stats = {}) => { req.profiler.add(stats); if (err) { @@ -309,9 +309,8 @@ function getImage ({ previewBackend, label }) { const options = { mapConfigProvider, format, width, height, bbox }; - previewBackend.getImage(options, (err, image, stats) => { + previewBackend.getImage(options, (err, image, stats = {}) => { req.profiler.add(stats); - req.profiler.done('render-' + format); if (err) { err.label = label; diff --git a/lib/api/map/tile-layergroup-controller.js b/lib/api/map/tile-layergroup-controller.js index a067c2e6..5c5c468f 100644 --- a/lib/api/map/tile-layergroup-controller.js +++ b/lib/api/map/tile-layergroup-controller.js @@ -96,8 +96,6 @@ function getStatusCode (tile, format) { function getTile (tileBackend) { return function getTileMiddleware (req, res, next) { - req.profiler.start(`windshaft.${req.params.layer ? 'maplayer_tile' : 'map_tile'}`); - const { mapConfigProvider } = res.locals; const { token } = res.locals; const { layer, z, x, y, format } = req.params; diff --git a/lib/api/middlewares/authorize.js b/lib/api/middlewares/authorize.js index 506f0b77..82f20a12 100644 --- a/lib/api/middlewares/authorize.js +++ b/lib/api/middlewares/authorize.js @@ -3,8 +3,6 @@ module.exports = function authorize (authBackend) { return function authorizeMiddleware (req, res, next) { authBackend.authorize(req, res, (err, authorized) => { - req.profiler.done('authorize'); - if (err) { return next(err); } diff --git a/lib/api/middlewares/check-json-content-type.js b/lib/api/middlewares/check-json-content-type.js index 57d7b5a8..e894809c 100644 --- a/lib/api/middlewares/check-json-content-type.js +++ b/lib/api/middlewares/check-json-content-type.js @@ -6,8 +6,6 @@ module.exports = function checkJsonContentType () { return next(new Error('POST data must be of type application/json')); } - req.profiler.done('checkJsonContentTypeMiddleware'); - next(); }; }; diff --git a/lib/api/middlewares/db-conn-setup.js b/lib/api/middlewares/db-conn-setup.js index 7a7e76a8..a81e8ba0 100644 --- a/lib/api/middlewares/db-conn-setup.js +++ b/lib/api/middlewares/db-conn-setup.js @@ -7,8 +7,6 @@ module.exports = function dbConnSetup (pgConnection) { const { user } = res.locals; pgConnection.setDBConn(user, res.locals, (err) => { - req.profiler.done('dbConnSetup'); - if (err) { if (err.message && err.message.indexOf('name not found') !== -1) { err.http_status = 404; diff --git a/lib/api/middlewares/increment-map-view-count.js b/lib/api/middlewares/increment-map-view-count.js index ea4fc285..c226d1d6 100644 --- a/lib/api/middlewares/increment-map-view-count.js +++ b/lib/api/middlewares/increment-map-view-count.js @@ -11,8 +11,6 @@ module.exports = function incrementMapViewCount (metadataBackend) { // Error won't blow up, just be logged. metadataBackend.incMapviewCount(user, statTag, (err) => { - req.profiler.done('incMapviewCount'); - if (err) { err.message = `Failed to increment mapview count for user '${user}'. ${err.message}`; logger.warn({ error: err }); diff --git a/lib/api/middlewares/init-profiler.js b/lib/api/middlewares/init-profiler.js deleted file mode 100644 index 86916822..00000000 --- a/lib/api/middlewares/init-profiler.js +++ /dev/null @@ -1,11 +0,0 @@ -'use strict'; - -module.exports = function initProfiler (isTemplateInstantiation) { - const operation = isTemplateInstantiation ? 'instance_template' : 'createmap'; - - return function initProfilerMiddleware (req, res, next) { - req.profiler.start(`windshaft-cartodb.${operation}_${req.method.toLowerCase()}`); - req.profiler.done(`${operation}.initProfilerMiddleware`); - next(); - }; -}; diff --git a/lib/api/middlewares/lzma.js b/lib/api/middlewares/lzma.js index b47c3b28..7e8d8051 100644 --- a/lib/api/middlewares/lzma.js +++ b/lib/api/middlewares/lzma.js @@ -24,8 +24,6 @@ module.exports = function lzma () { delete req.query.lzma; Object.assign(req.query, JSON.parse(result)); - req.profiler.done('lzma'); - next(); } catch (err) { next(new Error('Error parsing lzma as JSON: ' + err)); diff --git a/lib/api/middlewares/map-error.js b/lib/api/middlewares/map-error.js index fcc5ef2a..b3c915ee 100644 --- a/lib/api/middlewares/map-error.js +++ b/lib/api/middlewares/map-error.js @@ -4,7 +4,6 @@ module.exports = function mapError (options) { const { addContext = false, label = 'MAPS CONTROLLER' } = options; return function mapErrorMiddleware (err, req, res, next) { - req.profiler.done('error'); const { mapConfig } = res.locals; if (addContext) { diff --git a/lib/api/middlewares/profiler.js b/lib/api/middlewares/profiler.js index 9ed3eb7b..2cf0736f 100644 --- a/lib/api/middlewares/profiler.js +++ b/lib/api/middlewares/profiler.js @@ -8,13 +8,17 @@ module.exports = function profiler (options) { return function profilerMiddleware (req, res, next) { const { logger } = res.locals; + const { id } = logger.bindings(); req.profiler = new Profiler({ statsd_client: statsClient, profile: enabled }); + req.profiler.start(id); + res.on('finish', () => { + req.profiler.done('response'); logger.info({ stats: req.profiler.toJSON() }); try { diff --git a/lib/api/middlewares/send-response.js b/lib/api/middlewares/send-response.js index f37457d6..d65d7597 100644 --- a/lib/api/middlewares/send-response.js +++ b/lib/api/middlewares/send-response.js @@ -2,8 +2,6 @@ module.exports = function sendResponse () { return function sendResponseMiddleware (req, res, next) { - req.profiler.done('res'); - res.status(res.statusCode); if (Buffer.isBuffer(res.body)) { diff --git a/lib/api/template/admin-template-controller.js b/lib/api/template/admin-template-controller.js index 456f6bc0..1a9e90e2 100644 --- a/lib/api/template/admin-template-controller.js +++ b/lib/api/template/admin-template-controller.js @@ -166,8 +166,6 @@ function updateTemplate ({ templateMaps }) { function retrieveTemplate ({ templateMaps }) { return function retrieveTemplateMiddleware (req, res, next) { - req.profiler.start('windshaft-cartodb.get_template'); - const { user } = res.locals; const templateId = templateName(req.params.template_id); @@ -195,8 +193,6 @@ function retrieveTemplate ({ templateMaps }) { function destroyTemplate ({ templateMaps }) { return function destroyTemplateMiddleware (req, res, next) { - req.profiler.start('windshaft-cartodb.delete_template'); - const { user } = res.locals; const templateId = templateName(req.params.template_id); @@ -215,8 +211,6 @@ function destroyTemplate ({ templateMaps }) { function listTemplates ({ templateMaps }) { return function listTemplatesMiddleware (req, res, next) { - req.profiler.start('windshaft-cartodb.get_template_list'); - const { user } = res.locals; templateMaps.listTemplates(user, (err, templateIds) => { diff --git a/lib/api/template/named-template-controller.js b/lib/api/template/named-template-controller.js index 79b39d7c..41246d65 100644 --- a/lib/api/template/named-template-controller.js +++ b/lib/api/template/named-template-controller.js @@ -4,7 +4,6 @@ const cleanUpQueryParams = require('../middlewares/clean-up-query-params'); const credentials = require('../middlewares/credentials'); const dbConnSetup = require('../middlewares/db-conn-setup'); const authorize = require('../middlewares/authorize'); -const initProfiler = require('../middlewares/init-profiler'); const checkJsonContentType = require('../middlewares/check-json-content-type'); const incrementMapViewCount = require('../middlewares/increment-map-view-count'); const augmentLayergroupData = require('../middlewares/augment-layergroup-data'); @@ -74,7 +73,6 @@ module.exports = class NamedMapController { } middlewares () { - const isTemplateInstantiation = true; const useTemplateHash = true; const includeQuery = false; const label = 'NAMED MAP LAYERGROUP'; @@ -100,7 +98,6 @@ module.exports = class NamedMapController { dbConnSetup(this.pgConnection), rateLimit(this.userLimitsBackend, RATE_LIMIT_ENDPOINTS_GROUPS.NAMED), cleanUpQueryParams(['aggregation']), - initProfiler(isTemplateInstantiation), checkJsonContentType(), checkInstantiteLayergroup(), getTemplate( @@ -150,8 +147,6 @@ function checkInstantiteLayergroup () { } } - req.profiler.done('checkInstantiteLayergroup'); - return next(); }; } @@ -187,9 +182,6 @@ function getTemplate ( ); mapConfigProvider.getMapConfig((err, mapConfig, rendererParams, context, stats = {}) => { - req.profiler.done('named.getMapConfig'); - - stats.mapType = 'named'; req.profiler.add(stats); if (err) { diff --git a/lib/api/template/tile-template-controller.js b/lib/api/template/tile-template-controller.js index 1112678e..bb2b0aa7 100644 --- a/lib/api/template/tile-template-controller.js +++ b/lib/api/template/tile-template-controller.js @@ -67,9 +67,8 @@ function getTile ({ tileBackend, label }) { const { layer, z, x, y, format } = req.params; const params = { layer, z, x, y, format }; - tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats) => { + tileBackend.getTile(mapConfigProvider, params, (err, tile, headers, stats = {}) => { req.profiler.add(stats); - req.profiler.done('render-' + format); if (err) { err.label = label; diff --git a/lib/backends/auth.js b/lib/backends/auth.js index 2c9dd3ee..298b1d64 100644 --- a/lib/backends/auth.js +++ b/lib/backends/auth.js @@ -133,8 +133,6 @@ AuthBackend.prototype.authorize = function (req, res, callback) { if (isAuthorizedByApikey) { return this.pgConnection.setDBAuth(user, res.locals, 'regular', function (err) { - req.profiler.done('setDBAuth'); - if (err) { return callback(err); } @@ -150,8 +148,6 @@ AuthBackend.prototype.authorize = function (req, res, callback) { if (isAuthorizedBySigner) { return this.pgConnection.setDBAuth(user, res.locals, 'master', function (err) { - req.profiler.done('setDBAuth'); - if (err) { return callback(err); } @@ -163,8 +159,6 @@ AuthBackend.prototype.authorize = function (req, res, callback) { // if no signer name was given, use default api key if (!res.locals.signer) { return this.pgConnection.setDBAuth(user, res.locals, 'default', function (err) { - req.profiler.done('setDBAuth'); - if (err) { return callback(err); } diff --git a/lib/models/dataview/base.js b/lib/models/dataview/base.js index 0813e3b1..18897635 100644 --- a/lib/models/dataview/base.js +++ b/lib/models/dataview/base.js @@ -23,7 +23,7 @@ function getPGTypeName (pgType) { module.exports = class BaseDataview { getResult (psql, override, callback) { - this.sql(psql, override, (err, query, flags = null) => { + this.sql(psql, override, (err, query) => { if (err) { return callback(err); } @@ -36,20 +36,7 @@ module.exports = class BaseDataview { result = this.format(result, override); result.type = this.getType(); - // Overviews logging - const stats = {}; - - if (flags && flags.usesOverviews !== undefined) { - stats.usesOverviews = flags.usesOverviews; - } else { - stats.usesOverviews = false; - } - - if (this.getType) { - stats.dataviewType = this.getType(); - } - - return callback(null, result, stats); + return callback(null, result); }, true); // use read-only transaction }); } diff --git a/lib/models/dataview/overviews/aggregation.js b/lib/models/dataview/overviews/aggregation.js index da988731..2ab6add2 100644 --- a/lib/models/dataview/overviews/aggregation.js +++ b/lib/models/dataview/overviews/aggregation.js @@ -213,7 +213,7 @@ Aggregation.prototype.sql = function (psql, override, callback) { debug(aggregationSql); - return callback(null, aggregationSql, { usesOverviews: true }); + return callback(null, aggregationSql); }; var aggregationFnQueryTpl = { diff --git a/lib/models/dataview/overviews/formula.js b/lib/models/dataview/overviews/formula.js index 35d52008..e860f8ec 100644 --- a/lib/models/dataview/overviews/formula.js +++ b/lib/models/dataview/overviews/formula.js @@ -76,5 +76,5 @@ Formula.prototype.sql = function (psql, override, callback) { debug(formulaSql); - return callback(null, formulaSql, { usesOverviews: true }); + return callback(null, formulaSql); }; diff --git a/lib/models/dataview/overviews/histogram.js b/lib/models/dataview/overviews/histogram.js index 4df5bd01..d10c1ae7 100644 --- a/lib/models/dataview/overviews/histogram.js +++ b/lib/models/dataview/overviews/histogram.js @@ -179,7 +179,7 @@ Histogram.prototype.sql = function (psql, override, callback) { var histogramSql = this._buildQuery(override); - return callback(null, histogramSql, { usesOverviews: true }); + return callback(null, histogramSql); }; Histogram.prototype._buildQuery = function (override) { diff --git a/lib/models/mapconfig/adapter/mapconfig-overviews-adapter.js b/lib/models/mapconfig/adapter/mapconfig-overviews-adapter.js index 3cb39a50..6473c162 100644 --- a/lib/models/mapconfig/adapter/mapconfig-overviews-adapter.js +++ b/lib/models/mapconfig/adapter/mapconfig-overviews-adapter.js @@ -25,58 +25,50 @@ MapConfigOverviewsAdapter.prototype.getMapConfig = function (user, requestMapCon layers.forEach(layer => augmentLayersQueue.defer(this._augmentLayer.bind(this), user, layer, analysesResults)); - augmentLayersQueue.awaitAll(function layersAugmentQueueFinish (err, results) { + augmentLayersQueue.awaitAll(function layersAugmentQueueFinish (err, layers) { if (err) { return callback(err); } - const layers = results.map(result => result.layer); - const overviewsAddedToMapconfig = results.some(result => result.overviewsAddedToMapconfig); - if (!layers || layers.length === 0) { return callback(new Error('Missing layers array from layergroup config')); } requestMapConfig.layers = layers; - const stats = { overviewsAddedToMapconfig }; - - return callback(null, requestMapConfig, stats); + return callback(null, requestMapConfig); }); }; MapConfigOverviewsAdapter.prototype._augmentLayer = function (user, layer, analysesResults, callback) { - let overviewsAddedToMapconfig = false; if (layer.type !== 'mapnik' && layer.type !== 'cartodb') { - return callback(null, { layer, overviewsAddedToMapconfig }); + return callback(null, layer); } this.overviewsMetadataBackend.getOverviewsMetadata(user, layer.options.sql, (err, metadata) => { if (err) { - return callback(err, { layer, overviewsAddedToMapconfig }); + return callback(err); } if (_.isEmpty(metadata)) { - return callback(null, { layer, overviewsAddedToMapconfig }); + return callback(null, layer); } var filters = getFilters(analysesResults, layer); - overviewsAddedToMapconfig = true; - if (!filters) { layer.options = Object.assign({}, layer.options, getQueryRewriteData(layer, analysesResults, { overviews: metadata })); - return callback(null, { layer, overviewsAddedToMapconfig }); + return callback(null, layer); } var unfilteredQuery = getUnfilteredQuery(analysesResults, layer); this.filterStatsBackend.getFilterStats(user, unfilteredQuery, filters, function (err, stats) { if (err) { - return callback(null, { layer, overviewsAddedToMapconfig }); + return callback(null, layer); } layer.options = Object.assign({}, layer.options, getQueryRewriteData(layer, analysesResults, { @@ -84,7 +76,7 @@ MapConfigOverviewsAdapter.prototype._augmentLayer = function (user, layer, analy filter_stats: stats })); - return callback(null, { layer, overviewsAddedToMapconfig }); + return callback(null, layer); }); }); };