Skip to content

Commit 3b0728f

Browse files
Lms24Lms24
authored
fix(node-core): Handle custom scope in log messages without parameters (#18322)
Fixes a bug reported in #18318 where ```js Sentry.logger.info('Test message', { key: 'value' }, { scope: customScope }); ``` would not forward the `customScope` but instead drop the log all together.
1 parent f31b899 commit 3b0728f

File tree

4 files changed

+202
-2
lines changed

4 files changed

+202
-2
lines changed

‎dev-packages/node-core-integration-tests/suites/public-api/logs/subject.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import * as Sentry from '@sentry/node-core';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
const client = new Sentry.NodeClient({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
transport: loggingTransport,
7+
stackParser: Sentry.defaultStackParser,
8+
integrations: [],
9+
enableLogs: true,
10+
sendDefaultPii: true,
11+
});
12+
13+
const customScope = new Sentry.Scope();
14+
customScope.setClient(client);
15+
customScope.update({ user: { username: 'h4cktor' } });
16+
client.init();
17+
18+
async function run(): Promise<void> {
19+
Sentry.logger.info('test info', { foo: 'bar1' }, { scope: customScope });
20+
Sentry.logger.info('test info with %d', [1], { foo: 'bar2' }, { scope: customScope });
21+
Sentry.logger.info(Sentry.logger.fmt`test info with fmt ${1}`, { foo: 'bar3' }, { scope: customScope });
22+
23+
await Sentry.flush();
24+
}
25+
26+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
27+
void run();

‎dev-packages/node-core-integration-tests/suites/public-api/logs/test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { afterAll, describe, expect, test } from 'vitest';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
describe('logger public API', () => {
5+
afterAll(() => {
6+
cleanupChildProcesses();
7+
});
8+
9+
test('captures logs with custom scopes and parameters in different forms', async () => {
10+
const runner = createRunner(__dirname, 'subject.ts')
11+
.expect({
12+
log: {
13+
items: [
14+
{
15+
attributes: {
16+
foo: {
17+
type: 'string',
18+
value: 'bar1',
19+
},
20+
'sentry.sdk.name': {
21+
type: 'string',
22+
value: 'sentry.javascript.node',
23+
},
24+
'sentry.sdk.version': {
25+
type: 'string',
26+
value: expect.any(String),
27+
},
28+
'server.address': {
29+
type: 'string',
30+
value: 'M6QX4Q5HKV.local',
31+
},
32+
'user.name': {
33+
type: 'string',
34+
value: 'h4cktor',
35+
},
36+
},
37+
body: 'test info',
38+
level: 'info',
39+
severity_number: 9,
40+
timestamp: expect.any(Number),
41+
trace_id: expect.stringMatching(/^[\da-f]{32}$/),
42+
},
43+
{
44+
attributes: {
45+
foo: {
46+
type: 'string',
47+
value: 'bar2',
48+
},
49+
'sentry.message.parameter.0': {
50+
type: 'integer',
51+
value: 1,
52+
},
53+
'sentry.message.template': {
54+
type: 'string',
55+
value: 'test info with %d',
56+
},
57+
'sentry.sdk.name': {
58+
type: 'string',
59+
value: 'sentry.javascript.node',
60+
},
61+
'sentry.sdk.version': {
62+
type: 'string',
63+
value: expect.any(String),
64+
},
65+
'server.address': {
66+
type: 'string',
67+
value: 'M6QX4Q5HKV.local',
68+
},
69+
'user.name': {
70+
type: 'string',
71+
value: 'h4cktor',
72+
},
73+
},
74+
body: 'test info with 1',
75+
level: 'info',
76+
severity_number: 9,
77+
timestamp: expect.any(Number),
78+
trace_id: expect.stringMatching(/^[\da-f]{32}$/),
79+
},
80+
{
81+
attributes: {
82+
foo: {
83+
type: 'string',
84+
value: 'bar3',
85+
},
86+
'sentry.message.parameter.0': {
87+
type: 'integer',
88+
value: 1,
89+
},
90+
'sentry.message.template': {
91+
type: 'string',
92+
value: 'test info with fmt %s',
93+
},
94+
'sentry.sdk.name': {
95+
type: 'string',
96+
value: 'sentry.javascript.node',
97+
},
98+
'sentry.sdk.version': {
99+
type: 'string',
100+
value: expect.any(String),
101+
},
102+
'server.address': {
103+
type: 'string',
104+
value: 'M6QX4Q5HKV.local',
105+
},
106+
'user.name': {
107+
type: 'string',
108+
value: 'h4cktor',
109+
},
110+
},
111+
body: 'test info with fmt 1',
112+
level: 'info',
113+
severity_number: 9,
114+
timestamp: expect.any(Number),
115+
trace_id: expect.stringMatching(/^[\da-f]{32}$/),
116+
},
117+
],
118+
},
119+
})
120+
.start();
121+
122+
await runner.completed();
123+
});
124+
});

‎packages/node-core/src/logs/capture.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ export type CaptureLogArgs = CaptureLogArgWithTemplate | CaptureLogArgWithoutTem
3434
export function captureLog(level: LogSeverityLevel, ...args: CaptureLogArgs): void {
3535
const [messageOrMessageTemplate, paramsOrAttributes, maybeAttributesOrMetadata, maybeMetadata] = args;
3636
if (Array.isArray(paramsOrAttributes)) {
37-
const attributes = { ...(maybeAttributesOrMetadata as Log['attributes']) };
37+
// type-casting here because from the type definitions we know that `maybeAttributesOrMetadata` is an attributes object (or undefined)
38+
const attributes = { ...(maybeAttributesOrMetadata as Log['attributes'] | undefined) };
3839
attributes['sentry.message.template'] = messageOrMessageTemplate;
3940
paramsOrAttributes.forEach((param, index) => {
4041
attributes[`sentry.message.parameter.${index}`] = param;
@@ -44,7 +45,8 @@ export function captureLog(level: LogSeverityLevel, ...args: CaptureLogArgs): vo
4445
} else {
4546
_INTERNAL_captureLog(
4647
{ level, message: messageOrMessageTemplate, attributes: paramsOrAttributes },
47-
maybeMetadata?.scope,
48+
// type-casting here because from the type definitions we know that `maybeAttributesOrMetadata` is a metadata object (or undefined)
49+
(maybeAttributesOrMetadata as CaptureLogMetadata | undefined)?.scope ?? maybeMetadata?.scope,
4850
);
4951
}
5052
}

‎packages/node-core/test/logs/exports.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as sentryCore from '@sentry/core';
2+
import { Scope } from '@sentry/core';
23
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
34
import * as nodeLogger from '../../src/logs/exports';
45

@@ -172,4 +173,50 @@ describe('Node Logger', () => {
172173
);
173174
});
174175
});
176+
177+
describe('scustom cope', () => {
178+
it('calls _INTERNAL_captureLog with custom scope for basic log message', () => {
179+
const customScope = new Scope();
180+
nodeLogger.debug('User logged in', undefined, { scope: customScope });
181+
expect(mockCaptureLog).toHaveBeenCalledWith(
182+
{
183+
level: 'debug',
184+
message: 'User logged in',
185+
},
186+
customScope,
187+
);
188+
});
189+
190+
it('calls _INTERNAL_captureLog with custom scope for parametrized log message', () => {
191+
const customScope = new Scope();
192+
nodeLogger.debug('User %s logged in from %s', ['Alice', 'mobile'], undefined, { scope: customScope });
193+
expect(mockCaptureLog).toHaveBeenCalledWith(
194+
{
195+
level: 'debug',
196+
message: 'User Alice logged in from mobile',
197+
attributes: {
198+
'sentry.message.template': 'User %s logged in from %s',
199+
'sentry.message.parameter.0': 'Alice',
200+
'sentry.message.parameter.1': 'mobile',
201+
},
202+
},
203+
customScope,
204+
);
205+
});
206+
207+
it('calls _INTERNAL_captureLog with custom scope for fmt log message', () => {
208+
const customScope = new Scope();
209+
nodeLogger.debug(nodeLogger.fmt`User ${'Alice'} logged in from ${'mobile'}`, undefined, { scope: customScope });
210+
expect(mockCaptureLog).toHaveBeenCalledWith(
211+
{
212+
level: 'debug',
213+
message: expect.objectContaining({
214+
__sentry_template_string__: 'User %s logged in from %s',
215+
__sentry_template_values__: ['Alice', 'mobile'],
216+
}),
217+
},
218+
customScope,
219+
);
220+
});
221+
});
175222
});

0 commit comments

Comments
 (0)