Skip to content

Commit df3a966

Browse files
committed
Fix metric timing and code quality issues
- Move bucket done metric to task completion callback (only count successes) - Fix conductor full scan metric to skip throttled/error cases - Fix line length violation and indentation issues Issue: BB-740
1 parent a464b39 commit df3a966

File tree

3 files changed

+31
-21
lines changed

3 files changed

+31
-21
lines changed

extensions/lifecycle/LifecycleMetrics.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,9 @@ class LifecycleMetrics {
263263
}
264264

265265
/**
266-
* Increment the count of lifecycle-enabled buckets processed
267-
* during the current scan.
266+
* Increment the count of lifecycle-enabled buckets successfully processed
267+
* during the current scan. This is called after the bucket task completes
268+
* successfully, not when it is dispatched to the scheduler.
268269
* @param {Object} log - logger
269270
*/
270271
static onBucketProcessorBucketDone(log) {

extensions/lifecycle/bucketProcessor/LifecycleBucketProcessor.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,6 @@ class LifecycleBucketProcessor {
364364
return cb();
365365
}
366366

367-
LifecycleMetrics.onBucketProcessorBucketDone(this._log);
368-
369367
let task;
370368

371369
if (taskVersion === lifecycleTaskVersions.v1 || (!taskVersion && this._lcConfig.forceLegacyListing)) {
@@ -388,7 +386,15 @@ class LifecycleBucketProcessor {
388386
value: result,
389387
s3target: s3Client,
390388
backbeatMetadataProxy,
391-
}, cb);
389+
}, (err) => {
390+
// Increment metric after task successfully completes
391+
// This ensures we count actual processing completion, not just dispatch
392+
// Only count successful completions, not failures
393+
if (!err) {
394+
LifecycleMetrics.onBucketProcessorBucketDone(this._log);
395+
}
396+
return cb(err);
397+
});
392398
});
393399
}
394400

extensions/lifecycle/conductor/LifecycleConductor.js

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,8 @@ class LifecycleConductor {
358358

359359
_createBucketTaskMessages(tasks, scanId, log, cb) {
360360
if (this.lcConfig.forceLegacyListing) {
361-
return process.nextTick(cb, null, tasks.map(t => this._taskToMessage(t, lifecycleTaskVersions.v1, scanId, log)));
361+
return process.nextTick(cb, null, tasks.map(t =>
362+
this._taskToMessage(t, lifecycleTaskVersions.v1, scanId, log)));
362363
}
363364

364365
return async.mapLimit(tasks, 10, (t, taskDone) =>
@@ -460,13 +461,6 @@ class LifecycleConductor {
460461
},
461462
], err => {
462463
const elapsedMs = Date.now() - start;
463-
LifecycleMetrics.onConductorFullScan(
464-
log,
465-
elapsedMs,
466-
nBucketsListed,
467-
nBucketsQueued,
468-
nLifecycledBuckets
469-
);
470464
if (err && err.Throttling) {
471465
log.info('not starting new lifecycle batch', {
472466
reason: err,
@@ -485,20 +479,29 @@ class LifecycleConductor {
485479
const unknownCanonicalIds = this._accountIdCache.getMisses();
486480

487481
if (err) {
488-
log.error('lifecycle batch failed', {
489-
error: err,
490-
unknownCanonicalIds,
491-
conductorScanId: scanId,
492-
fullScanElapsedMs: elapsedMs,
493-
nBucketsListed,
494-
workflowsQueued: messageDeliveryReports,
495-
});
482+
log.error('lifecycle batch failed', {
483+
error: err,
484+
unknownCanonicalIds,
485+
conductorScanId: scanId,
486+
fullScanElapsedMs: elapsedMs,
487+
nBucketsListed,
488+
workflowsQueued: messageDeliveryReports,
489+
});
496490
if (cb) {
497491
cb(err);
498492
}
499493
return;
500494
}
501495

496+
// Only record metrics after a real scan completes (not throttled)
497+
LifecycleMetrics.onConductorFullScan(
498+
log,
499+
elapsedMs,
500+
nBucketsListed,
501+
nBucketsQueued,
502+
nLifecycledBuckets
503+
);
504+
502505
log.info('finished pushing lifecycle batch', {
503506
nBucketsQueued,
504507
nLifecycledBuckets,

0 commit comments

Comments
 (0)