Skip to content

Commit f9e74bb

Browse files
authored
Converted initial outbox logs to structured logging (#26599)
refs https://linear.app/ghost/issue/NY-1020/convert-outbox-logs-to-structured-logs ## Summary - converted the outbox missing-slug warning and outbox already-running guard logs to structured logging with a top-level `system` object and message second argument - added red/green unit coverage that captures real `@tryghost/logging` Bunyan JSON output in-memory and asserts structured fields - extracted shared test log capture utility (`ghost/core/test/utils/logging-utils.js`) with JSDoc and updated outbox unit tests to use top-level capture hooks ## Reviewer notes Mostly I'm looking for feedback on the new testing pattern introduced here. The new `captureLoggerOutput()` test helper redirects the actual JSON logs emitted by @tryghost/logging into an in-memory stream. This way we can assert on the _actual_ JSON that is output by the logger, which feels more useful to me than asserting on which arguments we pass to the logger. This doesn't cover all the logs that I want to change to structured logs for the referenced issue, but I wanted to align on the pattern here while keeping the PR relatively small; once we're aligned on the approach I can extend it to the other outbox/welcome email logs in a follow up PR.
1 parent bb5be3b commit f9e74bb

File tree

5 files changed

+124
-22
lines changed

5 files changed

+124
-22
lines changed

ghost/core/core/server/services/outbox/handlers/member-created.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ async function handle({payload}) {
1212
try {
1313
const slug = MEMBER_WELCOME_EMAIL_SLUGS[payload.status];
1414
if (!slug) {
15-
logging.warn(`${LOG_KEY} No automated email slug found for member status: ${payload.status}`);
15+
logging.warn({
16+
system: {
17+
event: 'outbox.member_created.no_slug_mapping',
18+
member_status: payload.status
19+
}
20+
}, `${LOG_KEY} No automated email slug found for member status`);
1621
return;
1722
}
1823

@@ -31,9 +36,11 @@ async function handle({payload}) {
3136
});
3237
} catch (err) {
3338
logging.error({
34-
message: `${LOG_KEY} Failed to track automated email send`,
39+
system: {
40+
event: 'outbox.member_created.track_send_failed'
41+
},
3542
err
36-
});
43+
}, `${LOG_KEY} Failed to track automated email send`);
3744
}
3845
}
3946

ghost/core/core/server/services/outbox/index.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ class OutboxServiceWrapper {
2424

2525
async startProcessing() {
2626
if (this.processing) {
27-
logging.info(`${OUTBOX_LOG_KEY}: Outbox job already running, skipping`);
27+
logging.info({
28+
system: {
29+
event: 'outbox.processing.skipped_already_running'
30+
}
31+
}, `${OUTBOX_LOG_KEY}: Outbox job already running, skipping`);
2832
return;
2933
}
3034
this.processing = true;

ghost/core/test/unit/server/services/outbox/handlers/member-created.test.js

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
const assert = require('node:assert/strict');
22
const sinon = require('sinon');
33
const rewire = require('rewire');
4+
const {captureLoggerOutput, findByEvent} = require('../../../../../utils/logging-utils');
45

56
describe('member-created handler', function () {
67
let handler;
78
let memberWelcomeEmailServiceStub;
8-
let loggingStub;
99
let AutomatedEmailStub;
1010
let AutomatedEmailRecipientStub;
11+
let logCapture;
1112

1213
beforeEach(function () {
1314
handler = rewire('../../../../../../core/server/services/outbox/handlers/member-created.js');
@@ -18,11 +19,6 @@ describe('member-created handler', function () {
1819
}
1920
};
2021

21-
loggingStub = {
22-
warn: sinon.stub(),
23-
error: sinon.stub()
24-
};
25-
2622
AutomatedEmailStub = {
2723
findOne: sinon.stub().resolves({id: 'ae123'})
2824
};
@@ -31,13 +27,15 @@ describe('member-created handler', function () {
3127
add: sinon.stub().resolves()
3228
};
3329

30+
logCapture = captureLoggerOutput();
31+
3432
handler.__set__('memberWelcomeEmailService', memberWelcomeEmailServiceStub);
35-
handler.__set__('logging', loggingStub);
3633
handler.__set__('AutomatedEmail', AutomatedEmailStub);
3734
handler.__set__('AutomatedEmailRecipient', AutomatedEmailRecipientStub);
3835
});
3936

4037
afterEach(function () {
38+
logCapture.restore();
4139
sinon.restore();
4240
});
4341

@@ -55,7 +53,6 @@ describe('member-created handler', function () {
5553
});
5654

5755
sinon.assert.calledOnce(memberWelcomeEmailServiceStub.api.send);
58-
sinon.assert.calledOnce(loggingStub.error);
5956
});
6057

6158
it('logs error when tracking fails', async function () {
@@ -72,10 +69,8 @@ describe('member-created handler', function () {
7269
}
7370
});
7471

75-
sinon.assert.calledOnce(loggingStub.error);
76-
const errorCall = loggingStub.error.getCall(0);
77-
assert.ok(errorCall.args[0].message.includes('Failed to track automated email send'));
78-
assert.equal(errorCall.args[0].err, dbError);
72+
const errorLog = findByEvent(logCapture.output, 'outbox.member_created.track_send_failed');
73+
assert.ok(errorLog);
7974
});
8075

8176
it('logs warning when status has no slug mapping', async function () {
@@ -90,8 +85,12 @@ describe('member-created handler', function () {
9085
});
9186

9287
sinon.assert.calledOnce(memberWelcomeEmailServiceStub.api.send);
93-
sinon.assert.calledOnce(loggingStub.warn);
94-
assert.ok(loggingStub.warn.getCall(0).args[0].includes('No automated email slug found'));
88+
const warningLog = findByEvent(logCapture.output, 'outbox.member_created.no_slug_mapping');
89+
assert.ok(warningLog);
90+
assert.deepEqual(warningLog.system, {
91+
event: 'outbox.member_created.no_slug_mapping',
92+
member_status: 'comped'
93+
});
9594
sinon.assert.notCalled(AutomatedEmailRecipientStub.add);
9695
});
9796
});

