From 5cf64ae341a08fe7a6518da2d27d90a87cd84a5e Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Wed, 3 Jun 2026 16:11:43 +0200 Subject: [PATCH 1/4] chore: add OpenTelemetry dependencies and consume arsenal tracing module + update yarn.lock after cherry-pick Issue: CLDSRV-884 (cherry picked from commit a6f74d337f6384f60846d4506ddf4313b651db1b) --- package.json | 4 +++ yarn.lock | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index cb07ebb836..02907223aa 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,10 @@ "@aws-sdk/signature-v4": "^3.374.0", "@azure/storage-blob": "^12.28.0", "@hapi/joi": "^17.1.1", + "@opentelemetry/api": "^1.9.0", + "@opentelemetry/instrumentation-http": "~0.218.0", + "@opentelemetry/instrumentation-ioredis": "~0.64.0", + "@opentelemetry/instrumentation-mongodb": "~0.69.0", "@smithy/node-http-handler": "^3.0.0", "arsenal": "git+https://github.com/scality/Arsenal#8.4.24", "async": "2.6.4", diff --git a/yarn.lock b/yarn.lock index 9b0e3a73ce..2625270238 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3455,6 +3455,20 @@ dependencies: semver "^7.3.5" +"@opentelemetry/api-logs@0.216.0": + version "0.216.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/api-logs/-/api-logs-0.216.0.tgz#7aa4b485ea2f2e21ffcb94120136c72f718c2eaf" + integrity sha512-KmGTgvxTJ0J01d4mOeX1wMV5NUTNf9HebIuOOGDfIn0a/IrnXIQbOnlylDyl9tkDv4h0DUpdI/GqCdLzfTkUXg== + dependencies: + "@opentelemetry/api" "^1.3.0" + +"@opentelemetry/api-logs@0.218.0": + version "0.218.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/api-logs/-/api-logs-0.218.0.tgz#7b9818e8dfdf1d3dcab88bfe4d6724f2f831f7ec" + integrity sha512-fmEWp5kXlGEc3i/lR698Hz41DfGyN4Tbe4g7L1AxSc7fF8Xeh/FQ9Quqpa9dVA413Q1Ad43QOLzU4JoXgbFPWw== + dependencies: + "@opentelemetry/api" "^1.3.0" + "@opentelemetry/api-logs@0.219.0": version "0.219.0" resolved "https://registry.yarnpkg.com/@opentelemetry/api-logs/-/api-logs-0.219.0.tgz#3303f10f43e6ff1741f8f6019e43a92723781630" @@ -3462,7 +3476,7 @@ dependencies: "@opentelemetry/api" "^1.3.0" -"@opentelemetry/api@^1.3.0", "@opentelemetry/api@^1.9.1": +"@opentelemetry/api@^1.3.0", "@opentelemetry/api@^1.9.0", "@opentelemetry/api@^1.9.1": version "1.9.1" resolved "https://registry.yarnpkg.com/@opentelemetry/api/-/api-1.9.1.tgz#c1b0346de336ba55af2d5a7970882037baedec05" integrity sha512-gLyJlPHPZYdAk1JENA9LeHejZe1Ti77/pTeFm/nMXmQH/HFZlcS/O2XJB+L8fkbrNSqhdtlvjBVjxwUYanNH5Q== @@ -3485,6 +3499,13 @@ resolved "https://registry.yarnpkg.com/@opentelemetry/context-async-hooks/-/context-async-hooks-2.8.0.tgz#24381ef388bc28d53fa8dad72dfd1429acc82016" integrity sha512-/3FIraneMcng67SUJCxvyInk/oxzwsxyadufk0wwfOBLf5wqtAGX4MoQASwSbndBPeARzBryUM9Azr5kHIdWLw== +"@opentelemetry/core@2.7.1": + version "2.7.1" + resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-2.7.1.tgz#162bfab46d6ff4da1bef240ea52e23a926b0fdbc" + integrity sha512-QAqIj32AtK6+pEVNG7EOVxHdE06RP+FM5qpiEJ4RtDcFIqKUZHYhl7/7UY5efhwmwNAg7j8QbJVBLxMerc0+gw== + dependencies: + "@opentelemetry/semantic-conventions" "^1.29.0" + "@opentelemetry/core@2.8.0": version "2.8.0" resolved "https://registry.yarnpkg.com/@opentelemetry/core/-/core-2.8.0.tgz#f6e86de3688bdb54a6ca8f4935363a5b588ae91c" @@ -3620,6 +3641,42 @@ "@opentelemetry/sdk-trace-base" "2.8.0" "@opentelemetry/semantic-conventions" "^1.29.0" +"@opentelemetry/instrumentation-http@~0.218.0": + version "0.218.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-http/-/instrumentation-http-0.218.0.tgz#0b26b702a6288fa11bdea48ff4a3635385a7d587" + integrity sha512-x9djaqdzpT8WAboep1H9nCAQ1E+MMsm08TNfA02TqM3bNNddZeiim+E3KMWVQFaX6JpUy7V0nm/wfN/K2Em+Zw== + dependencies: + "@opentelemetry/core" "2.7.1" + "@opentelemetry/instrumentation" "0.218.0" + "@opentelemetry/semantic-conventions" "^1.29.0" + forwarded-parse "2.1.2" + +"@opentelemetry/instrumentation-ioredis@~0.64.0": + version "0.64.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-ioredis/-/instrumentation-ioredis-0.64.0.tgz#b02a214263d5f3a6848f6911fe23f505ff6a9181" + integrity sha512-GQ36/amPdO1rVPXgrRZNnd6MktqwDcYalzpMRe9m55b3EwX4pazq8VB3qfTH67xboElqm/B9J1tBEnbQmcvaww== + dependencies: + "@opentelemetry/instrumentation" "^0.216.0" + "@opentelemetry/redis-common" "^0.38.3" + "@opentelemetry/semantic-conventions" "^1.33.0" + +"@opentelemetry/instrumentation-mongodb@~0.69.0": + version "0.69.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-mongodb/-/instrumentation-mongodb-0.69.0.tgz#b03010de06c816f973038165cfe4cea852b3d43f" + integrity sha512-kj8w2FN2/z0VIXMqcdAdJYtc0udH41Sb485jC7tLl0X4+OD3KLjyhjVoZOXH/gxp+N+BQY6SKgMNC0yi8nok9A== + dependencies: + "@opentelemetry/instrumentation" "^0.216.0" + "@opentelemetry/semantic-conventions" "^1.33.0" + +"@opentelemetry/instrumentation@0.218.0": + version "0.218.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.218.0.tgz#fcceb4ffb45f99c0d292600769150fc5944dc3e9" + integrity sha512-mIZil8Es+sYDK5m+DQiwAwF57F14TF2YlEqvIjZ/RQWcxDBwRGsKfdK2Tv65OU9meQKCMzSIFS9mxAcnAb6Bkg== + dependencies: + "@opentelemetry/api-logs" "0.218.0" + import-in-the-middle "^3.0.0" + require-in-the-middle "^8.0.0" + "@opentelemetry/instrumentation@0.219.0": version "0.219.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.219.0.tgz#84399affd8ee4a12cb199f325c4858654d8dedee" @@ -3629,6 +3686,15 @@ import-in-the-middle "^3.0.0" require-in-the-middle "^8.0.0" +"@opentelemetry/instrumentation@^0.216.0": + version "0.216.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation/-/instrumentation-0.216.0.tgz#048777113d0cb7f6b6613025055fc0d2f822bf49" + integrity sha512-BrY0b2K81OLgwBcFxY2wKgPFhq4DpindT+S83++zquc5Rtb2SuYLMkujgDRWMgZQDz+OT+dfvPnMGADPuw4FDw== + dependencies: + "@opentelemetry/api-logs" "0.216.0" + import-in-the-middle "^3.0.0" + require-in-the-middle "^8.0.0" + "@opentelemetry/otlp-exporter-base@0.219.0": version "0.219.0" resolved "https://registry.yarnpkg.com/@opentelemetry/otlp-exporter-base/-/otlp-exporter-base-0.219.0.tgz#68f4908f45f6df577d8e1c5b42761645f01cffc5" @@ -3673,6 +3739,11 @@ dependencies: "@opentelemetry/core" "2.8.0" +"@opentelemetry/redis-common@^0.38.3": + version "0.38.3" + resolved "https://registry.yarnpkg.com/@opentelemetry/redis-common/-/redis-common-0.38.3.tgz#31a0464a48a991c29408614e3725d94db7c11aee" + integrity sha512-VCghU1JYs/4gP6Gqf/xro9MEsZ7LrMv2uONVsaESKL38ZOB9BqnI98FfS23wjMnHlpuE+TTaWSoAVNpTwYXzjw== + "@opentelemetry/resources@2.8.0", "@opentelemetry/resources@^2.8.0": version "2.8.0" resolved "https://registry.yarnpkg.com/@opentelemetry/resources/-/resources-2.8.0.tgz#9bcb658ab6254f33099f4a95544b40d6f53cc946" @@ -3754,6 +3825,11 @@ resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.41.1.tgz#b04e7151c5913a7a006d4f465479da75efb98a7a" integrity sha512-/UhIkaZgPutTFmQ7RnIJGgDXZmtEJ7Dvi86xNTFWcnRxVRNk/aotsqDJYeEvDP+FSMB2SdW+pQzNMcWP0rwuNA== +"@opentelemetry/semantic-conventions@^1.33.0": + version "1.43.0" + resolved "https://registry.yarnpkg.com/@opentelemetry/semantic-conventions/-/semantic-conventions-1.43.0.tgz#f3f467e36c27332f0e735ec86cdcd78dd6f27865" + integrity sha512-eSYWTm620tTk45EKSedaUL8MFYI8hW164hIXsgIHyxu3VobUB3fFCu5t0hQby6OoWRPsG1KkKUG2M5UadiLiVg== + "@pkgjs/parseargs@^0.11.0": version "0.11.0" resolved "https://registry.yarnpkg.com/@pkgjs/parseargs/-/parseargs-0.11.0.tgz#a77ea742fab25775145434eb1d2328cf5013ac33" @@ -8401,6 +8477,11 @@ form-data@~2.3.2: combined-stream "^1.0.6" mime-types "^2.1.12" +forwarded-parse@2.1.2: + version "2.1.2" + resolved "https://registry.yarnpkg.com/forwarded-parse/-/forwarded-parse-2.1.2.tgz#08511eddaaa2ddfd56ba11138eee7df117a09325" + integrity sha512-alTFZZQDKMporBH77856pXgzhEzaUVmLCDk+egLgIgHst3Tpndzz8MnKe+GzRJRfvVdn69HhpW7cmXzvtLvJAw== + forwarded@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/forwarded/-/forwarded-0.2.0.tgz#2269936428aad4c15c7ebe9779a84bf0b2a81811" From 1ffff2bbdb905292aff63706b6774a8db04a6acb Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Wed, 3 Jun 2026 16:11:43 +0200 Subject: [PATCH 2/4] feat: wire OpenTelemetry bootstrap and shutdown via arsenal Keep functions not async but return the Promise for tests Issue: CLDSRV-884 (cherry picked from commit 0f86bf9edc9466d592c5720b43e3b2dd389788c2) --- index.js | 21 ++++++++++++ lib/server.js | 13 ++++--- tests/unit/server.js | 82 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index f5fa36c2e2..fe68a8a1b1 100644 --- a/index.js +++ b/index.js @@ -7,4 +7,25 @@ require('werelogs').stderrUtils.catchAndTimestampStderr( require('cluster').isPrimary ? 1 : null, ); +const tracing = require('arsenal/build/lib/tracing'); + +// Gated on isEnabled() so the OTEL-off path doesn't load Config early. +if (tracing.isEnabled() && !(require('./lib/Config').config.isCluster && require('cluster').isPrimary)) { + tracing.init({ + serviceName: 'cloudserver', + serviceVersion: require('./package.json').version, + instrumentations: () => { + const { HttpInstrumentation } = require('@opentelemetry/instrumentation-http'); + const { IORedisInstrumentation } = require('@opentelemetry/instrumentation-ioredis'); + const { MongoDBInstrumentation } = require('@opentelemetry/instrumentation-mongodb'); + const healthPaths = ['/live', '/ready', '/_/healthcheck', '/_/healthcheck/deep', '/metrics']; + return [ + new HttpInstrumentation(tracing.makeHttpInstrumentationConfig({ healthPaths })), + new IORedisInstrumentation({ requireParentSpan: true }), + new MongoDBInstrumentation({ enhancedDatabaseReporting: false }), + ]; + }, + }); +} + require('./lib/server.js')(); diff --git a/lib/server.js b/lib/server.js index a5bb364b61..eee6e84139 100644 --- a/lib/server.js +++ b/lib/server.js @@ -6,6 +6,7 @@ const arsenal = require('arsenal'); const { setServerHeader } = arsenal.s3routes.routesUtils; const { RedisClient, StatsClient } = arsenal.metrics; const monitoringClient = require('./utilities/monitoringHandler'); +const tracing = require('arsenal/build/lib/tracing'); const logger = require('./utilities/logger'); const { internalHandlers } = require('./utilities/internalHandlers'); @@ -332,14 +333,16 @@ class S3Server { if (this.config.rateLimiting?.enabled) { stopRefillJob(logger); } - Promise.all(this.servers.map(server => + return Promise.all(this.servers.map(server => new Promise(resolve => server.close(resolve)) - )).then(() => process.exit(0)); + )).finally(() => tracing.close()) + .finally(() => process.exit(0)); } caughtExceptionShutdown() { if (!this.cluster) { - process.exit(1); + return tracing.close() + .finally(() => process.exit(1)); } logger.error('shutdown of worker due to exception', { workerId: this.worker ? this.worker.id : undefined, @@ -348,8 +351,10 @@ class S3Server { // Will close all servers, cause disconnect event on primary and kill // worker process with 'SIGTERM'. if (this.worker) { - this.worker.kill(); + return tracing.close() + .finally(() => this.worker.kill()); } + return undefined; } startServer(listenOn, port, routeRequest) { diff --git a/tests/unit/server.js b/tests/unit/server.js index e0d8604e30..b3b5f81dc5 100644 --- a/tests/unit/server.js +++ b/tests/unit/server.js @@ -8,6 +8,7 @@ const arsenal = require('arsenal'); const uuid = require('uuid'); const logger = require('../../lib/utilities/logger'); const { config: defaultConfig } = require('../../lib/Config'); +const tracing = require('arsenal/build/lib/tracing'); const { S3Server } = require('../../lib/server'); describe('S3Server', () => { @@ -250,3 +251,84 @@ describe('S3Server request timeout', () => { assert.strictEqual(mockServer.requestTimeout, 0); }); }); + +describe('S3Server shutdown', () => { + let server; + let tracingCloseStub; + let exitStub; + + beforeEach(() => { + server = new S3Server({ + ...defaultConfig, + port: undefined, + listenOn: [], + internalPort: undefined, + internalListenOn: [], + metricsListenOn: [], + metricsPort: 8002, + }); + // S3Server.cleanUp iterates this.servers and closes each — empty + // array makes the Promise.all in cleanUp resolve immediately. + server.servers = []; + // Avoid touching the rateLimiting refill-job teardown path. + server.config = { ...server.config, rateLimiting: { enabled: false } }; + + tracingCloseStub = sinon.stub(tracing, 'close').resolves(); + exitStub = sinon.stub(process, 'exit'); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should flush OTEL via tracing.close before process.exit(0) on cleanUp', async () => { + await server.cleanUp(); + sinon.assert.callOrder(tracingCloseStub, exitStub); + assert.strictEqual(tracingCloseStub.callCount, 1); + assert.strictEqual(exitStub.callCount, 1); + assert.strictEqual(exitStub.firstCall.args[0], 0); + }); + + it('should flush OTEL on cleanUp even when a server.close errors', async () => { + const erroringServer = { + close() { + // dev/9.3 doesn't use first argument as error, throw instead + throw new Error('socket already closed'); + }, + }; + server.servers = [erroringServer]; + await assert.rejects(() => server.cleanUp(), /socket already closed/); + assert.strictEqual(tracingCloseStub.callCount, 1); + assert.strictEqual(exitStub.callCount, 1); + assert.strictEqual(exitStub.firstCall.args[0], 0); + }); + + it('should still exit(0) on cleanUp even if tracing.close rejects', async () => { + tracingCloseStub.rejects(new Error('flush failed')); + await assert.rejects(() => server.cleanUp(), /flush failed/); + assert.strictEqual(tracingCloseStub.callCount, 1); + assert.strictEqual(exitStub.firstCall.args[0], 0); + }); + + it('should flush OTEL before process.exit(1) on caughtExceptionShutdown (non-cluster)', async () => { + server.cluster = false; + await server.caughtExceptionShutdown(); + sinon.assert.callOrder(tracingCloseStub, exitStub); + assert.strictEqual(tracingCloseStub.callCount, 1); + assert.strictEqual(exitStub.firstCall.args[0], 1); + }); + + it('should flush OTEL before worker.kill on caughtExceptionShutdown (cluster worker)', async () => { + const killStub = sinon.stub(); + server.cluster = true; + server.worker = { id: 1, process: { pid: 12345 }, kill: killStub }; + + await server.caughtExceptionShutdown(); + + sinon.assert.callOrder(tracingCloseStub, killStub); + assert.strictEqual(tracingCloseStub.callCount, 1); + assert.strictEqual(killStub.callCount, 1); + // The non-cluster process.exit branch is skipped in this path. + assert.strictEqual(exitStub.callCount, 0); + }); +}); From 55f6bbd6c2961a034ed9d2ba4f753ce7fcb4c719 Mon Sep 17 00:00:00 2001 From: Thomas Flament Date: Wed, 3 Jun 2026 16:11:43 +0200 Subject: [PATCH 3/4] feat: instrument all S3 API handlers with OTEL spans Issue: CLDSRV-884 (cherry picked from commit b16b268d1db1832d62bce8995c072b040711982c) --- lib/api/api.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/api/api.js b/lib/api/api.js index 487dd7d0ce..c00461373e 100644 --- a/lib/api/api.js +++ b/lib/api/api.js @@ -79,6 +79,7 @@ const parseCopySource = require('./apiUtils/object/parseCopySource'); const { tagConditionKeyAuth } = require('./apiUtils/authorization/tagConditionKeys'); const { isRequesterASessionUser } = require('./apiUtils/authorization/permissionChecks'); const checkHttpHeadersSize = require('./apiUtils/object/checkHttpHeadersSize'); +const { instrumentApiMethod } = require('arsenal/build/lib/tracing'); const constants = require('../../constants'); const { config } = require('../Config.js'); const metadata = require('../metadata/wrapper'); @@ -609,4 +610,11 @@ const api = { handleAuthorizationResults, }; +const NON_INSTRUMENTED_KEYS = new Set(['callApiMethod', 'checkAuthResults', 'handleAuthorizationResults']); +for (const [name, handler] of Object.entries(api)) { + if (typeof handler === 'function' && !NON_INSTRUMENTED_KEYS.has(name)) { + api[name] = instrumentApiMethod(handler, name); + } +} + module.exports = api; From eb92f643bd740a1781f1e8422c77120ad14f63dc Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 16 Sep 2026 23:14:15 +0200 Subject: [PATCH 4/4] CLDSRV-884: Run prettier in development/9.3 --- lib/server.js | 70 ++++++++++++++++++------------------------ tests/unit/server.js | 73 ++++++++++++++++++++++---------------------- 2 files changed, 66 insertions(+), 77 deletions(-) diff --git a/lib/server.js b/lib/server.js index eee6e84139..e6084b03df 100644 --- a/lib/server.js +++ b/lib/server.js @@ -16,15 +16,11 @@ const { blacklistedPrefixes } = require('../constants'); const api = require('./api/api'); const dataWrapper = require('./data/wrapper'); const kms = require('./kms/wrapper'); -const locationStorageCheck = - require('./api/apiUtils/object/locationStorageCheck'); +const locationStorageCheck = require('./api/apiUtils/object/locationStorageCheck'); const vault = require('./auth/vault'); const metadata = require('./metadata/wrapper'); const { initManagement } = require('./management'); -const { - initManagementClient, - isManagementAgentUsed, -} = require('./management/agentClient'); +const { initManagementClient, isManagementAgentUsed } = require('./management/agentClient'); const { startCleanupJob } = require('./api/apiUtils/rateLimit/cleanup'); const { startRefillJob, stopRefillJob } = require('./api/apiUtils/rateLimit/refillJob'); @@ -47,8 +43,7 @@ updateAllEndpoints(); _config.on('location-constraints-update', () => { if (implName === 'multipleBackends') { const clients = parseLC(_config, vault); - client = new MultipleBackendGateway( - clients, metadata, locationStorageCheck); + client = new MultipleBackendGateway(clients, metadata, locationStorageCheck); } }); @@ -60,8 +55,7 @@ if (_config.localCache) { // stats client const STATS_INTERVAL = 5; // 5 seconds const STATS_EXPIRY = 30; // 30 seconds -const statsClient = new StatsClient(localCacheClient, STATS_INTERVAL, - STATS_EXPIRY); +const statsClient = new StatsClient(localCacheClient, STATS_INTERVAL, STATS_EXPIRY); const enableRemoteManagement = true; class S3Server { @@ -85,7 +79,7 @@ class S3Server { process.on('SIGHUP', this.cleanUp.bind(this)); process.on('SIGQUIT', this.cleanUp.bind(this)); process.on('SIGTERM', this.cleanUp.bind(this)); - process.on('SIGPIPE', () => { }); + process.on('SIGPIPE', () => {}); // This will pick up exceptions up the stack process.on('uncaughtException', err => { // If just send the error object results in empty @@ -131,9 +125,10 @@ class S3Server { const requestStartTime = process.hrtime.bigint(); // Skip server access logs for heartbeat. - const isLoggingEnabled = _config.serverAccessLogs - && (_config.serverAccessLogs.mode === serverAccessLogsModes.LOG_ONLY - || _config.serverAccessLogs.mode === serverAccessLogsModes.ENABLED); + const isLoggingEnabled = + _config.serverAccessLogs && + (_config.serverAccessLogs.mode === serverAccessLogsModes.LOG_ONLY || + _config.serverAccessLogs.mode === serverAccessLogsModes.ENABLED); const isInternalRoute = req.url.startsWith('/_'); const isBackbeatRoute = req.url.startsWith('/_/backbeat/'); if (isLoggingEnabled && (!isInternalRoute || isBackbeatRoute)) { @@ -177,9 +172,7 @@ class S3Server { labels.action = req.apiMethod; } monitoringClient.httpRequestsTotal.labels(labels).inc(); - monitoringClient.httpRequestDurationSeconds - .labels(labels) - .observe(responseTimeInNs / 1e9); + monitoringClient.httpRequestDurationSeconds.labels(labels).observe(responseTimeInNs / 1e9); monitoringClient.httpActiveRequests.dec(); }; res.on('close', monitorEndOfRequest); @@ -232,14 +225,13 @@ class S3Server { }; let reqUids = req.headers['x-scal-request-uids']; - if (reqUids !== undefined && !/*isValidReqUids*/(reqUids.length < 128)) { + if (reqUids !== undefined && !(/*isValidReqUids*/ (reqUids.length < 128))) { // simply ignore invalid id (any user can provide an // invalid request ID through a crafted header) reqUids = undefined; } - const log = (reqUids !== undefined ? - logger.newRequestLoggerFromSerializedUids(reqUids) : - logger.newRequestLogger()); + const log = + reqUids !== undefined ? logger.newRequestLoggerFromSerializedUids(reqUids) : logger.newRequestLogger(); log.end().addDefaultFields(clientInfo); log.debug('received admin request', clientInfo); @@ -293,8 +285,7 @@ class S3Server { server.requestTimeout = 0; // disabling request timeout server.on('connection', socket => { - socket.on('error', err => logger.info('request rejected', - { error: err })); + socket.on('error', err => logger.info('request rejected', { error: err })); }); // https://nodejs.org/dist/latest-v6.x/ @@ -310,8 +301,11 @@ class S3Server { }; const { address } = addr; logger.info('server started', { - address, port, - pid: process.pid, serverIP: address, serverPort: port + address, + port, + pid: process.pid, + serverIP: address, + serverPort: port, }); }); @@ -333,16 +327,14 @@ class S3Server { if (this.config.rateLimiting?.enabled) { stopRefillJob(logger); } - return Promise.all(this.servers.map(server => - new Promise(resolve => server.close(resolve)) - )).finally(() => tracing.close()) + return Promise.all(this.servers.map(server => new Promise(resolve => server.close(resolve)))) + .finally(() => tracing.close()) .finally(() => process.exit(0)); } caughtExceptionShutdown() { if (!this.cluster) { - return tracing.close() - .finally(() => process.exit(1)); + return tracing.close().finally(() => process.exit(1)); } logger.error('shutdown of worker due to exception', { workerId: this.worker ? this.worker.id : undefined, @@ -351,8 +343,7 @@ class S3Server { // Will close all servers, cause disconnect event on primary and kill // worker process with 'SIGTERM'. if (this.worker) { - return tracing.close() - .finally(() => this.worker.kill()); + return tracing.close().finally(() => this.worker.kill()); } return undefined; } @@ -368,10 +359,7 @@ class S3Server { } initiateStartup(log) { - series([ - next => metadata.setup(next), - next => clientCheck(true, log, next), - ], (err, results) => { + series([next => metadata.setup(next), next => clientCheck(true, log, next)], (err, results) => { if (err) { log.warn('initial health check failed, delaying startup', { error: err, @@ -422,8 +410,10 @@ class S3Server { try { logger.info('ServerAccessLogger config', { config: _config.serverAccessLogs }); - if (_config.serverAccessLogs.mode === serverAccessLogsModes.LOG_ONLY - || _config.serverAccessLogs.mode === serverAccessLogsModes.ENABLED) { + if ( + _config.serverAccessLogs.mode === serverAccessLogsModes.LOG_ONLY || + _config.serverAccessLogs.mode === serverAccessLogsModes.ENABLED + ) { var serverAccessLogger = new ServerAccessLogger( _config.serverAccessLogs.outputFile, _config.serverAccessLogs.highWaterMarkBytes, @@ -439,7 +429,6 @@ class S3Server { logger.error('ServerAccessLogger creation error', error); } - this.started = true; }); } @@ -495,8 +484,7 @@ function main() { }); const metricServer = new S3Server(_config); - metricServer.startServer(_config.metricsListenOn, - _config.metricsPort, metricServer.routeAdminRequest); + metricServer.startServer(_config.metricsListenOn, _config.metricsPort, metricServer.routeAdminRequest); } if (_config.isCluster && cluster.isWorker) { const server = new S3Server(_config, cluster.worker); diff --git a/tests/unit/server.js b/tests/unit/server.js index b3b5f81dc5..f33d08c093 100644 --- a/tests/unit/server.js +++ b/tests/unit/server.js @@ -27,7 +27,7 @@ describe('S3Server', () => { internalPort: undefined, internalListenOn: [], metricsListenOn: [], - metricsPort: 8002 + metricsPort: 8002, }; server = new S3Server(config); @@ -39,14 +39,15 @@ describe('S3Server', () => { sinon.restore(); }); - const waitReady = () => new Promise(resolve => { - const interval = setInterval(() => { - if (server.started) { - clearInterval(interval); - resolve(); - } - }, 100); - }); + const waitReady = () => + new Promise(resolve => { + const interval = setInterval(() => { + if (server.started) { + clearInterval(interval); + resolve(); + } + }, 100); + }); describe('initiateStartup', () => { beforeEach(() => { @@ -57,12 +58,13 @@ describe('S3Server', () => { // `sinon` matcher to match when the callback argument actually invokes the expected // function - const wrapperFor = expected => sinon.match(actual => { - const req = uuid.v4(); - const res = uuid.v4(); - actual(req, res); - return expected.calledWith(req, res); - }); + const wrapperFor = expected => + sinon.match(actual => { + const req = uuid.v4(); + const res = uuid.v4(); + actual(req, res); + return expected.calledWith(req, res); + }); it('should start API server with default port if no listenOn is provided', async () => { config.port = 8000; @@ -74,13 +76,12 @@ describe('S3Server', () => { assert.strictEqual(startServerStub.callCount, 2); assert(startServerStub.calledWith(wrapperFor(server.routeRequest), 8000)); assert(startServerStub.calledWith(wrapperFor(server.routeAdminRequest))); - }); - + it('should start API servers from listenOn array', async () => { config.listenOn = [ { port: 8000, ip: '127.0.0.1' }, - { port: 8001, ip: '0.0.0.0' } + { port: 8001, ip: '0.0.0.0' }, ]; config.port = 9999; // Should be ignored since listenOn is provided @@ -94,7 +95,7 @@ describe('S3Server', () => { assert(startServerStub.calledWith(wrapperFor(server.routeAdminRequest))); assert.strictEqual(startServerStub.neverCalledWith(sinon.any, 9999), true); }); - + it('should start internal API server with internalPort if no internalListenOn is provided', async () => { config.internalPort = 9000; @@ -105,11 +106,11 @@ describe('S3Server', () => { assert.strictEqual(startServerStub.callCount, 2); assert(startServerStub.calledWith(wrapperFor(server.internalRouteRequest), 9000)); }); - + it('should start internal API servers from internalListenOn array', async () => { config.internalListenOn = [ { port: 9000, ip: '127.0.0.1' }, - { port: 9001, ip: '0.0.0.0' } + { port: 9001, ip: '0.0.0.0' }, ]; config.internalPort = 9999; // Should be ignored since internalListenOn is provided @@ -123,29 +124,29 @@ describe('S3Server', () => { assert(startServerStub.calledWith(wrapperFor(server.routeAdminRequest))); assert.strictEqual(startServerStub.neverCalledWith(sinon.any, 9999), true); }); - + it('should start metrics server with metricsPort if no metricsListenOn is provided', async () => { config.metricsPort = 8012; server.initiateStartup(log); await waitReady(); - + assert.strictEqual(startServerStub.callCount, 1); assert(startServerStub.calledWith(wrapperFor(server.routeAdminRequest), 8012)); }); - + it('should start metrics servers from metricsListenOn array', async () => { config.metricsListenOn = [ { port: 8002, ip: '127.0.0.1' }, - { port: 8003, ip: '0.0.0.0' } + { port: 8003, ip: '0.0.0.0' }, ]; config.metricsPort = 9999; // Should be ignored since metricsListenOn is provided server.initiateStartup(log); await waitReady(); - + assert.strictEqual(startServerStub.callCount, 2); assert(startServerStub.calledWith(wrapperFor(server.routeAdminRequest), 8002, '127.0.0.1')); assert(startServerStub.calledWith(wrapperFor(server.routeAdminRequest), 8003, '0.0.0.0')); @@ -170,10 +171,10 @@ describe('S3Server', () => { describe('internalRouteRequest', () => { const resp = { - on: () => { }, - setHeader: () => { }, - writeHead: () => { }, - end: () => { }, + on: () => {}, + setHeader: () => {}, + writeHead: () => {}, + end: () => {}, }; let req; @@ -182,7 +183,7 @@ describe('S3Server', () => { req = { headers: {}, socket: { - setNoDelay: () => { }, + setNoDelay: () => {}, }, url: 'http://localhost:8000', }; @@ -220,7 +221,7 @@ describe('S3Server request timeout', () => { beforeEach(() => { sandbox = sinon.createSandbox(); - + // Create a mock server to capture the requestTimeout setting mockServer = { requestTimeout: null, @@ -228,7 +229,7 @@ describe('S3Server request timeout', () => { listen: sandbox.stub(), address: sandbox.stub().returns({ address: '127.0.0.1', port: 8000 }), }; - + // Mock server creation to return our mock sandbox.stub(http, 'createServer').returns(mockServer); sandbox.stub(https, 'createServer').returns(mockServer); @@ -241,12 +242,12 @@ describe('S3Server request timeout', () => { it('should set server.requestTimeout to 0 when starting server', () => { const server = new S3Server({ ...defaultConfig, - https: false + https: false, }); - + // Call _startServer which should set requestTimeout = 0 server._startServer(() => {}, 8000, '127.0.0.1'); - + // Verify that requestTimeout was set to 0 assert.strictEqual(mockServer.requestTimeout, 0); });