From ecc6cb26af246e0e20ee525562ea53d1a8c78c1c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 27 May 2025 20:15:01 -0400 Subject: [PATCH 1/3] feat(core): Add user to logs --- packages/core/src/logs/exports.ts | 41 ++- packages/core/test/lib/logs/exports.test.ts | 298 ++++++++++++++++++++ 2 files changed, 335 insertions(+), 4 deletions(-) diff --git a/packages/core/src/logs/exports.ts b/packages/core/src/logs/exports.ts index 9a738d503a80..af473f928ec2 100644 --- a/packages/core/src/logs/exports.ts +++ b/packages/core/src/logs/exports.ts @@ -1,8 +1,10 @@ import type { Client } from '../client'; import { _getTraceInfoFromScope } from '../client'; -import { getClient, getCurrentScope } from '../currentScopes'; +import { getClient, getCurrentScope, getGlobalScope, getIsolationScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; +import type { Scope, ScopeData } from '../scope'; import type { Log, SerializedLog, SerializedLogAttributeValue } from '../types-hoist/log'; +import { mergeScopeData } from '../utils/applyScopeDataToEvent'; import { _getSpanForScope } from '../utils/spanOnScope'; import { isParameterizedString } from '../utils-hoist/is'; import { logger } from '../utils-hoist/logger'; @@ -93,10 +95,11 @@ export function _INTERNAL_captureSerializedLog(client: Client, serializedLog: Se * @experimental This method will experience breaking changes. This is not yet part of * the stable Sentry SDK API and can be changed or removed without warning. */ +// eslint-disable-next-line complexity export function _INTERNAL_captureLog( beforeLog: Log, client: Client | undefined = getClient(), - scope = getCurrentScope(), + currentScope = getCurrentScope(), captureSerializedLog: (client: Client, log: SerializedLog) => void = _INTERNAL_captureSerializedLog, ): void { if (!client) { @@ -111,12 +114,27 @@ export function _INTERNAL_captureLog( return; } - const [, traceContext] = _getTraceInfoFromScope(client, scope); + const [, traceContext] = _getTraceInfoFromScope(client, currentScope); const processedLogAttributes = { ...beforeLog.attributes, }; + const { user } = getScopeData(currentScope); + // Only attach user to log attributes if sendDefaultPii is enabled + if (client.getOptions().sendDefaultPii) { + const { id, email, username } = user; + if (id && !processedLogAttributes['user.id']) { + processedLogAttributes['user.id'] = id; + } + if (email && !processedLogAttributes['user.email']) { + processedLogAttributes['user.email'] = email; + } + if (username && !processedLogAttributes['user.name']) { + processedLogAttributes['user.name'] = username; + } + } + if (release) { processedLogAttributes['sentry.release'] = release; } @@ -140,7 +158,7 @@ export function _INTERNAL_captureLog( }); } - const span = _getSpanForScope(scope); + const span = _getSpanForScope(currentScope); if (span) { // Add the parent span ID to the log attributes for trace context processedLogAttributes['sentry.trace.parent_span_id'] = span.spanContext().spanId; @@ -218,3 +236,18 @@ export function _INTERNAL_flushLogsBuffer(client: Client, maybeLogBuffer?: Array export function _INTERNAL_getLogBuffer(client: Client): Array | undefined { return GLOBAL_OBJ._sentryClientToLogBufferMap?.get(client); } + +/** + * Get the scope data for the current scope. + * @param currentScope - The current scope. + * @returns The scope data. + */ +function getScopeData(currentScope: Scope): ScopeData { + const scopeData = getGlobalScope().getScopeData(); + const isolationScope = getIsolationScope(); + if (isolationScope) { + mergeScopeData(scopeData, isolationScope.getScopeData()); + } + mergeScopeData(scopeData, currentScope.getScopeData()); + return scopeData; +} diff --git a/packages/core/test/lib/logs/exports.test.ts b/packages/core/test/lib/logs/exports.test.ts index 1ae570bc5968..008d33b780c9 100644 --- a/packages/core/test/lib/logs/exports.test.ts +++ b/packages/core/test/lib/logs/exports.test.ts @@ -375,4 +375,302 @@ describe('_INTERNAL_captureLog', () => { expect(beforeCaptureLogSpy).toHaveBeenCalledWith('afterCaptureLog', log); beforeCaptureLogSpy.mockRestore(); }); + + describe('user functionality', () => { + it('includes user data in log attributes when sendDefaultPii is enabled', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: '123', + email: 'user@example.com', + username: 'testuser', + }); + + _INTERNAL_captureLog({ level: 'info', message: 'test log with user' }, client, scope); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'user.id': { + value: '123', + type: 'string', + }, + 'user.email': { + value: 'user@example.com', + type: 'string', + }, + 'user.name': { + value: 'testuser', + type: 'string', + }, + }); + }); + + it('does not include user data in log attributes when sendDefaultPii is disabled', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: false, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: '123', + email: 'user@example.com', + username: 'testuser', + }); + + _INTERNAL_captureLog({ level: 'info', message: 'test log without user' }, client, scope); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({}); + }); + + it('includes partial user data when only some fields are available', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: '123', + // email and username are missing + }); + + _INTERNAL_captureLog({ level: 'info', message: 'test log with partial user' }, client, scope); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'user.id': { + value: '123', + type: 'string', + }, + }); + }); + + it('includes user email and username without id', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + email: 'user@example.com', + username: 'testuser', + // id is missing + }); + + _INTERNAL_captureLog({ level: 'info', message: 'test log with email and username' }, client, scope); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'user.email': { + value: 'user@example.com', + type: 'string', + }, + 'user.name': { + value: 'testuser', + type: 'string', + }, + }); + }); + + it('does not include user data when user object is empty', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({}); + + _INTERNAL_captureLog({ level: 'info', message: 'test log with empty user' }, client, scope); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({}); + }); + + it('combines user data with other log attributes', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + release: '1.0.0', + environment: 'test', + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: '123', + email: 'user@example.com', + }); + + _INTERNAL_captureLog( + { + level: 'info', + message: 'test log with user and other attributes', + attributes: { component: 'auth', action: 'login' }, + }, + client, + scope, + ); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + component: { + value: 'auth', + type: 'string', + }, + action: { + value: 'login', + type: 'string', + }, + 'user.id': { + value: '123', + type: 'string', + }, + 'user.email': { + value: 'user@example.com', + type: 'string', + }, + 'sentry.release': { + value: '1.0.0', + type: 'string', + }, + 'sentry.environment': { + value: 'test', + type: 'string', + }, + }); + }); + + it('handles user data with non-string values', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: 123, // number instead of string + email: 'user@example.com', + username: undefined, // undefined value + }); + + _INTERNAL_captureLog({ level: 'info', message: 'test log with non-string user values' }, client, scope); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'user.id': { + value: 123, + type: 'integer', + }, + 'user.email': { + value: 'user@example.com', + type: 'string', + }, + }); + }); + + it('preserves existing user attributes in log and does not override them', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: '123', + email: 'user@example.com', + }); + + _INTERNAL_captureLog( + { + level: 'info', + message: 'test log with existing user attributes', + attributes: { + 'user.id': 'existing-id', // This should NOT be overridden by scope user + 'user.custom': 'custom-value', // This should be preserved + }, + }, + client, + scope, + ); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'user.custom': { + value: 'custom-value', + type: 'string', + }, + 'user.id': { + value: 'existing-id', // Existing value is preserved + type: 'string', + }, + 'user.email': { + value: 'user@example.com', // Only added because user.email wasn't already present + type: 'string', + }, + }); + }); + + it('only adds scope user data for attributes that do not already exist', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + sendDefaultPii: true, + }); + const client = new TestClient(options); + const scope = new Scope(); + scope.setUser({ + id: 'scope-id', + email: 'scope@example.com', + username: 'scope-user', + }); + + _INTERNAL_captureLog( + { + level: 'info', + message: 'test log with partial existing user attributes', + attributes: { + 'user.email': 'existing@example.com', // This should be preserved + 'other.attr': 'value', + }, + }, + client, + scope, + ); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'other.attr': { + value: 'value', + type: 'string', + }, + 'user.email': { + value: 'existing@example.com', // Existing email is preserved + type: 'string', + }, + 'user.id': { + value: 'scope-id', // Added from scope because not present + type: 'string', + }, + 'user.name': { + value: 'scope-user', // Added from scope because not present + type: 'string', + }, + }); + }); + }); }); From 4f6f3017df07ceb2c7eeb5425559e5a82867db1c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 May 2025 21:05:37 -0400 Subject: [PATCH 2/3] address PR feedback --- packages/core/src/logs/exports.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/core/src/logs/exports.ts b/packages/core/src/logs/exports.ts index af473f928ec2..c4abad21c60f 100644 --- a/packages/core/src/logs/exports.ts +++ b/packages/core/src/logs/exports.ts @@ -120,7 +120,7 @@ export function _INTERNAL_captureLog( ...beforeLog.attributes, }; - const { user } = getScopeData(currentScope); + const { user } = getMergedScopeData(currentScope); // Only attach user to log attributes if sendDefaultPii is enabled if (client.getOptions().sendDefaultPii) { const { id, email, username } = user; @@ -238,16 +238,15 @@ export function _INTERNAL_getLogBuffer(client: Client): Array | u } /** - * Get the scope data for the current scope. + * Get the scope data for the current scope after merging with the + * global scope and isolation scope. + * * @param currentScope - The current scope. * @returns The scope data. */ -function getScopeData(currentScope: Scope): ScopeData { +function getMergedScopeData(currentScope: Scope): ScopeData { const scopeData = getGlobalScope().getScopeData(); - const isolationScope = getIsolationScope(); - if (isolationScope) { - mergeScopeData(scopeData, isolationScope.getScopeData()); - } + mergeScopeData(scopeData, getIsolationScope().getScopeData()); mergeScopeData(scopeData, currentScope.getScopeData()); return scopeData; } From bd72d4f05fac39f6fdff87b83f6a334428409562 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 28 May 2025 21:17:36 -0400 Subject: [PATCH 3/3] refactor log attribute setting --- packages/core/src/logs/exports.ts | 55 ++++++++++--------- packages/core/test/lib/logs/exports.test.ts | 60 +++++++++++++++++++++ 2 files changed, 89 insertions(+), 26 deletions(-) diff --git a/packages/core/src/logs/exports.ts b/packages/core/src/logs/exports.ts index c4abad21c60f..f44817c13715 100644 --- a/packages/core/src/logs/exports.ts +++ b/packages/core/src/logs/exports.ts @@ -63,6 +63,25 @@ export function logAttributeToSerializedLogAttribute(value: unknown): Serialized } } +/** + * Sets a log attribute if the value exists and the attribute key is not already present. + * + * @param logAttributes - The log attributes object to modify. + * @param key - The attribute key to set. + * @param value - The value to set (only sets if truthy and key not present). + * @param setEvenIfPresent - Whether to set the attribute if it is present. Defaults to true. + */ +function setLogAttribute( + logAttributes: Record, + key: string, + value: unknown, + setEvenIfPresent = true, +): void { + if (value && (!logAttributes[key] || setEvenIfPresent)) { + logAttributes[key] = value; + } +} + /** * Captures a serialized log event and adds it to the log buffer for the given client. * @@ -95,7 +114,6 @@ export function _INTERNAL_captureSerializedLog(client: Client, serializedLog: Se * @experimental This method will experience breaking changes. This is not yet part of * the stable Sentry SDK API and can be changed or removed without warning. */ -// eslint-disable-next-line complexity export function _INTERNAL_captureLog( beforeLog: Log, client: Client | undefined = getClient(), @@ -124,30 +142,17 @@ export function _INTERNAL_captureLog( // Only attach user to log attributes if sendDefaultPii is enabled if (client.getOptions().sendDefaultPii) { const { id, email, username } = user; - if (id && !processedLogAttributes['user.id']) { - processedLogAttributes['user.id'] = id; - } - if (email && !processedLogAttributes['user.email']) { - processedLogAttributes['user.email'] = email; - } - if (username && !processedLogAttributes['user.name']) { - processedLogAttributes['user.name'] = username; - } + setLogAttribute(processedLogAttributes, 'user.id', id, false); + setLogAttribute(processedLogAttributes, 'user.email', email, false); + setLogAttribute(processedLogAttributes, 'user.name', username, false); } - if (release) { - processedLogAttributes['sentry.release'] = release; - } + setLogAttribute(processedLogAttributes, 'sentry.release', release); + setLogAttribute(processedLogAttributes, 'sentry.environment', environment); - if (environment) { - processedLogAttributes['sentry.environment'] = environment; - } - - const { sdk } = client.getSdkMetadata() ?? {}; - if (sdk) { - processedLogAttributes['sentry.sdk.name'] = sdk.name; - processedLogAttributes['sentry.sdk.version'] = sdk.version; - } + const { name, version } = client.getSdkMetadata()?.sdk ?? {}; + setLogAttribute(processedLogAttributes, 'sentry.sdk.name', name); + setLogAttribute(processedLogAttributes, 'sentry.sdk.version', version); const beforeLogMessage = beforeLog.message; if (isParameterizedString(beforeLogMessage)) { @@ -159,10 +164,8 @@ export function _INTERNAL_captureLog( } const span = _getSpanForScope(currentScope); - if (span) { - // Add the parent span ID to the log attributes for trace context - processedLogAttributes['sentry.trace.parent_span_id'] = span.spanContext().spanId; - } + // Add the parent span ID to the log attributes for trace context + setLogAttribute(processedLogAttributes, 'sentry.trace.parent_span_id', span?.spanContext().spanId); const processedLog = { ...beforeLog, attributes: processedLogAttributes }; diff --git a/packages/core/test/lib/logs/exports.test.ts b/packages/core/test/lib/logs/exports.test.ts index 008d33b780c9..8c1fe4d8e76f 100644 --- a/packages/core/test/lib/logs/exports.test.ts +++ b/packages/core/test/lib/logs/exports.test.ts @@ -673,4 +673,64 @@ describe('_INTERNAL_captureLog', () => { }); }); }); + + it('overrides user-provided system attributes with SDK values', () => { + const options = getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + _experiments: { enableLogs: true }, + release: 'sdk-release-1.0.0', + environment: 'sdk-environment', + }); + const client = new TestClient(options); + + // Mock getSdkMetadata to return SDK info + vi.spyOn(client, 'getSdkMetadata').mockReturnValue({ + sdk: { + name: 'sentry.javascript.node', + version: '7.0.0', + }, + }); + + const scope = new Scope(); + + _INTERNAL_captureLog( + { + level: 'info', + message: 'test log with user-provided system attributes', + attributes: { + 'sentry.release': 'user-release-2.0.0', + 'sentry.environment': 'user-environment', + 'sentry.sdk.name': 'user-sdk-name', + 'sentry.sdk.version': 'user-sdk-version', + 'user.custom': 'preserved-value', + }, + }, + client, + scope, + ); + + const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; + expect(logAttributes).toEqual({ + 'user.custom': { + value: 'preserved-value', + type: 'string', + }, + 'sentry.release': { + value: 'sdk-release-1.0.0', + type: 'string', + }, + 'sentry.environment': { + value: 'sdk-environment', + type: 'string', + }, + 'sentry.sdk.name': { + value: 'sentry.javascript.node', + type: 'string', + }, + 'sentry.sdk.version': { + value: '7.0.0', + type: 'string', + }, + }); + }); });