-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(logs): Add internal replay_is_buffering
flag
#17752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d65681c
7112b90
ac2751a
2238e52
231ff56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -424,6 +424,7 @@ describe('_INTERNAL_captureLog', () => { | |
// Simulate behavior: return ID for sampled sessions | ||
return onlyIfSampled ? 'sampled-replay-id' : 'any-replay-id'; | ||
}), | ||
getRecordingMode: vi.fn(() => 'session'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
@@ -480,6 +481,7 @@ describe('_INTERNAL_captureLog', () => { | |
// Buffer mode should still return ID even with onlyIfSampled=true | ||
return 'buffer-replay-id'; | ||
}), | ||
getRecordingMode: vi.fn(() => 'buffer'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
@@ -494,6 +496,10 @@ describe('_INTERNAL_captureLog', () => { | |
value: 'buffer-replay-id', | ||
type: 'string', | ||
}, | ||
'sentry._internal.replay_is_buffering': { | ||
value: true, | ||
type: 'boolean', | ||
}, | ||
}); | ||
}); | ||
|
||
|
@@ -527,6 +533,7 @@ describe('_INTERNAL_captureLog', () => { | |
// Mock replay integration | ||
const mockReplayIntegration = { | ||
getReplayId: vi.fn(() => 'test-replay-id'), | ||
getRecordingMode: vi.fn(() => 'session'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
@@ -590,6 +597,196 @@ describe('_INTERNAL_captureLog', () => { | |
expect(logAttributes).not.toHaveProperty('sentry.replay_id'); | ||
}); | ||
}); | ||
|
||
it('sets replay_is_buffering attribute when replay is in buffer mode', () => { | ||
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableLogs: true }); | ||
const client = new TestClient(options); | ||
const scope = new Scope(); | ||
scope.setClient(client); | ||
|
||
// Mock replay integration with buffer mode | ||
const mockReplayIntegration = { | ||
getReplayId: vi.fn(() => 'buffer-replay-id'), | ||
getRecordingMode: vi.fn(() => 'buffer'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
||
_INTERNAL_captureLog({ level: 'info', message: 'test log with buffered replay' }, scope); | ||
|
||
expect(mockReplayIntegration.getReplayId).toHaveBeenCalledWith(true); | ||
expect(mockReplayIntegration.getRecordingMode).toHaveBeenCalled(); | ||
|
||
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; | ||
expect(logAttributes).toEqual({ | ||
'sentry.replay_id': { | ||
value: 'buffer-replay-id', | ||
type: 'string', | ||
}, | ||
'sentry._internal.replay_is_buffering': { | ||
value: true, | ||
type: 'boolean', | ||
}, | ||
}); | ||
}); | ||
|
||
it('does not set replay_is_buffering attribute when replay is in session mode', () => { | ||
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableLogs: true }); | ||
const client = new TestClient(options); | ||
const scope = new Scope(); | ||
scope.setClient(client); | ||
|
||
// Mock replay integration with session mode | ||
const mockReplayIntegration = { | ||
getReplayId: vi.fn(() => 'session-replay-id'), | ||
getRecordingMode: vi.fn(() => 'session'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
||
_INTERNAL_captureLog({ level: 'info', message: 'test log with session replay' }, scope); | ||
|
||
expect(mockReplayIntegration.getReplayId).toHaveBeenCalledWith(true); | ||
expect(mockReplayIntegration.getRecordingMode).toHaveBeenCalled(); | ||
|
||
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; | ||
expect(logAttributes).toEqual({ | ||
'sentry.replay_id': { | ||
value: 'session-replay-id', | ||
type: 'string', | ||
}, | ||
}); | ||
expect(logAttributes).not.toHaveProperty('sentry._internal.replay_is_buffering'); | ||
}); | ||
|
||
it('does not set replay_is_buffering attribute when replay is undefined mode', () => { | ||
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableLogs: true }); | ||
const client = new TestClient(options); | ||
const scope = new Scope(); | ||
scope.setClient(client); | ||
|
||
// Mock replay integration with undefined mode (replay stopped/disabled) | ||
const mockReplayIntegration = { | ||
getReplayId: vi.fn(() => 'stopped-replay-id'), | ||
getRecordingMode: vi.fn(() => undefined), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
||
_INTERNAL_captureLog({ level: 'info', message: 'test log with stopped replay' }, scope); | ||
|
||
expect(mockReplayIntegration.getReplayId).toHaveBeenCalledWith(true); | ||
expect(mockReplayIntegration.getRecordingMode).toHaveBeenCalled(); | ||
|
||
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; | ||
expect(logAttributes).toEqual({ | ||
'sentry.replay_id': { | ||
value: 'stopped-replay-id', | ||
type: 'string', | ||
}, | ||
}); | ||
expect(logAttributes).not.toHaveProperty('sentry._internal.replay_is_buffering'); | ||
}); | ||
|
||
it('does not set replay_is_buffering attribute when no replay ID is available', () => { | ||
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableLogs: true }); | ||
const client = new TestClient(options); | ||
const scope = new Scope(); | ||
scope.setClient(client); | ||
|
||
// Mock replay integration that returns no replay ID but has buffer mode | ||
const mockReplayIntegration = { | ||
getReplayId: vi.fn(() => undefined), | ||
getRecordingMode: vi.fn(() => 'buffer'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
||
_INTERNAL_captureLog({ level: 'info', message: 'test log with buffer mode but no replay ID' }, scope); | ||
|
||
expect(mockReplayIntegration.getReplayId).toHaveBeenCalledWith(true); | ||
// getRecordingMode should not be called if there's no replay ID | ||
expect(mockReplayIntegration.getRecordingMode).not.toHaveBeenCalled(); | ||
|
||
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; | ||
expect(logAttributes).toEqual({}); | ||
expect(logAttributes).not.toHaveProperty('sentry.replay_id'); | ||
expect(logAttributes).not.toHaveProperty('sentry.internal.replay_is_buffering'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Test Mismatch: Incorrect Attribute VerificationTests verifying the Additional Locations (1) |
||
}); | ||
|
||
it('does not set replay_is_buffering attribute when replay integration is missing', () => { | ||
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, enableLogs: true }); | ||
const client = new TestClient(options); | ||
const scope = new Scope(); | ||
scope.setClient(client); | ||
|
||
// Mock no replay integration found | ||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(undefined); | ||
|
||
_INTERNAL_captureLog({ level: 'info', message: 'test log without replay integration' }, scope); | ||
|
||
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; | ||
expect(logAttributes).toEqual({}); | ||
expect(logAttributes).not.toHaveProperty('sentry.replay_id'); | ||
expect(logAttributes).not.toHaveProperty('sentry._internal.replay_is_buffering'); | ||
}); | ||
|
||
it('combines replay_is_buffering with other replay attributes', () => { | ||
const options = getDefaultTestClientOptions({ | ||
dsn: PUBLIC_DSN, | ||
enableLogs: true, | ||
release: '1.0.0', | ||
environment: 'test', | ||
}); | ||
const client = new TestClient(options); | ||
const scope = new Scope(); | ||
scope.setClient(client); | ||
|
||
// Mock replay integration with buffer mode | ||
const mockReplayIntegration = { | ||
getReplayId: vi.fn(() => 'buffer-replay-id'), | ||
getRecordingMode: vi.fn(() => 'buffer'), | ||
}; | ||
|
||
vi.spyOn(client, 'getIntegrationByName').mockReturnValue(mockReplayIntegration as any); | ||
|
||
_INTERNAL_captureLog( | ||
{ | ||
level: 'info', | ||
message: 'test log with buffer replay and other attributes', | ||
attributes: { component: 'auth', action: 'login' }, | ||
}, | ||
scope, | ||
); | ||
|
||
const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes; | ||
expect(logAttributes).toEqual({ | ||
component: { | ||
value: 'auth', | ||
type: 'string', | ||
}, | ||
action: { | ||
value: 'login', | ||
type: 'string', | ||
}, | ||
'sentry.release': { | ||
value: '1.0.0', | ||
type: 'string', | ||
}, | ||
'sentry.environment': { | ||
value: 'test', | ||
type: 'string', | ||
}, | ||
'sentry.replay_id': { | ||
value: 'buffer-replay-id', | ||
type: 'string', | ||
}, | ||
'sentry._internal.replay_is_buffering': { | ||
value: true, | ||
type: 'boolean', | ||
}, | ||
}); | ||
}); | ||
}); | ||
|
||
describe('user functionality', () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AbhiPrasad I kept the inlined types here, I assume we have no access to the replay types in core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could extract the types into core to make this a more "public" API, it also helps to keep this consistent. I'd just do it in another PR though.