ghost/core/test/unit/server/services/outbox/index.test.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,29 @@
1+
const assert = require('node:assert/strict');
12
const sinon = require('sinon');
23
const rewire = require('rewire');
34
const DomainEvents = require('@tryghost/domain-events');
5+
const {captureLoggerOutput, findByEvent} = require('../../../../utils/logging-utils');
46
const StartOutboxProcessingEvent = require('../../../../../core/server/services/outbox/events/start-outbox-processing-event');
57

68
describe('Outbox Service', function () {
79
let service;
810
let processOutboxStub;
9-
let loggingStub;
1011
let jobsStub;
12+
let logCapture;
1113

1214
beforeEach(function () {
1315
service = rewire('../../../../../core/server/services/outbox/index.js');
1416

1517
processOutboxStub = sinon.stub().resolves('Processed');
16-
loggingStub = {info: sinon.stub(), error: sinon.stub()};
1718
jobsStub = {scheduleOutboxJob: sinon.stub()};
19+
logCapture = captureLoggerOutput();
1820

1921
service.__set__('processOutbox', processOutboxStub);
20-
service.__set__('logging', loggingStub);
2122
service.__set__('jobs', jobsStub);
2223
});
2324

2425
afterEach(async function () {
26+
logCapture.restore();
2527
sinon.restore();
2628
await DomainEvents.allSettled();
2729
});
@@ -46,7 +48,11 @@ describe('Outbox Service', function () {
4648
await service.startProcessing();
4749

4850
sinon.assert.notCalled(processOutboxStub);
49-
sinon.assert.calledOnce(loggingStub.info);
51+
const infoLog = findByEvent(logCapture.output, 'outbox.processing.skipped_already_running');
52+
assert.ok(infoLog);
53+
assert.deepEqual(infoLog.system, {
54+
event: 'outbox.processing.skipped_already_running'
55+
});
5056
});
5157
});
5258
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
const bunyan = require('bunyan');
2+
const {PassThrough} = require('stream');
3+
const logging = require('@tryghost/logging');
4+
5+
/**
6+
* Parse newline-delimited JSON log records from a buffered string.
7+
*
8+
* @param {string} buffer - Buffered log text that may contain multiple records.
9+
* @param {Array<object>} output - Collected parsed log records.
10+
* @returns {string} Remaining partial record that did not end with a newline.
11+
*/
12+
function parseBufferedJsonLogs(buffer, output) {
13+
const lines = buffer.split('\n');
14+
const remaining = lines.pop();
15+
16+
for (const line of lines) {
17+
if (!line.trim()) {
18+
continue;
19+
}
20+
21+
output.push(JSON.parse(line));
22+
}
23+
24+
return remaining;
25+
}
26+
27+
/**
28+
* Temporarily redirects Ghost logger streams to an in-memory Bunyan stream.
29+
*
30+
* This allows tests to assert on real serialized JSON output from
31+
* `@tryghost/logging` without stubbing logger methods.
32+
*
33+
* @returns {{output: Array<object>, restore: () => void}} Capture handle.
34+
*/
35+
function captureLoggerOutput() {
36+
const output = [];
37+
const stream = new PassThrough();
38+
let buffered = '';
39+
40+
stream.on('data', (chunk) => {
41+
buffered += chunk.toString();
42+
buffered = parseBufferedJsonLogs(buffered, output);
43+
});
44+
45+
const originalStreams = logging.streams;
46+
const originalSerializerSource = Object.values(originalStreams)[0]?.log?.serializers;
47+
logging.streams = {
48+
capture: {
49+
name: 'capture',
50+
log: bunyan.createLogger({
51+
name: 'test-logger',
52+
serializers: originalSerializerSource || logging.serializers,
53+
streams: [{
54+
type: 'stream',
55+
stream,
56+
level: 'trace'
57+
}]
58+
})
59+
}
60+
};
61+
62+
return {
63+
output,
64+
restore() {
65+
buffered = parseBufferedJsonLogs(buffered, output);
66+
logging.streams = originalStreams;
67+
stream.destroy();
68+
}
69+
};
70+
}
71+
72+
/**
73+
* Find the first structured log record matching a system event.
74+
*
75+
* @param {Array<object>} output - Captured log records.
76+
* @param {string} event - Structured event name to match.
77+
* @returns {object|undefined} First matching log record.
78+
*/
79+
function findByEvent(output, event) {
80+
return output.find(log => log.system?.event === event);
81+
}
82+
83+
module.exports = {
84+
captureLoggerOutput,
85+
findByEvent
86+
};

0 commit comments

Comments
 (0)