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 5 commits
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
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { Event, EventHint } from '@sentry/types';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import type { ReplayContainer } from '../types';
import { isErrorEvent, isFeedbackEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils';
import { isRrwebError } from '../util/isRrwebError';
import { logger } from '../util/logger';
import { addFeedbackBreadcrumb } from './util/addFeedbackBreadcrumb';
import { shouldSampleForBufferEvent } from './util/shouldSampleForBufferEvent';

Expand Down Expand Up @@ -50,7 +50,7 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even
// Unless `captureExceptions` is enabled, we want to ignore errors coming from rrweb
// As there can be a bunch of stuff going wrong in internals there, that we don't want to bubble up to users
if (isRrwebError(event, hint) && !replay.getOptions()._experiments.captureExceptions) {
DEBUG_BUILD && logger.log('[Replay] Ignoring error from rrweb internals', event);
DEBUG_BUILD && logger.log('Ignoring error from rrweb internals', event);
return null;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { getClient } from '@sentry/core';
import type { Breadcrumb, BreadcrumbHint, FetchBreadcrumbData, XhrBreadcrumbData } from '@sentry/types';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import type { FetchHint, ReplayContainer, ReplayNetworkOptions, XhrHint } from '../types';
import { logger } from '../util/logger';
import { captureFetchBreadcrumbToReplay, enrichFetchBreadcrumb } from './util/fetchUtils';
import { captureXhrBreadcrumbToReplay, enrichXhrBreadcrumb } from './util/xhrUtils';

Expand Down Expand Up @@ -80,6 +80,7 @@ export function beforeAddNetworkBreadcrumb(
}
} catch (e) {
DEBUG_BUILD && logger.warn('Error when enriching network breadcrumb');
DEBUG_BUILD && logger.exception(e);
}
}

Expand Down
14 changes: 9 additions & 5 deletions packages/replay-internal/src/coreHandlers/util/fetchUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { setTimeout } from '@sentry-internal/browser-utils';
import type { Breadcrumb, FetchBreadcrumbData } from '@sentry/types';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../../debug-build';
import type {
Expand All @@ -11,6 +10,7 @@ import type {
ReplayNetworkRequestData,
ReplayNetworkRequestOrResponse,
} from '../../types';
import { logger } from '../../util/logger';
import { addNetworkBreadcrumb } from './addNetworkBreadcrumb';
import {
buildNetworkRequestOrResponse,
Expand Down Expand Up @@ -42,7 +42,8 @@ export async function captureFetchBreadcrumbToReplay(
const result = makeNetworkReplayBreadcrumb('resource.fetch', data);
addNetworkBreadcrumb(options.replay, result);
} catch (error) {
DEBUG_BUILD && logger.error('[Replay] Failed to capture fetch breadcrumb', error);
DEBUG_BUILD && logger.error('Failed to capture fetch breadcrumb');
DEBUG_BUILD && logger.exception(error);
}
}

Expand Down Expand Up @@ -192,7 +193,8 @@ function getResponseData(

return buildNetworkRequestOrResponse(headers, size, undefined);
} catch (error) {
DEBUG_BUILD && logger.warn('[Replay] Failed to serialize response body', error);
DEBUG_BUILD && logger.warn('Failed to serialize response body');
Copy link
Member

Choose a reason for hiding this comment

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

I think the one improvement we can do here more is to avoid having to double-call this in so many places. Instead, what about we make this something like this:

DEBUG_BUILD && logger.exception(error, 'Failed to serialize response body');

where the second argument is optional, and if defined will just logger.error this string? This may safe a few more bytes, and IMHO is also nicer from a usage experience...?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated, though it didn't seem to help bundle size too much

DEBUG_BUILD && logger.exception(error);
// fallback
return buildNetworkRequestOrResponse(headers, responseBodySize, undefined);
}
Expand All @@ -209,7 +211,8 @@ async function _parseFetchResponseBody(response: Response): Promise<[string | un
const text = await _tryGetResponseText(res);
return [text];
} catch (error) {
DEBUG_BUILD && logger.warn('[Replay] Failed to get text body from response', error);
DEBUG_BUILD && logger.warn('Failed to get text body from response');
DEBUG_BUILD && logger.exception(error);
return [undefined, 'BODY_PARSE_ERROR'];
}
}
Expand Down Expand Up @@ -279,7 +282,8 @@ function _tryCloneResponse(response: Response): Response | void {
return response.clone();
} catch (error) {
// this can throw if the response was already consumed before
DEBUG_BUILD && logger.warn('[Replay] Failed to clone response body', error);
DEBUG_BUILD && logger.warn('Failed to clone response body');
DEBUG_BUILD && logger.exception(error);
}
}

