Skip to content
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

feat(replay): Add a replay-specific logger #13256

Merged
merged 8 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor
  • Loading branch information
billyvg committed Aug 7, 2024
commit 11415f55f413b05f021c2ef8b6d1261ecd1fb4b2
12 changes: 5 additions & 7 deletions packages/replay-internal/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,11 @@ export class ReplayContainer implements ReplayContainerInterface {
}

// Configure replay logger w/ experimental options
if (this._options._experiments.traceInternals) {
logger.enableTraceInternals();
}

if (this._options._experiments.captureExceptions) {
logger.enableCaptureInternalExceptions();
}
const experiments = options._experiments;
logger.setConfig({
captureExceptions: !!experiments.captureExceptions,
traceInternals: !!experiments.traceInternals,
});
}

/** Get the event context. */
Expand Down
57 changes: 29 additions & 28 deletions packages/replay-internal/src/util/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@ import { DEBUG_BUILD } from '../debug-build';

type ReplayConsoleLevels = Extract<ConsoleLevel, 'info' | 'warn' | 'error' | 'log'>;
const CONSOLE_LEVELS: readonly ReplayConsoleLevels[] = ['info', 'warn', 'error', 'log'] as const;
const PREFIX = '[Replay] ';

type LoggerMethod = (...args: unknown[]) => void;
type LoggerConsoleMethods = Record<'info' | 'warn' | 'error' | 'log', LoggerMethod>;
type LoggerConsoleMethods = Record<ReplayConsoleLevels, LoggerMethod>;

interface LoggerConfig {
captureExceptions: boolean;
traceInternals: boolean;
}

/** JSDoc */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** JSDoc */

interface ReplayLogger extends LoggerConsoleMethods {
Expand All @@ -21,13 +27,18 @@ interface ReplayLogger extends LoggerConsoleMethods {
* Captures exceptions (`Error`) if "capture internal exceptions" is enabled
*/
exception: LoggerMethod;
enableCaptureInternalExceptions(): void;
disableCaptureInternalExceptions(): void;
enableTraceInternals(): void;
disableTraceInternals(): void;
/**
* Configures the logger with additional debugging behavior
*/
setConfig(config: LoggerConfig): void;
}

function _addBreadcrumb(message: string, level: SeverityLevel = 'info'): void {
function _addBreadcrumb(message: unknown, level: SeverityLevel = 'info'): void {
// Only support strings for breadcrumbs
if (typeof message !== 'string') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: Do we need this check? IMHO we generally use this ourselves, worst case ${message} will convert whatever it is to a string anyhow, which I guess is OK here...?

return;
}

// Wait a tick here to avoid race conditions for some initial logs
// which may be added before replay is initialized
addBreadcrumb(
Expand All @@ -37,7 +48,7 @@ function _addBreadcrumb(message: string, level: SeverityLevel = 'info'): void {
logger: 'replay',
},
level,
message: `[Replay] ${message}`,
message: `${PREFIX} ${message}`,
},
{ level },
);
Expand All @@ -50,46 +61,36 @@ function makeReplayLogger(): ReplayLogger {
const _logger: Partial<ReplayLogger> = {
exception: () => undefined,
infoTick: () => undefined,
enableCaptureInternalExceptions: () => {
_capture = true;
},
disableCaptureInternalExceptions: () => {
_capture = false;
},
enableTraceInternals: () => {
_trace = true;
},
disableTraceInternals: () => {
_trace = false;
setConfig: (opts: LoggerConfig) => {
_capture = opts.captureExceptions;
_trace = opts.traceInternals;
},
};

if (DEBUG_BUILD) {
_logger.exception = (error: unknown) => {
coreLogger.error('[Replay] ', error);
coreLogger.error(PREFIX, error);

if (_capture) {
captureException(error);
}

// No need for a breadcrumb is `_capture` is enabled since it should be
// captured as an exception
if (_trace && !_capture) {
} else if (_trace) {
// No need for a breadcrumb is `_capture` is enabled since it should be
// captured as an exception
_addBreadcrumb(error instanceof Error ? error.message : 'Unknown error');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly we can skip this check, as Error should be casted to a reasonable string automatically when we do ${message}?

}
};

_logger.infoTick = (...args: unknown[]) => {
coreLogger.info('[Replay] ', ...args);
coreLogger.info(PREFIX, ...args);
if (_trace) {
setTimeout(() => typeof args[0] === 'string' && _addBreadcrumb(args[0]), 0);
setTimeout(() => _addBreadcrumb(args[0]), 0);
}
};

CONSOLE_LEVELS.forEach(name => {
_logger[name] = (...args: unknown[]) => {
coreLogger[name]('[Replay] ', ...args);
if (_trace && typeof args[0] === 'string') {
coreLogger[name](PREFIX, ...args);
if (_trace) {
_addBreadcrumb(args[0]);
}
};
Expand Down
9 changes: 4 additions & 5 deletions packages/replay-internal/test/integration/flush.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,7 @@ describe('Integration | flush', () => {
});

it('logs warning if flushing initial segment without checkout', async () => {
// replay.getOptions()._experiments.traceInternals = true;
logger.enableTraceInternals();
logger.setConfig({ traceInternals: true });

sessionStorage.clear();
clearSession(replay);
Expand Down Expand Up @@ -410,11 +409,11 @@ describe('Integration | flush', () => {
},
]);

logger.disableTraceInternals();
logger.setConfig({ traceInternals: false });
});

it('logs warning if adding event that is after maxReplayDuration', async () => {
logger.enableTraceInternals();
logger.setConfig({ traceInternals: true });

const spyLogger = vi.spyOn(SentryUtils.logger, 'info');

Expand Down Expand Up @@ -448,7 +447,7 @@ describe('Integration | flush', () => {
} because it is after maxReplayDuration`,
);

logger.disableTraceInternals();
logger.setConfig({ traceInternals: false });
spyLogger.mockRestore();
});

Expand Down
Loading