diff --git a/.jshintrc b/.jshintrc index e5c5749ea..6b5af2b22 100644 --- a/.jshintrc +++ b/.jshintrc @@ -2,7 +2,7 @@ "node": true, "curly": true, "eqeqeq": true, - "esversion": 6, + "esversion": 9, "freeze": true, "immed": true, "indent": 2, diff --git a/controller/search.js b/controller/search.js index d949ef5e1..f8e896d91 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,4 +1,5 @@ const _ = require('lodash'); +const querystring = require('querystring'); const searchService = require('../service/search'); const logger = require('pelias-logger').get('api'); @@ -10,7 +11,9 @@ function isRequestTimeout(err) { return _.get(err, 'status') === 408; } -function setup( apiConfig, esclient, query, should_execute ){ +function setup( peliasConfig, esclient, query, should_execute ){ + const apiConfig = (peliasConfig || {}).api || {}; + function controller( req, res, next ){ if (!should_execute(req, res)) { return next(); @@ -57,8 +60,17 @@ function setup( apiConfig, esclient, query, should_execute ){ cmd.explain = true; } - logger.debug( '[ES req]', cmd ); - debugLog.push(req, {ES_req: cmd}); + const debugUrl = req.clean.exposeInternalDebugTools ? + `${peliasConfig.esclient.hosts[0]}/${apiConfig.indexName}/_search?` + + querystring.stringify({ + source_content_type: 'application/json', + source: JSON.stringify(cmd.body) + }) : undefined; + + debugLog.push(req, { + debugUrl, + ES_req: cmd + }); operation.attempt((currentAttempt) => { const initialTime = debugLog.beginTimer(req, `Attempt ${currentAttempt}`); diff --git a/middleware/interpolate.js b/middleware/interpolate.js index 7cfdeb7fe..a879d0b35 100644 --- a/middleware/interpolate.js +++ b/middleware/interpolate.js @@ -32,6 +32,7 @@ example response from interpolation web service: function error_intercepting_service(service, req) { return (street_result, next) => { service(req, street_result, (err, interpolation_result, metadata) => { + if (err) { logger.error(`[middleware:interpolation] ${_.defaultTo(err.message, err)}`); // now that the error has been caught and reported, act as if there was no error @@ -49,7 +50,7 @@ function error_intercepting_service(service, req) { }; } -function setup(service, should_execute) { +function setup(service, should_execute, interpolationConfiguration) { return function controller(req, res, next) { // bail early if the service shouldn't execute if (!should_execute(req, res)) { @@ -62,7 +63,6 @@ function setup(service, should_execute) { const street_results = _.get(res, 'data', []).filter(result => result.layer === 'street'); // perform interpolations asynchronously for all relevant hits - const start = (new Date()).getTime(); const initialTime = debugLog.beginTimer(req); const startTime = Date.now(); @@ -86,11 +86,13 @@ function setup(service, should_execute) { response_time: _.get(interpolation_result, 'metadata.response_time') }; + const internalDebug = interpolationConfiguration ? interpolationConfiguration.getQueryDebug(req, source_result) : {}; + // invalid / not useful response, debug log for posterity // note: leave this hit unmodified if (!_.has(interpolation_result, 'properties')) { resultLogInfo.outcome = 'miss'; - debugLog.push(req, 'miss'); + debugLog.push(req, {interpolation_result: 'miss', ...internalDebug}); logger.info('interpolation response', resultLogInfo); return; } @@ -98,7 +100,7 @@ function setup(service, should_execute) { // the interpolation service returned a valid result // we now merge those values with the existing 'street' record logger.info('interpolation response', resultLogInfo); - debugLog.push(req, interpolation_result); + debugLog.push(req, {interpolation_result, ...internalDebug}); // -- metadata -- source_result.layer = 'address'; diff --git a/package.json b/package.json index 5764539dc..d0ce18892 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "lodash": "^4.17.4", "markdown": "^0.5.0", "morgan": "^1.8.2", - "pelias-compare": "^0.1.5", + "pelias-compare": "^0.1.16", "pelias-config": "^4.0.0", "pelias-labels": "^1.8.0", "pelias-logger": "^1.2.0", diff --git a/routes/v1.js b/routes/v1.js index bc86042c5..5ec75b587 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -211,15 +211,15 @@ function addRoutes(app, peliasConfig) { controllers.placeholder(placeholderService, geometricFiltersApply, placeholderGeodisambiguationShouldExecute), controllers.placeholder(placeholderService, geometricFiltersApply, placeholderIdsLookupShouldExecute), // try 3 different query types: address search using ids, cascading fallback, pelias parser - controllers.search(peliasConfig.api, esclient, queries.address_search_using_ids, searchWithIdsShouldExecute), - controllers.search(peliasConfig.api, esclient, queries.search, fallbackQueryShouldExecute), + controllers.search(peliasConfig, esclient, queries.address_search_using_ids, searchWithIdsShouldExecute), + controllers.search(peliasConfig, esclient, queries.search, fallbackQueryShouldExecute), sanitizers.defer_to_pelias_parser(peliasConfig.api, shouldDeferToPeliasParser), //run additional sanitizers needed for pelias parser - controllers.search(peliasConfig.api, esclient, queries.search_pelias_parser, searchPeliasParserShouldExecute), + controllers.search(peliasConfig, esclient, queries.search_pelias_parser, searchPeliasParserShouldExecute), middleware.trimByGranularity(), middleware.distance('focus.point.'), middleware.confidenceScore(peliasConfig.api), middleware.confidenceScoreFallback(), - middleware.interpolate(interpolationService, interpolationShouldExecute), + middleware.interpolate(interpolationService, interpolationShouldExecute, interpolationConfiguration), middleware.sortResponseData(sorting, predicates.hasAdminOnlyResults), middleware.dedupe(), middleware.accuracy(), @@ -237,12 +237,12 @@ function addRoutes(app, peliasConfig) { middleware.requestLanguage, middleware.sizeCalculator(), controllers.structured_libpostal(structuredLibpostalService, structuredLibpostalShouldExecute), - controllers.search(peliasConfig.api, esclient, queries.structured_geocoding, not(hasResponseDataOrRequestErrors)), + controllers.search(peliasConfig, esclient, queries.structured_geocoding, not(hasResponseDataOrRequestErrors)), middleware.trimByGranularityStructured(), middleware.distance('focus.point.'), middleware.confidenceScore(peliasConfig.api), middleware.confidenceScoreFallback(), - middleware.interpolate(interpolationService, interpolationShouldExecute), + middleware.interpolate(interpolationService, interpolationShouldExecute, interpolationConfiguration), middleware.dedupe(), middleware.accuracy(), middleware.localNamingConventions(), @@ -258,7 +258,7 @@ function addRoutes(app, peliasConfig) { sanitizers.autocomplete.middleware(peliasConfig.api), middleware.requestLanguage, middleware.sizeCalculator(), - controllers.search(peliasConfig.api, esclient, queries.autocomplete, not(hasResponseDataOrRequestErrors)), + controllers.search(peliasConfig, esclient, queries.autocomplete, not(hasResponseDataOrRequestErrors)), middleware.distance('focus.point.'), middleware.confidenceScore(peliasConfig.api), middleware.dedupe(), @@ -276,7 +276,7 @@ function addRoutes(app, peliasConfig) { sanitizers.reverse.middleware(peliasConfig.api), middleware.requestLanguage, middleware.sizeCalculator(2), - controllers.search(peliasConfig.api, esclient, queries.reverse, nonCoarseReverseShouldExecute), + controllers.search(peliasConfig, esclient, queries.reverse, nonCoarseReverseShouldExecute), controllers.coarse_reverse(pipService, coarseReverseShouldExecute), middleware.distance('point.'), // reverse confidence scoring depends on distance from origin @@ -297,7 +297,7 @@ function addRoutes(app, peliasConfig) { sanitizers.nearby.middleware(peliasConfig.api), middleware.requestLanguage, middleware.sizeCalculator(), - controllers.search(peliasConfig.api, esclient, queries.reverse, not(hasResponseDataOrRequestErrors)), + controllers.search(peliasConfig, esclient, queries.reverse, not(hasResponseDataOrRequestErrors)), middleware.distance('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first diff --git a/sanitizer/_debug.js b/sanitizer/_debug.js index b6a7f5b9e..673540d36 100644 --- a/sanitizer/_debug.js +++ b/sanitizer/_debug.js @@ -13,6 +13,7 @@ function _setup(exposeInternalDebugTools) { } clean.enableDebug = false; + clean.exposeInternalDebugTools = exposeInternalDebugTools || false; if (_.isEqual(raw.debug, {})) { return messages; diff --git a/service/configurations/Interpolation.js b/service/configurations/Interpolation.js index be9e1f0ed..a7e63aba8 100644 --- a/service/configurations/Interpolation.js +++ b/service/configurations/Interpolation.js @@ -2,6 +2,7 @@ const url = require('url'); const _ = require('lodash'); const Debug = require('../../helper/debug'); const debugLog = new Debug('interpolation:request'); +const querystring = require('querystring'); const ServiceConfiguration = require('pelias-microservice-wrapper').ServiceConfiguration; @@ -18,15 +19,26 @@ class Interpolation extends ServiceConfiguration { lon: hit.center_point.lon }; - debugLog.push(req, params); - return params; } - getUrl(req) { + getUrl(_req) { return url.resolve(this.baseUrl, 'search/geojson'); } + getQueryDebug(req, hit) { + const params = this.getParameters(req, hit); + + if (req.clean.exposeInternalDebugTools) { + const table = url.resolve(this.baseUrl, 'extract/table') + '?' + querystring.stringify({...params, names: params.street}); + const raw = this.getUrl() + '?' + querystring.stringify(params); + const demo = url.resolve(this.baseUrl, 'demo') + `/#16/${hit.center_point.lat}/${hit.center_point.lon}` + + '?' + querystring.stringify({name: params.street}); + return { links: {table, raw, demo}, params }; + } else { + return { params }; + } + } } module.exports = Interpolation; diff --git a/test/unit/controller/search.js b/test/unit/controller/search.js index c3aa22043..66d903450 100644 --- a/test/unit/controller/search.js +++ b/test/unit/controller/search.js @@ -15,7 +15,9 @@ module.exports.tests.interface = function(test, common) { module.exports.tests.success = function(test, common) { test('successful request to search service should set data and meta', (t) => { const config = { - indexName: 'indexName value' + api: { + indexName: 'indexName value' + } }; const esclient = 'this is the esclient'; const query = () => { @@ -77,7 +79,9 @@ module.exports.tests.success = function(test, common) { test('undefined meta should set empty object into res', (t) => { const config = { - indexName: 'indexName value' + api: { + indexName: 'indexName value' + } }; const esclient = 'this is the esclient'; const query = () => { @@ -138,7 +142,9 @@ module.exports.tests.success = function(test, common) { test('undefined docs should log 0 results', (t) => { const config = { - indexName: 'indexName value' + api: { + indexName: 'indexName value' + } }; const esclient = 'this is the esclient'; const query = () => { @@ -204,7 +210,9 @@ module.exports.tests.success = function(test, common) { test('successful request on retry to search service should log info message', (t) => { const config = { - indexName: 'indexName value' + api: { + indexName: 'indexName value' + } }; const esclient = 'this is the esclient'; const query = () => { @@ -288,7 +296,9 @@ module.exports.tests.success = function(test, common) { module.exports.tests.timeout = function(test, common) { test('default # of request timeout retries should be 3', (t) => { const config = { - indexName: 'indexName value' + api: { + indexName: 'indexName value' + } }; const esclient = 'this is the esclient'; const query = () => { @@ -359,8 +369,10 @@ module.exports.tests.timeout = function(test, common) { test('explicit apiConfig.requestRetries should retry that many times', (t) => { const config = { - indexName: 'indexName value', - requestRetries: 17 + api: { + indexName: 'indexName value', + requestRetries: 17 + } }; const esclient = 'this is the esclient'; const query = () => { @@ -444,8 +456,10 @@ module.exports.tests.timeout = function(test, common) { test('string error should not retry and be logged as-is', (t) => { const config = { - indexName: 'indexName value', - requestRetries: 17 + api: { + indexName: 'indexName value', + requestRetries: 17 + } }; const esclient = 'this is the esclient'; const query = () => { diff --git a/test/unit/sanitizer/_debug.js b/test/unit/sanitizer/_debug.js index f193e2fed..620ea9215 100644 --- a/test/unit/sanitizer/_debug.js +++ b/test/unit/sanitizer/_debug.js @@ -7,7 +7,7 @@ module.exports.tests.sanitize_debug = function(test, common) { test(`debug flag is on: ${value}`, function(t) { const raw = { debug: value }; const clean = {}; - const expected_clean = { enableDebug: true }; + const expected_clean = { enableDebug: true, exposeInternalDebugTools: false }; const messages = sanitizer.sanitize(raw, clean); @@ -23,7 +23,7 @@ module.exports.tests.sanitize_debug = function(test, common) { const internalSanitizer = require('../../../sanitizer/_debug')(true); const raw = { debug: value }; const clean = {}; - const expected_clean = { enableDebug: true, enableElasticDebug: true }; + const expected_clean = { enableDebug: true, enableElasticDebug: true, exposeInternalDebugTools: true }; const messages = internalSanitizer.sanitize(raw, clean); @@ -36,7 +36,7 @@ module.exports.tests.sanitize_debug = function(test, common) { test(`debug flag is on: ${value} and exposeInternalDebugTools unset`, function(t) { const raw = { debug: value }; const clean = {}; - const expected_clean = { enableDebug: false }; + const expected_clean = { enableDebug: false, exposeInternalDebugTools: false }; const messages = sanitizer.sanitize(raw, clean); @@ -52,7 +52,7 @@ module.exports.tests.sanitize_debug = function(test, common) { const internalSanitizer = require('../../../sanitizer/_debug')(true); const raw = { debug: value }; const clean = {}; - const expected_clean = { enableDebug: true, enableElasticDebug: true, enableElasticExplain: true }; + const expected_clean = { enableDebug: true, enableElasticDebug: true, enableElasticExplain: true, exposeInternalDebugTools: true }; const messages = internalSanitizer.sanitize(raw, clean); @@ -65,7 +65,7 @@ module.exports.tests.sanitize_debug = function(test, common) { test(`debug flag is on: ${value} and exposeInternalDebugTools unset`, function(t) { const raw = { debug: value }; const clean = {}; - const expected_clean = { enableDebug: false }; + const expected_clean = { enableDebug: false, exposeInternalDebugTools: false }; const messages = sanitizer.sanitize(raw, clean); @@ -80,7 +80,7 @@ module.exports.tests.sanitize_debug = function(test, common) { test(`non-truthy values should set clean.debug to false: ${value}`, function(t) { const raw = { debug: value }; const clean = {}; - const expected_clean = { enableDebug: false }; + const expected_clean = { enableDebug: false, exposeInternalDebugTools: false }; const messages = sanitizer.sanitize(raw, clean); @@ -94,7 +94,7 @@ module.exports.tests.sanitize_debug = function(test, common) { test(`unknown value shold return error`, function(t) { const raw = { debug: 'value' }; const clean = {}; - const expected_clean = { enableDebug: false }; + const expected_clean = { enableDebug: false, exposeInternalDebugTools: false }; const messages = sanitizer.sanitize(raw, clean);