Expand Down
10 changes: 6 additions & 4 deletions packages/replay-internal/src/coreHandlers/util/networkUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dropUndefinedKeys, logger, stringMatchesSomePattern } from '@sentry/utils';
import { dropUndefinedKeys, stringMatchesSomePattern } from '@sentry/utils';

import { NETWORK_BODY_MAX_SIZE, WINDOW } from '../../constants';
import { DEBUG_BUILD } from '../../debug-build';
Expand All @@ -10,6 +10,7 @@ import type {
ReplayNetworkRequestOrResponse,
ReplayPerformanceEntry,
} from '../../types';
import { logger } from '../../util/logger';

/** Get the size of a body. */
export function getBodySize(body: RequestInit['body']): number | undefined {
Expand Down Expand Up @@ -77,12 +78,13 @@ export function getBodyString(body: unknown): [string | undefined, NetworkMetaWa
if (!body) {
return [undefined];
}
} catch {
DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body);
} catch (error) {
DEBUG_BUILD && logger.warn('Failed to serialize body', body);
DEBUG_BUILD && logger.exception(error);
return [undefined, 'BODY_PARSE_ERROR'];
}

DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body);
DEBUG_BUILD && logger.info('Skipping network body because of body type', body);

return [undefined, 'UNPARSEABLE_BODY_TYPE'];
}
Expand Down
14 changes: 8 additions & 6 deletions packages/replay-internal/src/coreHandlers/util/xhrUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils';
import type { Breadcrumb, XhrBreadcrumbData } from '@sentry/types';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../../debug-build';
import type {
Expand All @@ -10,6 +9,7 @@ import type {
ReplayNetworkRequestData,
XhrHint,
} from '../../types';
import { logger } from '../../util/logger';
import { addNetworkBreadcrumb } from './addNetworkBreadcrumb';
import {
buildNetworkRequestOrResponse,
Expand Down Expand Up @@ -39,7 +39,8 @@ export async function captureXhrBreadcrumbToReplay(
const result = makeNetworkReplayBreadcrumb('resource.xhr', data);
addNetworkBreadcrumb(options.replay, result);
} catch (error) {
DEBUG_BUILD && logger.error('[Replay] Failed to capture xhr breadcrumb', error);
DEBUG_BUILD && logger.warn('Failed to capture xhr breadcrumb');
DEBUG_BUILD && logger.exception(error);
}
}

Expand Down Expand Up @@ -161,7 +162,7 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM
errors.push(e);
}

DEBUG_BUILD && logger.warn('[Replay] Failed to get xhr response body', ...errors);
DEBUG_BUILD && logger.warn('Failed to get xhr response body', ...errors);

return [undefined];
}
Expand Down Expand Up @@ -197,12 +198,13 @@ export function _parseXhrResponse(
if (!body) {
return [undefined];
}
} catch {
DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body);
} catch (error) {
DEBUG_BUILD && logger.warn('Failed to serialize body', body);
DEBUG_BUILD && logger.exception(error);
return [undefined, 'BODY_PARSE_ERROR'];
}

DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body);
DEBUG_BUILD && logger.info('Skipping network body because of body type', body);

return [undefined, 'UNPARSEABLE_BODY_TYPE'];
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { ReplayRecordingData } from '@sentry/types';

