Skip to content

Commit 85211d6

Browse files
authored
Fixed email analytics timing log metrics (#27403)
no ref `apiPollingTimeMs` was wrapping the entire `provider.fetchLatest()` call, which executes `processBatch` (processing + intermediate aggregation) inside Mailgun's pagination loop. This meant API time included processing and aggregation time, causing reported percentages to exceed 100%. Processing and aggregation timers were already measured independently and correctly; only the API timer was wrong.
1 parent 0599597 commit 85211d6

1 file changed

Lines changed: 8 additions & 5 deletions

File tree

ghost/core/core/server/services/email-analytics/email-analytics-service.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ module.exports = class EmailAnalyticsService {
320320
this.queries.setJobTimestamp(fetchData.jobName, 'started', begin);
321321

322322
// Timing metrics
323-
let apiPollingTimeMs = 0;
323+
const fetchStartMs = Date.now();
324324
let processingTimeMs = 0;
325325
let aggregationTimeMs = 0;
326326

@@ -386,9 +386,9 @@ module.exports = class EmailAnalyticsService {
386386
// Aggregate and clear the processingResult
387387
// We do this here because otherwise it could take a long time before the new events are visible in the stats
388388
try {
389-
const aggregationStart = Date.now();
389+
const intermediateAggregationStart = Date.now();
390390
await this.aggregateStats(processingResult, includeOpenedEvents);
391-
aggregationTimeMs += (Date.now() - aggregationStart);
391+
aggregationTimeMs += (Date.now() - intermediateAggregationStart);
392392
lastAggregation = Date.now();
393393
// Remove aggregated emailIds and memberIds from tracking sets to avoid re-aggregating at the end
394394
processingResult.emailIds.forEach(id => allEmailIds.delete(id));
@@ -409,9 +409,7 @@ module.exports = class EmailAnalyticsService {
409409

410410
try {
411411
for (const provider of this.providers) {
412-
const apiStart = Date.now();
413412
await provider.fetchLatest(processBatch, {begin, end, maxEvents, events: eventTypes});
414-
apiPollingTimeMs += (Date.now() - apiStart);
415413
}
416414
} catch (err) {
417415
if (err.message !== 'Fetching canceled') {
@@ -464,6 +462,11 @@ module.exports = class EmailAnalyticsService {
464462

465463
fetchData.running = false;
466464

465+
const totalTimeMs = Date.now() - fetchStartMs;
466+
// Derived by subtraction because fetchLatest() invokes processBatch internally,
467+
// so directly timing fetchLatest() would double-count processing and aggregation time.
468+
const apiPollingTimeMs = totalTimeMs - processingTimeMs - aggregationTimeMs;
469+
467470
if (error) {
468471
throw error;
469472
}

0 commit comments

Comments
 (0)