Skip to content

Commit 75fc4d2

Browse files
Lms24Lms24
committed
pass in client to avoid setTimeout
1 parent abb8660 commit 75fc4d2

File tree

7 files changed

+60
-56
lines changed

7 files changed

+60
-56
lines changed

‎packages/browser-utils/src/metrics/browserMetrics.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable max-lines */
2-
import type { Measurements, Span, SpanAttributes, SpanAttributeValue, StartSpanOptions } from '@sentry/core';
2+
import type { Client, Measurements, Span, SpanAttributes, SpanAttributeValue, StartSpanOptions } from '@sentry/core';
33
import {
44
browserPerformanceTimeOrigin,
55
getActiveSpan,
@@ -83,6 +83,7 @@ let _clsEntry: LayoutShift | undefined;
8383
interface StartTrackingWebVitalsOptions {
8484
recordClsStandaloneSpans: boolean;
8585
recordLcpStandaloneSpans: boolean;
86+
client: Client;
8687
}
8788

8889
/**
@@ -94,6 +95,7 @@ interface StartTrackingWebVitalsOptions {
9495
export function startTrackingWebVitals({
9596
recordClsStandaloneSpans,
9697
recordLcpStandaloneSpans,
98+
client,
9799
}: StartTrackingWebVitalsOptions): () => void {
98100
const performance = getBrowserPerformanceAPI();
99101
if (performance && browserPerformanceTimeOrigin()) {
@@ -102,9 +104,9 @@ export function startTrackingWebVitals({
102104
WINDOW.performance.mark('sentry-tracing-init');
103105
}
104106
const fidCleanupCallback = _trackFID();
105-
const lcpCleanupCallback = recordLcpStandaloneSpans ? trackLcpAsStandaloneSpan() : _trackLCP();
107+
const lcpCleanupCallback = recordLcpStandaloneSpans ? trackLcpAsStandaloneSpan(client) : _trackLCP();
106108
const ttfbCleanupCallback = _trackTtfb();
107-
const clsCleanupCallback = recordClsStandaloneSpans ? trackClsAsStandaloneSpan() : _trackCLS();
109+
const clsCleanupCallback = recordClsStandaloneSpans ? trackClsAsStandaloneSpan(client) : _trackCLS();
108110

109111
return (): void => {
110112
fidCleanupCallback();

‎packages/browser-utils/src/metrics/cls.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { SpanAttributes } from '@sentry/core';
1+
import type { Client, SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
44
getCurrentScope,
@@ -24,7 +24,7 @@ import { listenForWebVitalReportEvents, msToSec, startStandaloneWebVitalSpan, su
2424
* Once either of these events triggers, the CLS value is sent as a standalone span and we stop
2525
* measuring CLS.
2626
*/
27-
export function trackClsAsStandaloneSpan(): void {
27+
export function trackClsAsStandaloneSpan(client: Client): void {
2828
let standaloneCLsValue = 0;
2929
let standaloneClsEntry: LayoutShift | undefined;
3030

@@ -41,7 +41,7 @@ export function trackClsAsStandaloneSpan(): void {
4141
standaloneClsEntry = entry;
4242
}, true);
4343

44-
listenForWebVitalReportEvents((reportEvent, pageloadSpanId) => {
44+
listenForWebVitalReportEvents(client, (reportEvent, pageloadSpanId) => {
4545
sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId, reportEvent);
4646
cleanupClsHandler();
4747
});
@@ -79,6 +79,7 @@ function sendStandaloneClsSpan(
7979
}
8080

8181
const span = startStandaloneWebVitalSpan({
82+
type: 'cls',
8283
name,
8384
transaction: routeName,
8485
attributes,

‎packages/browser-utils/src/metrics/inp.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ export const _onInp: InstrumentationHandlerCallback = ({ metric }) => {
129129
};
130130

131131
const span = startStandaloneWebVitalSpan({
132+
type: 'inp',
132133
name,
133134
transaction: routeName,
134135
attributes,

‎packages/browser-utils/src/metrics/lcp.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { SpanAttributes } from '@sentry/core';
1+
import type { Client, SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
44
getCurrentScope,
@@ -24,7 +24,7 @@ import { listenForWebVitalReportEvents, msToSec, startStandaloneWebVitalSpan, su
2424
* Once either of these events triggers, the LCP value is sent as a standalone span and we stop
2525
* measuring LCP for subsequent routes.
2626
*/
27-
export function trackLcpAsStandaloneSpan(): void {
27+
export function trackLcpAsStandaloneSpan(client: Client): void {
2828
let standaloneLcpValue = 0;
2929
let standaloneLcpEntry: LargestContentfulPaint | undefined;
3030

@@ -41,7 +41,7 @@ export function trackLcpAsStandaloneSpan(): void {
4141
standaloneLcpEntry = entry;
4242
}, true);
4343

44-
listenForWebVitalReportEvents((reportEvent, pageloadSpanId) => {
44+
listenForWebVitalReportEvents(client, (reportEvent, pageloadSpanId) => {
4545
_sendStandaloneLcpSpan(standaloneLcpValue, standaloneLcpEntry, pageloadSpanId, reportEvent);
4646
cleanupLcpHandler();
4747
});
@@ -92,6 +92,7 @@ export function _sendStandaloneLcpSpan(
9292
}
9393

9494
const span = startStandaloneWebVitalSpan({
95+
type: 'lcp',
9596
name,
9697
transaction: routeName,
9798
attributes,

‎packages/browser-utils/src/metrics/utils.ts

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
import type { Integration, SentrySpan, Span, SpanAttributes, SpanTimeInput, StartSpanOptions } from '@sentry/core';
2-
import {
3-
getActiveSpan,
4-
getClient,
5-
getCurrentScope,
6-
getRootSpan,
7-
spanToJSON,
8-
startInactiveSpan,
9-
withActiveSpan,
1+
import type {
2+
Client,
3+
Integration,
4+
SentrySpan,
5+
Span,
6+
SpanAttributes,
7+
SpanTimeInput,
8+
StartSpanOptions,
109
} from '@sentry/core';
10+
import { debug, getClient, getCurrentScope, spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core';
11+
import { DEBUG_BUILD } from '../debug-build';
1112
import { WINDOW } from '../types';
1213
import { onHidden } from './web-vitals/lib/onHidden';
1314

@@ -55,6 +56,7 @@ export function startAndEndSpan(
5556
}
5657

5758
interface StandaloneWebVitalSpanOptions {
59+
type: 'lcp' | 'cls' | 'inp';
5860
name: string;
5961
transaction?: string;
6062
attributes: SpanAttributes;
@@ -83,7 +85,7 @@ export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptio
8385
return;
8486
}
8587

86-
const { name, transaction, attributes: passedAttributes, startTime } = options;
88+
const { name, transaction, attributes: passedAttributes, startTime, type } = options;
8789

8890
const { release, environment, sendDefaultPii } = client.getOptions();
8991
// We need to get the replay, user, and activeTransaction from the current scope
@@ -125,6 +127,9 @@ export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptio
125127
...passedAttributes,
126128
};
127129

130+
DEBUG_BUILD &&
131+
debug.log('Starting standalone web vital span', { type, name, transaction, startTime }, 'attributes:', attributes);
132+
128133
return startInactiveSpan({
129134
name,
130135
attributes,
@@ -205,6 +210,7 @@ export function supportsWebVital(entryType: 'layout-shift' | 'largest-contentful
205210
* - pageloadSpanId: the span id of the pageload span. This is used to link the web vital span to the pageload span.
206211
*/
207212
export function listenForWebVitalReportEvents(
213+
client: Client,
208214
collectorCallback: (event: WebVitalReportEvent, pageloadSpanId: string) => void,
209215
) {
210216
let pageloadSpanId: string | undefined;
@@ -218,28 +224,20 @@ export function listenForWebVitalReportEvents(
218224
}
219225

220226
onHidden(() => {
221-
if (!collected) {
222-
_runCollectorCallbackOnce('pagehide');
223-
}
227+
_runCollectorCallbackOnce('pagehide');
224228
});
225229

226-
setTimeout(() => {
227-
const client = getClient();
228-
if (!client) {
229-
return;
230+
const unsubscribeStartNavigation = client.on('beforeStartNavigationSpan', (_, options) => {
231+
// we only want to collect LCP if we actually navigate. Redirects should be ignored.
232+
if (!options?.isRedirect) {
233+
_runCollectorCallbackOnce('navigation');
234+
unsubscribeStartNavigation?.();
235+
unsubscribeAfterStartPageLoadSpan?.();
230236
}
237+
});
231238

232-
const unsubscribeStartNavigation = client.on('beforeStartNavigationSpan', (_, options) => {
233-
// we only want to collect LCP if we actually navigate. Redirects should be ignored.
234-
if (!options?.isRedirect) {
235-
_runCollectorCallbackOnce('navigation');
236-
unsubscribeStartNavigation?.();
237-
}
238-
});
239-
240-
const unsubscribeAfterStartPageLoadSpan = client.on('afterStartPageLoadSpan', span => {
241-
pageloadSpanId = span.spanContext().spanId;
242-
unsubscribeAfterStartPageLoadSpan?.();
243-
});
244-
}, 0);
239+
const unsubscribeAfterStartPageLoadSpan = client.on('afterStartPageLoadSpan', span => {
240+
pageloadSpanId = span.spanContext().spanId;
241+
unsubscribeAfterStartPageLoadSpan?.();
242+
});
245243
}

‎packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
440440
_collectWebVitals = startTrackingWebVitals({
441441
recordClsStandaloneSpans: enableStandaloneClsSpans || false,
442442
recordLcpStandaloneSpans: enableStandaloneLcpSpans || false,
443+
client,
443444
});
444445

445446
if (enableInp) {

‎packages/core/src/client.ts

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { dsnToString, makeDsn } from './utils/dsn';
3737
import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils/envelope';
3838
import { getPossibleEventMessages } from './utils/eventUtils';
3939
import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils/is';
40-
import { debug } from './utils/logger';
40+
import { logger } from './utils/logger';
4141
import { merge } from './utils/merge';
4242
import { checkOrSetAlreadyCaught, uuid4 } from './utils/misc';
4343
import { parseSampleRate } from './utils/parseSampleRate';
@@ -154,7 +154,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
154154
if (options.dsn) {
155155
this._dsn = makeDsn(options.dsn);
156156
} else {
157-
DEBUG_BUILD && debug.warn('No DSN provided, client will not send events.');
157+
DEBUG_BUILD && logger.warn('No DSN provided, client will not send events.');
158158
}
159159

160160
if (this._dsn) {
@@ -182,7 +182,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
182182

183183
// ensure we haven't captured this very object before
184184
if (checkOrSetAlreadyCaught(exception)) {
185-
DEBUG_BUILD && debug.log(ALREADY_SEEN_ERROR);
185+
DEBUG_BUILD && logger.log(ALREADY_SEEN_ERROR);
186186
return eventId;
187187
}
188188

@@ -237,7 +237,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
237237

238238
// ensure we haven't captured this very object before
239239
if (hint?.originalException && checkOrSetAlreadyCaught(hint.originalException)) {
240-
DEBUG_BUILD && debug.log(ALREADY_SEEN_ERROR);
240+
DEBUG_BUILD && logger.log(ALREADY_SEEN_ERROR);
241241
return eventId;
242242
}
243243

@@ -429,15 +429,15 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
429429
if ('aggregates' in session) {
430430
const sessionAttrs = session.attrs || {};
431431
if (!sessionAttrs.release && !clientReleaseOption) {
432-
DEBUG_BUILD && debug.warn(MISSING_RELEASE_FOR_SESSION_ERROR);
432+
DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR);
433433
return;
434434
}
435435
sessionAttrs.release = sessionAttrs.release || clientReleaseOption;
436436
sessionAttrs.environment = sessionAttrs.environment || clientEnvironmentOption;
437437
session.attrs = sessionAttrs;
438438
} else {
439439
if (!session.release && !clientReleaseOption) {
440-
DEBUG_BUILD && debug.warn(MISSING_RELEASE_FOR_SESSION_ERROR);
440+
DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR);
441441
return;
442442
}
443443
session.release = session.release || clientReleaseOption;
@@ -465,7 +465,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
465465
// would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
466466
// With typescript 4.1 we could even use template literal types
467467
const key = `${reason}:${category}`;
468-
DEBUG_BUILD && debug.log(`Recording outcome: "${key}"${count > 1 ? ` (${count} times)` : ''}`);
468+
DEBUG_BUILD && logger.log(`Recording outcome: "${key}"${count > 1 ? ` (${count} times)` : ''}`);
469469
this._outcomes[key] = (this._outcomes[key] || 0) + count;
470470
}
471471
}
@@ -877,12 +877,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
877877

878878
if (this._isEnabled() && this._transport) {
879879
return this._transport.send(envelope).then(null, reason => {
880-
DEBUG_BUILD && debug.error('Error while sending envelope:', reason);
880+
DEBUG_BUILD && logger.error('Error while sending envelope:', reason);
881881
return reason;
882882
});
883883
}
884884

885-
DEBUG_BUILD && debug.error('Transport disabled');
885+
DEBUG_BUILD && logger.error('Transport disabled');
886886

887887
return resolvedSyncPromise({});
888888
}
@@ -1032,7 +1032,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
10321032
isolationScope = getIsolationScope(),
10331033
): PromiseLike<string | undefined> {
10341034
if (DEBUG_BUILD && isErrorEvent(event)) {
1035-
debug.log(`Captured error event \`${getPossibleEventMessages(event)[0] || '<unknown>'}\``);
1035+
logger.log(`Captured error event \`${getPossibleEventMessages(event)[0] || '<unknown>'}\``);
10361036
}
10371037

10381038
return this._processEvent(event, hint, currentScope, isolationScope).then(
@@ -1042,11 +1042,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
10421042
reason => {
10431043
if (DEBUG_BUILD) {
10441044
if (_isDoNotSendEventError(reason)) {
1045-
debug.log(reason.message);
1045+
logger.log(reason.message);
10461046
} else if (_isInternalError(reason)) {
1047-
debug.warn(reason.message);
1047+
logger.warn(reason.message);
10481048
} else {
1049-
debug.warn(reason);
1049+
logger.warn(reason);
10501050
}
10511051
}
10521052
return undefined;
@@ -1207,22 +1207,22 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
12071207
* Sends client reports as an envelope.
12081208
*/
12091209
protected _flushOutcomes(): void {
1210-
DEBUG_BUILD && debug.log('Flushing outcomes...');
1210+
DEBUG_BUILD && logger.log('Flushing outcomes...');
12111211

12121212
const outcomes = this._clearOutcomes();
12131213

12141214
if (outcomes.length === 0) {
1215-
DEBUG_BUILD && debug.log('No outcomes to send');
1215+
DEBUG_BUILD && logger.log('No outcomes to send');
12161216
return;
12171217
}
12181218

12191219
// This is really the only place where we want to check for a DSN and only send outcomes then
12201220
if (!this._dsn) {
1221-
DEBUG_BUILD && debug.log('No dsn provided, will not send outcomes');
1221+
DEBUG_BUILD && logger.log('No dsn provided, will not send outcomes');
12221222
return;
12231223
}
12241224

1225-
DEBUG_BUILD && debug.log('Sending outcomes:', outcomes);
1225+
DEBUG_BUILD && logger.log('Sending outcomes:', outcomes);
12261226

12271227
const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn));
12281228

0 commit comments

Comments
 (0)