Skip to content

Commit

Permalink
feat(debug): Add links to interpolation/elasticsearch
Browse files Browse the repository at this point in the history
  • Loading branch information
blackmad authored and orangejulius committed Jun 26, 2020
1 parent 56b6bdb commit 71b6f56
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 37 deletions.
2 changes: 1 addition & 1 deletion .jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"node": true,
"curly": true,
"eqeqeq": true,
"esversion": 6,
"esversion": 9,
"freeze": true,
"immed": true,
"indent": 2,
Expand Down
18 changes: 15 additions & 3 deletions controller/search.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const _ = require('lodash');
const querystring = require('querystring');

const searchService = require('../service/search');
const logger = require('pelias-logger').get('api');
Expand All @@ -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();
Expand Down Expand Up @@ -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}`);
Expand Down
10 changes: 6 additions & 4 deletions middleware/interpolate.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)) {
Expand All @@ -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();
Expand All @@ -86,19 +86,21 @@ 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;
}

// 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';
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 9 additions & 9 deletions routes/v1.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions sanitizer/_debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ function _setup(exposeInternalDebugTools) {
}

clean.enableDebug = false;
clean.exposeInternalDebugTools = exposeInternalDebugTools || false;

if (_.isEqual(raw.debug, {})) {
return messages;
Expand Down
18 changes: 15 additions & 3 deletions service/configurations/Interpolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
32 changes: 23 additions & 9 deletions test/unit/controller/search.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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 = () => {
Expand Down
14 changes: 7 additions & 7 deletions test/unit/sanitizer/_debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down

0 comments on commit 71b6f56

Please sign in to comment.