From 797e773bb35bb9594884c776e46b94fdf8db2478 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Mon, 6 Jul 2026 16:21:39 +0200 Subject: [PATCH 1/3] Prettify files touched by this change --- lib/utilities/reportHandler.js | 432 +++++++++++--------- tests/functional/utilities/reportHandler.js | 183 +++++---- 2 files changed, 337 insertions(+), 278 deletions(-) diff --git a/lib/utilities/reportHandler.js b/lib/utilities/reportHandler.js index c468d20c4b..f886916302 100644 --- a/lib/utilities/reportHandler.js +++ b/lib/utilities/reportHandler.js @@ -57,7 +57,7 @@ function getCapabilities(cfg = config) { // Consistency & safety checks for capabilities that depend on other config values const localVolumeCap = process.env.LOCAL_VOLUME_CAPABILITY || 'true'; - caps.locationTypeLocal &&= (localVolumeCap === '1' || localVolumeCap.toLowerCase() === 'true'); + caps.locationTypeLocal &&= localVolumeCap === '1' || localVolumeCap.toLowerCase() === 'true'; caps.secureChannelOptimizedPath &&= hasWSOptionalDependencies(); caps.managedLifecycle &&= cfg.supportedLifecycleRules.includes('Expiration'); caps.managedLifecycleTransition &&= cfg.supportedLifecycleRules.includes('Transition'); @@ -87,8 +87,10 @@ function cleanup(obj) { } function isAuthorized(clientIP, req) { - return ipCheck.ipMatchCidrList(config.healthChecks.allowFrom, clientIP) && - req.headers['x-scal-report-token'] === config.reportToken; + return ( + ipCheck.ipMatchCidrList(config.healthChecks.allowFrom, clientIP) && + req.headers['x-scal-report-token'] === config.reportToken + ); } function getGitVersion(cb) { @@ -107,22 +109,28 @@ function getSystemStats() { const cpuInfo = os.cpus(); const model = cpuInfo[0].model; const speed = cpuInfo[0].speed; - const times = cpuInfo. - map(c => c.times). - reduce((prev, cur) => - Object.assign({}, { - user: prev.user + cur.user, - nice: prev.nice + cur.nice, - sys: prev.sys + cur.sys, - idle: prev.idle + cur.idle, - irq: prev.irq + cur.irq, - }), { + const times = cpuInfo + .map(c => c.times) + .reduce( + (prev, cur) => + Object.assign( + {}, + { + user: prev.user + cur.user, + nice: prev.nice + cur.nice, + sys: prev.sys + cur.sys, + idle: prev.idle + cur.idle, + irq: prev.irq + cur.irq, + }, + ), + { user: 0, nice: 0, sys: 0, idle: 0, irq: 0, - }); + }, + ); return { memory: { @@ -252,18 +260,19 @@ function _getMetricsByLocation(endpoint, sites, requestMethod, log, cb) { async.mapLimit( sites, ASYNCLIMIT, - (site, next) => requestMethod(endpoint, site, log, (err, res) => { - if (err) { - log.debug('Error in retrieving site metrics', { - method: '_getMetricsByLocation', - error: err, - site, - requestType: requestMethod.name, - }); - return next(null, { site, stats: {} }); - } - return next(null, { site, stats: res }); - }), + (site, next) => + requestMethod(endpoint, site, log, (err, res) => { + if (err) { + log.debug('Error in retrieving site metrics', { + method: '_getMetricsByLocation', + error: err, + site, + requestType: requestMethod.name, + }); + return next(null, { site, stats: {} }); + } + return next(null, { site, stats: res }); + }), (err, locStats) => { if (err) { log.error('failed to get stats for site', { @@ -278,7 +287,7 @@ function _getMetricsByLocation(endpoint, sites, requestMethod, log, cb) { retObj[locStat.site] = locStat.stats; }); return cb(null, retObj); - } + }, ); } @@ -286,19 +295,21 @@ function _getMetrics(sites, requestMethod, log, cb, _testConfig) { const conf = (_testConfig && _testConfig.backbeat) || config.backbeat; const { host, port } = conf; const endpoint = `http://${host}:${port}`; - return async.parallel({ - all: done => requestMethod(endpoint, 'all', log, done), - byLocation: done => _getMetricsByLocation(endpoint, sites, - requestMethod, log, done), - }, (err, res) => { - if (err) { - return cb(err); - } - const all = (res && res.all) || {}; - const byLocation = (res && res.byLocation) || {}; - const retObj = Object.assign({}, all, { byLocation }); - return cb(null, retObj); - }); + return async.parallel( + { + all: done => requestMethod(endpoint, 'all', log, done), + byLocation: done => _getMetricsByLocation(endpoint, sites, requestMethod, log, done), + }, + (err, res) => { + if (err) { + return cb(err); + } + const all = (res && res.all) || {}; + const byLocation = (res && res.byLocation) || {}; + const retObj = Object.assign({}, all, { byLocation }); + return cb(null, retObj); + }, + ); } function getCRRMetrics(log, cb, _testConfig) { @@ -307,58 +318,73 @@ function getCRRMetrics(log, cb, _testConfig) { }); const { replicationEndpoints } = _testConfig || config; const sites = replicationEndpoints.map(endpoint => endpoint.site); - return _getMetrics(sites, _crrMetricRequest, log, (err, retObj) => { - if (err) { - log.error('failed to get CRR stats', { - method: 'getCRRMetrics', - error: err, - }); - return cb(null, {}); - } - return cb(null, retObj); - }, _testConfig); + return _getMetrics( + sites, + _crrMetricRequest, + log, + (err, retObj) => { + if (err) { + log.error('failed to get CRR stats', { + method: 'getCRRMetrics', + error: err, + }); + return cb(null, {}); + } + return cb(null, retObj); + }, + _testConfig, + ); } function getIngestionMetrics(sites, log, cb, _testConfig) { log.debug('request Ingestion metrics from backbeat api', { method: 'getIngestionMetrics', }); - return _getMetrics(sites, _ingestionMetricRequest, log, (err, retObj) => { - if (err) { - log.error('failed to get Ingestion stats', { - method: 'getIngestionMetrics', - error: err, - }); - return cb(null, {}); - } - return cb(null, retObj); - }, _testConfig); + return _getMetrics( + sites, + _ingestionMetricRequest, + log, + (err, retObj) => { + if (err) { + log.error('failed to get Ingestion stats', { + method: 'getIngestionMetrics', + error: err, + }); + return cb(null, {}); + } + return cb(null, retObj); + }, + _testConfig, + ); } function _getStates(statusPath, schedulePath, log, cb, _testConfig) { const conf = (_testConfig && _testConfig.backbeat) || config.backbeat; const { host, port } = conf; const endpoint = `http://${host}:${port}`; - async.parallel({ - states: done => _makeRequest(endpoint, statusPath, done), - schedules: done => _makeRequest(endpoint, schedulePath, done), - }, (err, res) => { - if (err) { - return cb(err); - } - const locationSchedules = {}; - Object.keys(res.schedules).forEach(loc => { - const val = res.schedules[loc]; - if (!isNaN(Date.parse(val))) { - locationSchedules[loc] = new Date(val); + async.parallel( + { + states: done => _makeRequest(endpoint, statusPath, done), + schedules: done => _makeRequest(endpoint, schedulePath, done), + }, + (err, res) => { + if (err) { + return cb(err); } - }); - const retObj = { - states: res.states || {}, - schedules: locationSchedules, - }; - return cb(null, retObj); - }); + const locationSchedules = {}; + Object.keys(res.schedules).forEach(loc => { + const val = res.schedules[loc]; + if (!isNaN(Date.parse(val))) { + locationSchedules[loc] = new Date(val); + } + }); + const retObj = { + states: res.states || {}, + schedules: locationSchedules, + }; + return cb(null, retObj); + }, + ); } function getReplicationStates(log, cb, _testConfig) { @@ -366,25 +392,31 @@ function getReplicationStates(log, cb, _testConfig) { method: 'getReplicationStates', }); const { crrStatus, crrSchedules } = REQ_PATHS; - return _getStates(crrStatus, crrSchedules, log, (err, res) => { - if (err) { - if (err === 'responseError') { - log.error('error response from backbeat api', { - error: res, - method: 'getReplicationStates', - service: 'replication', - }); - } else { - log.error('unable to perform request to backbeat api', { - error: err, - method: 'getReplicationStates', - service: 'replication', - }); + return _getStates( + crrStatus, + crrSchedules, + log, + (err, res) => { + if (err) { + if (err === 'responseError') { + log.error('error response from backbeat api', { + error: res, + method: 'getReplicationStates', + service: 'replication', + }); + } else { + log.error('unable to perform request to backbeat api', { + error: err, + method: 'getReplicationStates', + service: 'replication', + }); + } + return cb(null, {}); } - return cb(null, {}); - } - return cb(null, res); - }, _testConfig); + return cb(null, res); + }, + _testConfig, + ); } function getIngestionStates(log, cb, _testConfig) { @@ -392,67 +424,81 @@ function getIngestionStates(log, cb, _testConfig) { method: 'getIngestionStates', }); const { ingestionStatus, ingestionSchedules } = REQ_PATHS; - return _getStates(ingestionStatus, ingestionSchedules, log, (err, res) => { - if (err) { - if (err === 'responseError') { - log.error('error response from backbeat api', { - error: res, - method: 'getIngestionStates', - service: 'ingestion', - }); - } else { - log.error('unable to perform request to backbeat api', { - error: err, - method: 'getIngestionStates', - service: 'ingestion', - }); + return _getStates( + ingestionStatus, + ingestionSchedules, + log, + (err, res) => { + if (err) { + if (err === 'responseError') { + log.error('error response from backbeat api', { + error: res, + method: 'getIngestionStates', + service: 'ingestion', + }); + } else { + log.error('unable to perform request to backbeat api', { + error: err, + method: 'getIngestionStates', + service: 'ingestion', + }); + } + return cb(null, {}); } - return cb(null, {}); - } - return cb(null, res); - }, _testConfig); + return cb(null, res); + }, + _testConfig, + ); } function getIngestionInfo(log, cb, _testConfig) { log.debug('requesting location ingestion info from backbeat api', { method: 'getIngestionInfo', }); - async.waterfall([ - done => getIngestionStates(log, done, _testConfig), - (stateObj, done) => { - // if getIngestionStates returned an error or the returned object - // did not return an expected response - if (Object.keys(stateObj).length === 0 || !stateObj.states) { - log.debug('no ingestion locations found', { - method: 'getIngestionInfo', - }); - return done(null, stateObj, {}); - } - const sites = Object.keys(stateObj.states); - return getIngestionMetrics(sites, log, (err, res) => { - if (err) { - log.error('failed to get Ingestion stats', { + async.waterfall( + [ + done => getIngestionStates(log, done, _testConfig), + (stateObj, done) => { + // if getIngestionStates returned an error or the returned object + // did not return an expected response + if (Object.keys(stateObj).length === 0 || !stateObj.states) { + log.debug('no ingestion locations found', { method: 'getIngestionInfo', - error: err, }); return done(null, stateObj, {}); } - return done(null, stateObj, res); - }, _testConfig); - }, - ], (err, stateObj, metricObj) => { - if (err) { - log.error('failed to get ingestion info', { - method: 'getIngestionInfo', - error: err, + const sites = Object.keys(stateObj.states); + return getIngestionMetrics( + sites, + log, + (err, res) => { + if (err) { + log.error('failed to get Ingestion stats', { + method: 'getIngestionInfo', + error: err, + }); + return done(null, stateObj, {}); + } + return done(null, stateObj, res); + }, + _testConfig, + ); + }, + ], + (err, stateObj, metricObj) => { + if (err) { + log.error('failed to get ingestion info', { + method: 'getIngestionInfo', + error: err, + }); + return cb(null, {}); + } + return cb(null, { + metrics: metricObj, + status: stateObj, }); - return cb(null, {}); - } - return cb(null, { - metrics: metricObj, - status: stateObj, - }); - }); + }, + ); } /** @@ -478,52 +524,54 @@ function reportHandler(clientIP, req, res, log) { } // TODO propagate value of req.headers['x-scal-report-skip-cache'] - async.parallel({ - getUUID: cb => metadata.getUUID(log, cb), - getMDDiskUsage: cb => metadata.getDiskUsage(log, cb), - getDataDiskUsage: cb => data.getDiskUsage(log, cb), - getVersion: cb => getGitVersion(cb), - getObjectCount: cb => metadata.countItems(log, cb), - getCRRMetrics: cb => getCRRMetrics(log, cb), - getReplicationStates: cb => getReplicationStates(log, cb), - getIngestionInfo: cb => getIngestionInfo(log, cb), - getVaultReport: cb => vault.report(log, cb), - }, - (err, results) => { - if (err) { - res.writeHead(500, { 'Content-Type': 'application/json' }); - res.write(JSON.stringify(err)); - log.errorEnd('could not gather report', { error: err }); - } else { - const getObjectCount = results.getObjectCount; - const crrStatsObj = Object.assign({}, results.getCRRMetrics); - crrStatsObj.stalled = { count: getObjectCount.stalled || 0 }; - delete getObjectCount.stalled; - const response = { - utcTime: new Date(), - uuid: results.getUUID, - reportModelVersion: REPORT_MODEL_VERSION, - - mdDiskUsage: results.getMDDiskUsage, - dataDiskUsage: results.getDataDiskUsage, - serverVersion: results.getVersion, - systemStats: getSystemStats(), - itemCounts: getObjectCount, - crrStats: crrStatsObj, - repStatus: results.getReplicationStates, - config: cleanup(config), - capabilities: getCapabilities(), - ingestStats: results.getIngestionInfo.metrics, - ingestStatus: results.getIngestionInfo.status, - vaultReport: results.getVaultReport, - }; - monitoring.crrCacheToProm(results); - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.write(JSON.stringify(response)); - log.end().debug('report handler finished'); - } - res.end(); - }); + async.parallel( + { + getUUID: cb => metadata.getUUID(log, cb), + getMDDiskUsage: cb => metadata.getDiskUsage(log, cb), + getDataDiskUsage: cb => data.getDiskUsage(log, cb), + getVersion: cb => getGitVersion(cb), + getObjectCount: cb => metadata.countItems(log, cb), + getCRRMetrics: cb => getCRRMetrics(log, cb), + getReplicationStates: cb => getReplicationStates(log, cb), + getIngestionInfo: cb => getIngestionInfo(log, cb), + getVaultReport: cb => vault.report(log, cb), + }, + (err, results) => { + if (err) { + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.write(JSON.stringify(err)); + log.errorEnd('could not gather report', { error: err }); + } else { + const getObjectCount = results.getObjectCount; + const crrStatsObj = Object.assign({}, results.getCRRMetrics); + crrStatsObj.stalled = { count: getObjectCount.stalled || 0 }; + delete getObjectCount.stalled; + const response = { + utcTime: new Date(), + uuid: results.getUUID, + reportModelVersion: REPORT_MODEL_VERSION, + + mdDiskUsage: results.getMDDiskUsage, + dataDiskUsage: results.getDataDiskUsage, + serverVersion: results.getVersion, + systemStats: getSystemStats(), + itemCounts: getObjectCount, + crrStats: crrStatsObj, + repStatus: results.getReplicationStates, + config: cleanup(config), + capabilities: getCapabilities(), + ingestStats: results.getIngestionInfo.metrics, + ingestStatus: results.getIngestionInfo.status, + vaultReport: results.getVaultReport, + }; + monitoring.crrCacheToProm(results); + res.writeHead(200, { 'Content-Type': 'application/json' }); + res.write(JSON.stringify(response)); + log.end().debug('report handler finished'); + } + res.end(); + }, + ); } module.exports = { diff --git a/tests/functional/utilities/reportHandler.js b/tests/functional/utilities/reportHandler.js index dc5948eeb7..fe49f8bb91 100644 --- a/tests/functional/utilities/reportHandler.js +++ b/tests/functional/utilities/reportHandler.js @@ -142,16 +142,16 @@ function requestHandler(req, res) { } } else { switch (req.url) { - case '/_/crr/status': - case '/_/ingestion/status': - res.write(JSON.stringify(expectedStatusResults)); - break; - case '/_/crr/resume/all': - case '/_/ingestion/resume/all': - res.write(JSON.stringify(expectedScheduleResults)); - break; - default: - break; + case '/_/crr/status': + case '/_/ingestion/status': + res.write(JSON.stringify(expectedStatusResults)); + break; + case '/_/crr/resume/all': + case '/_/ingestion/resume/all': + res.write(JSON.stringify(expectedScheduleResults)); + break; + default: + break; } } res.end(); @@ -168,8 +168,7 @@ function requestHandler(req, res) { describe('Test Request Failure Cases', () => { before(done => { - httpServer = http.createServer(requestFailHandler) - .listen(testPort); + httpServer = http.createServer(requestFailHandler).listen(testPort); httpServer.on('listening', done); httpServer.on('error', err => { process.stdout.write(`https server: ${err.stack}\n`); @@ -181,8 +180,7 @@ function requestHandler(req, res) { httpServer.close(); }); - it('should return empty object if a request error occurs', - done => { + it('should return empty object if a request error occurs', done => { const endpoint = 'http://nonexists:4242'; item.method(endpoint, 'all', logger, (err, res) => { assert.ifError(err); @@ -191,8 +189,7 @@ function requestHandler(req, res) { }); }); - it('should return empty object if response status code is >= 400', - done => { + it('should return empty object if response status code is >= 400', done => { const endpoint = 'http://localhost:4242'; item.method(endpoint, 'all', logger, (err, res) => { assert.ifError(err); @@ -205,8 +202,7 @@ function requestHandler(req, res) { describe('Test Request Success Cases', () => { const endpoint = 'http://localhost:4242'; before(done => { - httpServer = http.createServer(requestHandler) - .listen(testPort); + httpServer = http.createServer(requestHandler).listen(testPort); httpServer.on('listening', done); httpServer.on('error', err => { process.stdout.write(`https server: ${err.stack}\n`); @@ -221,8 +217,7 @@ function requestHandler(req, res) { it('should return correct location metrics', done => { item.method(endpoint, 'site1', logger, (err, res) => { assert.ifError(err); - assert.deepStrictEqual( - res, item.result.byLocation.site1); + assert.deepStrictEqual(res, item.result.byLocation.site1); done(); }); }); @@ -256,27 +251,33 @@ function requestHandler(req, res) { it('should return correct results', done => { if (item.method.name === 'getIngestionMetrics') { const sites = ['site1', 'site2']; - item.method(sites, logger, (err, res) => { - assert.ifError(err); - assert.deepStrictEqual(res, item.result); - done(); - }, config); + item.method( + sites, + logger, + (err, res) => { + assert.ifError(err); + assert.deepStrictEqual(res, item.result); + done(); + }, + config, + ); } else { - item.method(logger, (err, res) => { - assert.ifError(err); - assert.deepStrictEqual(res, item.result); - done(); - }, config); + item.method( + logger, + (err, res) => { + assert.ifError(err); + assert.deepStrictEqual(res, item.result); + done(); + }, + config, + ); } }); }); }); }); -[ - { method: getReplicationStates }, - { method: getIngestionStates }, -].forEach(item => { +[{ method: getReplicationStates }, { method: getIngestionStates }].forEach(item => { describe(`reportHandler::${item.method.name}`, function testSuite() { this.timeout(20000); const testPort = '4242'; @@ -284,8 +285,7 @@ function requestHandler(req, res) { describe('Test Request Failure Cases', () => { before(done => { - httpServer = http.createServer(requestFailHandler) - .listen(testPort); + httpServer = http.createServer(requestFailHandler).listen(testPort); httpServer.on('listening', done); httpServer.on('error', err => { process.stdout.write(`https server: ${err.stack}\n`); @@ -297,29 +297,34 @@ function requestHandler(req, res) { httpServer.close(); }); - it('should return empty object if a request error occurs', - done => { - item.method(logger, (err, res) => { - assert.ifError(err); - assert.deepStrictEqual(res, {}); - done(); - }, { backbeat: { host: 'nonexisthost', port: testPort } }); + it('should return empty object if a request error occurs', done => { + item.method( + logger, + (err, res) => { + assert.ifError(err); + assert.deepStrictEqual(res, {}); + done(); + }, + { backbeat: { host: 'nonexisthost', port: testPort } }, + ); }); - it('should return empty object if response status code is >= 400', - done => { - item.method(logger, (err, res) => { - assert.ifError(err); - assert.deepStrictEqual(res, {}); - done(); - }, { backbeat: { host: 'localhost', port: testPort } }); + it('should return empty object if response status code is >= 400', done => { + item.method( + logger, + (err, res) => { + assert.ifError(err); + assert.deepStrictEqual(res, {}); + done(); + }, + { backbeat: { host: 'localhost', port: testPort } }, + ); }); }); describe('Test Request Success Cases', () => { before(done => { - httpServer = http.createServer(requestHandler) - .listen(testPort); + httpServer = http.createServer(requestHandler).listen(testPort); httpServer.on('listening', done); httpServer.on('error', err => { process.stdout.write(`https server: ${err.stack}\n`); @@ -332,20 +337,24 @@ function requestHandler(req, res) { }); it('should return correct results', done => { - item.method(logger, (err, res) => { - const expectedResults = { - states: { - site1: 'enabled', - site2: 'disabled', - }, - schedules: { - site2: expectedScheduleResults.site2, - }, - }; - assert.ifError(err); - assert.deepStrictEqual(res, expectedResults); - done(); - }, { backbeat: { host: 'localhost', port: testPort } }); + item.method( + logger, + (err, res) => { + const expectedResults = { + states: { + site1: 'enabled', + site2: 'disabled', + }, + schedules: { + site2: expectedScheduleResults.site2, + }, + }; + assert.ifError(err); + assert.deepStrictEqual(res, expectedResults); + done(); + }, + { backbeat: { host: 'localhost', port: testPort } }, + ); }); }); }); @@ -358,8 +367,7 @@ describe('reportHanlder::getIngestionInfo', function testSuite() { describe('Test Request Success Cases', () => { before(done => { - httpServer = http.createServer(requestHandler) - .listen(testPort); + httpServer = http.createServer(requestHandler).listen(testPort); httpServer.on('listening', done); httpServer.on('error', err => { process.stdout.write(`https server: ${err.stack}\n`); @@ -372,25 +380,28 @@ describe('reportHanlder::getIngestionInfo', function testSuite() { }); it('should return correct results', done => { - getIngestionInfo(logger, (err, res) => { - const expectedStatusResults = { - states: { - site1: 'enabled', - site2: 'disabled', - }, - schedules: { - site2: expectedScheduleResults.site2, - }, - }; - assert.ifError(err); + getIngestionInfo( + logger, + (err, res) => { + const expectedStatusResults = { + states: { + site1: 'enabled', + site2: 'disabled', + }, + schedules: { + site2: expectedScheduleResults.site2, + }, + }; + assert.ifError(err); - assert(res.metrics); - assert(res.status); - assert.deepStrictEqual(res.status, expectedStatusResults); - assert.deepStrictEqual(res.metrics, - ingestionExpectedResultsRef); - done(); - }, config); + assert(res.metrics); + assert(res.status); + assert.deepStrictEqual(res.status, expectedStatusResults); + assert.deepStrictEqual(res.metrics, ingestionExpectedResultsRef); + done(); + }, + config, + ); }); it('should return empty if no ingestion locations exist', done => { From 881981a31b0cac1b511ca2c577400fbc6b9b71c0 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Mon, 6 Jul 2026 16:22:06 +0200 Subject: [PATCH 2/3] Forward X-Scal-Request-Uids on Backbeat report/CRR-metrics requests Propagate the werelogs request-uid chain (log.getSerializedUids()) on the Backbeat report / CRR-metrics HTTP calls made through reportHandler's _makeRequest, via the x-scal-request-uids header. This matches the header cloudserver already reads on incoming admin requests, so req_id continuity is preserved across the Backbeat metric and state/schedule calls. Issue: CLDSRV-947 --- lib/utilities/reportHandler.js | 41 ++++++++++++--------- tests/functional/utilities/reportHandler.js | 4 ++ 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/lib/utilities/reportHandler.js b/lib/utilities/reportHandler.js index f886916302..6104aacb5b 100644 --- a/lib/utilities/reportHandler.js +++ b/lib/utilities/reportHandler.js @@ -151,25 +151,32 @@ function getSystemStats() { }; } -const _makeRequest = (endpoint, path, cb) => { +const _makeRequest = (endpoint, path, log, cb) => { const url = `${endpoint}${path}`; - request.get(url, { json: true }, (error, response, body) => { - if (error) { - return cb(error); - } - if (response.statusCode >= 400) { - return cb('responseError', body); - } - if (body) { - return cb(null, body); - } - return cb(null, {}); - }); + request.get( + url, + { + json: true, + headers: { 'x-scal-request-uids': log.getSerializedUids() }, + }, + (error, response, body) => { + if (error) { + return cb(error); + } + if (response.statusCode >= 400) { + return cb('responseError', body); + } + if (body) { + return cb(null, body); + } + return cb(null, {}); + }, + ); }; function _crrMetricRequest(endpoint, site, log, cb) { const path = `${REQ_PATHS.crrMetricPrefix}/${site}`; - return _makeRequest(endpoint, path, (err, res) => { + return _makeRequest(endpoint, path, log, (err, res) => { if (err) { if (err === 'responseError') { log.error('error response from backbeat api', { @@ -219,7 +226,7 @@ function _crrMetricRequest(endpoint, site, log, cb) { function _ingestionMetricRequest(endpoint, site, log, cb) { const path = `${REQ_PATHS.ingestionMetricPrefix}/${site}`; - return _makeRequest(endpoint, path, (err, res) => { + return _makeRequest(endpoint, path, log, (err, res) => { if (err) { if (err === 'responseError') { log.error('error response from backbeat api', { @@ -364,8 +371,8 @@ function _getStates(statusPath, schedulePath, log, cb, _testConfig) { const endpoint = `http://${host}:${port}`; async.parallel( { - states: done => _makeRequest(endpoint, statusPath, done), - schedules: done => _makeRequest(endpoint, schedulePath, done), + states: done => _makeRequest(endpoint, statusPath, log, done), + schedules: done => _makeRequest(endpoint, schedulePath, log, done), }, (err, res) => { if (err) { diff --git a/tests/functional/utilities/reportHandler.js b/tests/functional/utilities/reportHandler.js index fe49f8bb91..ca7b24e264 100644 --- a/tests/functional/utilities/reportHandler.js +++ b/tests/functional/utilities/reportHandler.js @@ -131,11 +131,13 @@ function requestFailHandler(req, res) { function requestHandler(req, res) { const { url } = req; if (url.startsWith('/_/metrics/crr/')) { + assert.strictEqual(req.headers['x-scal-request-uids'], 'dummy:Serialized:Uids'); const site = url.split('/_/metrics/crr/')[1] || ''; if (crrRequestResults[site]) { res.write(JSON.stringify(crrRequestResults[site])); } } else if (url.startsWith('/_/metrics/ingestion/')) { + assert.strictEqual(req.headers['x-scal-request-uids'], 'dummy:Serialized:Uids'); const site = url.split('/_/metrics/ingestion/')[1] || ''; if (ingestionRequestResults[site]) { res.write(JSON.stringify(ingestionRequestResults[site])); @@ -144,10 +146,12 @@ function requestHandler(req, res) { switch (req.url) { case '/_/crr/status': case '/_/ingestion/status': + assert.strictEqual(req.headers['x-scal-request-uids'], 'dummy:Serialized:Uids'); res.write(JSON.stringify(expectedStatusResults)); break; case '/_/crr/resume/all': case '/_/ingestion/resume/all': + assert.strictEqual(req.headers['x-scal-request-uids'], 'dummy:Serialized:Uids'); res.write(JSON.stringify(expectedScheduleResults)); break; default: From ab2f16c98ac25faee2b72b58bc323bff563bf654 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Mon, 6 Jul 2026 16:46:58 +0200 Subject: [PATCH 3/3] Migrate _makeRequest to async/await Convert reportHandler's _makeRequest, whose signature this change modified, to an async function using the dual callback+async continuation trampoline: callers keep the callback contract while the body is async/await. This preempts the CodeQL js/callback-style-function alert on the function this PR touched. _makeRequest is an internal HTTP helper (endpoint, path, log, cb), not a CORS-bearing lib/api handler, so the trampoline omits the err.additionalResHeaders / collectCorsHeaders handling. request.get yields two success values (response, body), which util.promisify cannot express, so it is wrapped in a manual Promise. The (err, res) callback contract is preserved exactly, including the cb('responseError', body) sentinel path. The .then()/.catch() on the trampoline (eslint promise/prefer-await-to-then and CodeQL js/promise-then-usage) are the accepted trade-off of this pattern. Issue: CLDSRV-947 --- lib/utilities/reportHandler.js | 53 ++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/lib/utilities/reportHandler.js b/lib/utilities/reportHandler.js index 6104aacb5b..d9c3573ff1 100644 --- a/lib/utilities/reportHandler.js +++ b/lib/utilities/reportHandler.js @@ -151,28 +151,39 @@ function getSystemStats() { }; } -const _makeRequest = (endpoint, path, log, cb) => { +async function _makeRequest(endpoint, path, log, cb) { + if (typeof cb === 'function') { + return _makeRequest(endpoint, path, log) + .then(res => cb(null, res)) + .catch(err => (err.isResponseError ? cb('responseError', err.body) : cb(err))); + } const url = `${endpoint}${path}`; - request.get( - url, - { - json: true, - headers: { 'x-scal-request-uids': log.getSerializedUids() }, - }, - (error, response, body) => { - if (error) { - return cb(error); - } - if (response.statusCode >= 400) { - return cb('responseError', body); - } - if (body) { - return cb(null, body); - } - return cb(null, {}); - }, - ); -}; + const result = await new Promise((resolve, reject) => { + request.get( + url, + { + json: true, + headers: { 'x-scal-request-uids': log.getSerializedUids() }, + }, + (error, response, body) => { + if (error) { + return reject(error); + } + return resolve({ response, body }); + }, + ); + }); + if (result.response.statusCode >= 400) { + const err = new Error('responseError'); + err.isResponseError = true; + err.body = result.body; + throw err; + } + if (result.body) { + return result.body; + } + return {}; +} function _crrMetricRequest(endpoint, site, log, cb) { const path = `${REQ_PATHS.crrMetricPrefix}/${site}`;