fixup! Refactor: pass scanId as parameter instead of instance field and fix metric timing

benzekrimaha · benzekrimaha · commit e909f9e2814e · 2026-03-19T11:12:07.000+01:00
fix(tests): update tests to reflect messageId removal and scanId parameter changes
diff --git a/extensions/lifecycle/bucketProcessor/LifecycleBucketProcessor.js b/extensions/lifecycle/bucketProcessor/LifecycleBucketProcessor.js
@@ -386,7 +386,7 @@ class LifecycleBucketProcessor {
                 value: result,
                 s3target: s3Client,
                 backbeatMetadataProxy,
-            }, (err) => {
+            }, err => {
                 // Increment metric after task successfully completes
                 // This ensures we count actual processing completion, not just dispatch
                 // Only count successful completions, not failures
diff --git a/tests/functional/lifecycle/LifecycleConductor.spec.js b/tests/functional/lifecycle/LifecycleConductor.spec.js
@@ -41,15 +41,15 @@ const expected2Messages = (version='v2') => ([
     {
         value: {
             action: 'processObjects',
-            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id', messageId: 'test-message-id' },
+            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id' },
             target: { bucket: 'bucket1', owner: 'owner1', taskVersion: version },
             details: {},
         },
     },
     {
         value: {
             action: 'processObjects',
-            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id', messageId: 'test-message-id' },
+            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id' },
             target: { bucket: 'bucket1-2', owner: 'owner1', taskVersion: version },
             details: {},
         },
@@ -60,31 +60,31 @@ const expected4Messages = (version='v2') => ([
     {
         value: {
             action: 'processObjects',
-            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id', messageId: 'test-message-id' },
+            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id' },
             target: { bucket: 'bucket1', owner: 'owner1', taskVersion: version },
             details: {},
         },
     },
     {
         value: {
             action: 'processObjects',
-            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id', messageId: 'test-message-id' },
+            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id' },
             target: { bucket: 'bucket1-2', owner: 'owner1', taskVersion: version },
             details: {},
         },
     },
     {
         value: {
             action: 'processObjects',
-            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id', messageId: 'test-message-id' },
+            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id' },
             target: { bucket: 'bucket3', owner: 'owner3', taskVersion: version },
             details: {},
         },
     },
     {
         value: {
             action: 'processObjects',
-            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id', messageId: 'test-message-id' },
+            contextInfo: { reqId: 'test-request-id', conductorScanId: 'test-scan-id' },
             target: { bucket: 'bucket4', owner: 'owner4', taskVersion: version },
             details: {},
         },
diff --git a/tests/unit/lifecycle/LifecycleConductor.spec.js b/tests/unit/lifecycle/LifecycleConductor.spec.js
@@ -239,15 +239,18 @@ describe('Lifecycle Conductor', () => {
             };
 
             sinon.stub(conductor, '_controlBacklog').callsFake(cb => cb(null));
+            const metricStub = sinon.stub(LifecycleMetrics, 'onConductorFullScan');
 
             conductor.processBuckets(err => {
                 assert.ifError(err);
-                assert(conductor._scanId);
-                assert(typeof conductor._scanId === 'string');
+                assert(metricStub.calledOnce);
+                const scanId = metricStub.firstCall.args[0];
+                assert(scanId);
+                assert(typeof scanId === 'string');
                 // Validate UUID v4 format: 8-4-4-4-12 hex characters
                 const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
-                assert(uuidRegex.test(conductor._scanId), 
-                    `conductorScanId should be a valid UUID v4, got: ${conductor._scanId}`);
+                assert(uuidRegex.test(scanId), 
+                    `conductorScanId should be a valid UUID v4, got: ${scanId}`);
                 done();
             });
         });
@@ -264,7 +267,7 @@ describe('Lifecycle Conductor', () => {
             sinon.stub(conductor._mongodbClient, 'getIndexingJobs')
                 .callsFake((_, cb) => cb(null, ['job1', 'job2']));
             sinon.stub(conductor, 'listBuckets')
-                .callsFake((mQueue, log, cb) => {
+                .callsFake((mQueue, scanId, log, cb) => {
                     mQueue.push({
                         bucketName: 'testbucket',
                         canonicalId: 'testId',
@@ -281,7 +284,7 @@ describe('Lifecycle Conductor', () => {
                     setTimeout(() => cb(null, []), 1000);
                 });
             sinon.stub(conductor, '_createBucketTaskMessages')
-                .callsFake((_a, _b, cb) => {
+                .callsFake((_a, _b, _c, cb) => {
                     order.push(2);
                     assert(conductor.activeIndexingJobsRetrieved);
                     setTimeout(() => cb(null, []), 1000);
@@ -307,9 +310,7 @@ describe('Lifecycle Conductor', () => {
 
     describe('_indexesGetOrCreate', () => {
         it('should include conductor scan id in task context', () => {
-            conductor._scanId = 'scan-id-test';
-
-            const taskMessage = conductor._taskToMessage(getTask(true), lifecycleTaskVersions.v2, log);
+            const taskMessage = conductor._taskToMessage(getTask(true), lifecycleTaskVersions.v2, 'scan-id-test', log);
             const parsed = JSON.parse(taskMessage.message);
             assert.strictEqual(parsed.contextInfo.conductorScanId, 'scan-id-test');
         });
@@ -505,7 +506,7 @@ describe('Lifecycle Conductor', () => {
             const lcConductor = makeLifecycleConductorWithFilters({
                 bucketSource: 'zookeeper',
             });
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 2);
                 assert.strictEqual(queue.length(), 2);
                 const expectedQueue = [
@@ -528,7 +529,7 @@ describe('Lifecycle Conductor', () => {
             const lcConductor = makeLifecycleConductorWithFilters({
                 bucketSource: 'bucketd',
             }, markers);
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 2);
                 assert.strictEqual(queue.length(), 2);
 
@@ -559,7 +560,7 @@ describe('Lifecycle Conductor', () => {
                 'invalid:bucketuid789',
                 'invalid',
             ]);
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 2);
                 assert.strictEqual(queue.length(), 2);
                 const expectedQueue = [
@@ -582,7 +583,7 @@ describe('Lifecycle Conductor', () => {
                 bucketsDenied: [bucket1],
                 bucketSource: 'zookeeper',
             });
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 1);
                 assert.strictEqual(queue.length(), 1);
                 const expectedQueue = [
@@ -602,7 +603,7 @@ describe('Lifecycle Conductor', () => {
                 bucketsDenied: [bucket1],
                 bucketSource: 'bucketd',
             }, markers);
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 1);
                 assert.strictEqual(queue.length(), 1);
                 const expectedQueue = [
@@ -623,7 +624,7 @@ describe('Lifecycle Conductor', () => {
                 accountsDenied: [`${accountName1}:${account1}`],
                 bucketSource: 'zookeeper',
             });
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 1);
                 assert.strictEqual(queue.length(), 1);
                 const expectedQueue = [
@@ -643,7 +644,7 @@ describe('Lifecycle Conductor', () => {
                 accountsDenied: [`${accountName1}:${account1}`],
                 bucketSource: 'bucketd',
             }, markers);
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 1);
                 assert.strictEqual(queue.length(), 1);
                 const expectedQueue = [
@@ -665,7 +666,7 @@ describe('Lifecycle Conductor', () => {
                 bucketsDenied: [bucket2],
                 bucketSource: 'zookeeper',
             });
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 0);
                 assert.strictEqual(queue.length(), 0);
                 const expectedQueue = [];
@@ -681,7 +682,7 @@ describe('Lifecycle Conductor', () => {
                 bucketsDenied: [bucket2],
                 bucketSource: 'bucketd',
             }, markers);
-            lcConductor.listBuckets(queue, fakeLogger, (err, length) => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, (err, length) => {
                 assert.strictEqual(length, 0);
                 assert.strictEqual(queue.length(), 0);
                 const expectedQueue = [];
@@ -706,7 +707,7 @@ describe('Lifecycle Conductor', () => {
                 getCollection: () => ({ find: findStub })
             };
             sinon.stub(lcConductor._zkClient, 'getData').yields(null, null, null);
-            lcConductor.listBuckets(queue, fakeLogger, err => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, err => {
                 assert.ifError(err);
                 assert.deepEqual(findStub.getCall(0).args[0], {});
                 done();
@@ -728,7 +729,7 @@ describe('Lifecycle Conductor', () => {
                 getCollection: () => ({ find: findStub })
             };
             sinon.stub(lcConductor._zkClient, 'getData').yields(null, null, null);
-            lcConductor.listBuckets(queue, fakeLogger, err => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, err => {
                 assert.ifError(err);
                 assert.deepEqual(findStub.getCall(0).args[0], {
                     'value.owner': {
@@ -756,7 +757,7 @@ describe('Lifecycle Conductor', () => {
                 getCollection: () => ({ find: findStub })
             };
             sinon.stub(lcConductor._zkClient, 'getData').yields(null, null, null);
-            lcConductor.listBuckets(queue, fakeLogger, err => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, err => {
                 assert.ifError(err);
                 assert.deepEqual(findStub.getCall(0).args[0], {
                     _id: {
@@ -781,7 +782,7 @@ describe('Lifecycle Conductor', () => {
                 getCollection: () => ({ find: findStub })
             };
             sinon.stub(lcConductor._zkClient, 'getData').yields(null, Buffer.from('bucket1'), null);
-            lcConductor.listBuckets(queue, fakeLogger, err => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, err => {
                 assert.ifError(err);
                 assert.deepEqual(findStub.getCall(0).args[0], {
                     _id: {
@@ -808,7 +809,7 @@ describe('Lifecycle Conductor', () => {
                 getCollection: () => ({ find: findStub })
             };
             sinon.stub(lcConductor._zkClient, 'getData').yields(null, Buffer.from('bucket1'), null);
-            lcConductor.listBuckets(queue, fakeLogger, err => {
+            lcConductor.listBuckets(queue, 'test-scan-id', fakeLogger, err => {
                 assert.ifError(err);
                 assert.deepEqual(findStub.getCall(0).args[0], {
                     '_id': {
diff --git a/tests/utils/BackbeatTestConsumer.js b/tests/utils/BackbeatTestConsumer.js
@@ -40,14 +40,6 @@ class BackbeatTestConsumer extends BackbeatConsumer {
                         parsedMsg.contextInfo.conductorScanId =
                             expectedMsg.value.contextInfo.conductorScanId;
                     }
-                    // messageId is also generated per message: check it exists,
-                    // then normalize for comparison
-                    if (expectedMsg.value.contextInfo?.messageId === 'test-message-id') {
-                        assert(parsedMsg.contextInfo?.messageId,
-                            'expected contextInfo.messageId field');
-                        parsedMsg.contextInfo.messageId =
-                            expectedMsg.value.contextInfo.messageId;
-                    }
                 }
                 assert.deepStrictEqual(
                     parsedMsg, expectedMsg.value,