import { logger } from '@sentry/utils';
import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants';
import { DEBUG_BUILD } from '../debug-build';
import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types';
import { logger } from '../util/logger';
import { timestampToMs } from '../util/timestamp';
import { WorkerHandler } from './WorkerHandler';
import { EventBufferSizeExceededError } from './error';
Expand Down Expand Up @@ -88,7 +88,8 @@ export class EventBufferCompressionWorker implements EventBuffer {

// We do not wait on this, as we assume the order of messages is consistent for the worker
this._worker.postMessage('clear').then(null, e => {
DEBUG_BUILD && logger.warn('[Replay] Sending "clear" message to worker failed', e);
DEBUG_BUILD && logger.warn('Sending "clear" message to worker failed', e);
DEBUG_BUILD && logger.exception(e);
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import type { ReplayRecordingData } from '@sentry/types';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types';
import { logInfo } from '../util/log';
import { logger } from '../util/logger';
import { EventBufferArray } from './EventBufferArray';
import { EventBufferCompressionWorker } from './EventBufferCompressionWorker';

Expand Down Expand Up @@ -90,7 +89,8 @@ export class EventBufferProxy implements EventBuffer {
} catch (error) {
// If the worker fails to load, we fall back to the simple buffer.
// Nothing more to do from our side here
logInfo('[Replay] Failed to load the compression worker, falling back to simple buffer');
DEBUG_BUILD && logger.info('Failed to load the compression worker, falling back to simple buffer');
DEBUG_BUILD && logger.exception(error);
return;
}

Expand All @@ -117,7 +117,8 @@ export class EventBufferProxy implements EventBuffer {
try {
await Promise.all(addEventPromises);
} catch (error) {
DEBUG_BUILD && logger.warn('[Replay] Failed to add events when switching buffers.', error);
DEBUG_BUILD && logger.error('Failed to add events when switching buffers.');
DEBUG_BUILD && logger.exception(error);
}
}
}
8 changes: 3 additions & 5 deletions packages/replay-internal/src/eventBuffer/WorkerHandler.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import type { WorkerRequest, WorkerResponse } from '../types';
import { logInfo } from '../util/log';
import { logger } from '../util/logger';

/**
* Event buffer that uses a web worker to compress events.
Expand Down Expand Up @@ -57,7 +55,7 @@ export class WorkerHandler {
* Destroy the worker.
*/
public destroy(): void {
logInfo('[Replay] Destroying compression worker');
DEBUG_BUILD && logger.info('Destroying compression worker');
this._worker.terminate();
}

Expand Down Expand Up @@ -85,7 +83,7 @@ export class WorkerHandler {

if (!response.success) {
// TODO: Do some error handling, not sure what
DEBUG_BUILD && logger.error('[Replay]', response.response);
DEBUG_BUILD && logger.error('Error in compression worker: ', response.response);

reject(new Error('Error in compression worker'));
return;
Expand Down
10 changes: 6 additions & 4 deletions packages/replay-internal/src/eventBuffer/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { getWorkerURL } from '@sentry-internal/replay-worker';

import { DEBUG_BUILD } from '../debug-build';
import type { EventBuffer } from '../types';
import { logInfo } from '../util/log';
import { logger } from '../util/logger';
import { EventBufferArray } from './EventBufferArray';
import { EventBufferProxy } from './EventBufferProxy';

Expand Down Expand Up @@ -32,7 +33,7 @@ export function createEventBuffer({
}
}

logInfo('[Replay] Using simple buffer');
DEBUG_BUILD && logger.info('Using simple buffer');
return new EventBufferArray();
}

Expand All @@ -44,11 +45,12 @@ function _loadWorker(customWorkerUrl?: string): EventBufferProxy | void {
return;
}

logInfo(`[Replay] Using compression worker${customWorkerUrl ? ` from ${customWorkerUrl}` : ''}`);
DEBUG_BUILD && logger.info(`Using compression worker${customWorkerUrl ? ` from ${customWorkerUrl}` : ''}`);
const worker = new Worker(workerUrl);
return new EventBufferProxy(worker);
} catch (error) {
logInfo('[Replay] Failed to create compression worker');
DEBUG_BUILD && logger.warn('Failed to create compression worker');
DEBUG_BUILD && logger.exception(error);
// Fall back to use simple event buffer array
}
}
Expand Down
Loading
Loading