-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(browser): Add option to explicitly end pageload span via reportPageLoaded()
#17697
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 4 commits
5e5b6bc
9ac071e
64ef836
c1762f1
145086e
bd61a09
f69665e
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 |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
window._testBaseTimestamp = performance.timeOrigin / 1000; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
integrations: [Sentry.browserTracingIntegration({ explicitPageloadEnd: true })], | ||
tracesSampleRate: 1, | ||
debug: true, | ||
}); | ||
|
||
setTimeout(() => { | ||
Sentry.reportPageLoaded(); | ||
}, 2500); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { expect } from '@playwright/test'; | ||
import { | ||
SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, | ||
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, | ||
} from '@sentry/browser'; | ||
import { sentryTest } from '../../../../../utils/fixtures'; | ||
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; | ||
|
||
sentryTest( | ||
'waits for Sentry.reportPageLoaded() to be called when `explicitPageloadEnd` is true', | ||
async ({ getLocalTestUrl, page }) => { | ||
if (shouldSkipTracingTest()) { | ||
sentryTest.skip(); | ||
} | ||
|
||
const pageloadEventPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); | ||
|
||
const url = await getLocalTestUrl({ testDir: __dirname }); | ||
|
||
await page.goto(url); | ||
|
||
const eventData = envelopeRequestParser(await pageloadEventPromise); | ||
|
||
const traceContextData = eventData.contexts?.trace?.data; | ||
const spanDurationSeconds = eventData.timestamp! - eventData.start_timestamp!; | ||
|
||
expect(traceContextData).toMatchObject({ | ||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', | ||
['sentry.idle_span_finish_reason']: 'reportPageLoaded', | ||
}); | ||
|
||
// We wait for 2.5 seconds before calling Sentry.reportPageLoaded() | ||
// the margins are to account for timing weirdness in CI to avoid flakes | ||
expect(spanDurationSeconds).toBeGreaterThan(2); | ||
expect(spanDurationSeconds).toBeLessThan(3); | ||
}, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
window._testBaseTimestamp = performance.timeOrigin / 1000; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
integrations: [Sentry.browserTracingIntegration({ explicitPageloadEnd: true, finalTimeout: 3000 })], | ||
tracesSampleRate: 1, | ||
debug: true, | ||
}); | ||
|
||
// not calling Sentry.reportPageLoaded() on purpose! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { expect } from '@playwright/test'; | ||
import { | ||
SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, | ||
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, | ||
} from '@sentry/browser'; | ||
import { sentryTest } from '../../../../../utils/fixtures'; | ||
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; | ||
|
||
sentryTest( | ||
'final timeout cancels the pageload span even if `explicitPageloadEnd` is true', | ||
async ({ getLocalTestUrl, page }) => { | ||
if (shouldSkipTracingTest()) { | ||
sentryTest.skip(); | ||
} | ||
|
||
const pageloadEventPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); | ||
|
||
const url = await getLocalTestUrl({ testDir: __dirname }); | ||
|
||
await page.goto(url); | ||
|
||
const eventData = envelopeRequestParser(await pageloadEventPromise); | ||
|
||
const traceContextData = eventData.contexts?.trace?.data; | ||
const spanDurationSeconds = eventData.timestamp! - eventData.start_timestamp!; | ||
|
||
expect(traceContextData).toMatchObject({ | ||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', | ||
['sentry.idle_span_finish_reason']: 'finalTimeout', | ||
}); | ||
|
||
// We wait for 3 seconds before calling Sentry.reportPageLoaded() | ||
// the margins are to account for timing weirdness in CI to avoid flakes | ||
expect(spanDurationSeconds).toBeGreaterThan(2.5); | ||
expect(spanDurationSeconds).toBeLessThan(3.5); | ||
}, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
window._testBaseTimestamp = performance.timeOrigin / 1000; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
integrations: [Sentry.browserTracingIntegration({ explicitPageloadEnd: true, instrumentNavigation: false })], | ||
tracesSampleRate: 1, | ||
debug: true, | ||
}); | ||
|
||
setTimeout(() => { | ||
Sentry.startBrowserTracingNavigationSpan(Sentry.getClient(), { name: 'custom_navigation' }); | ||
}, 1000); | ||
|
||
setTimeout(() => { | ||
Sentry.reportPageLoaded(); | ||
}, 2500); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import { expect } from '@playwright/test'; | ||
import { | ||
SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, | ||
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, | ||
} from '@sentry/browser'; | ||
import { sentryTest } from '../../../../../utils/fixtures'; | ||
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers'; | ||
|
||
sentryTest( | ||
'starting a navigation span cancels the pageload span even if `explicitPageloadEnd` is true', | ||
async ({ getLocalTestUrl, page }) => { | ||
if (shouldSkipTracingTest()) { | ||
sentryTest.skip(); | ||
} | ||
|
||
const pageloadEventPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload'); | ||
|
||
const url = await getLocalTestUrl({ testDir: __dirname }); | ||
|
||
await page.goto(url); | ||
|
||
const eventData = envelopeRequestParser(await pageloadEventPromise); | ||
|
||
const traceContextData = eventData.contexts?.trace?.data; | ||
const spanDurationSeconds = eventData.timestamp! - eventData.start_timestamp!; | ||
|
||
expect(traceContextData).toMatchObject({ | ||
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1, | ||
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', | ||
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', | ||
['sentry.idle_span_finish_reason']: 'cancelled', | ||
}); | ||
|
||
// ending span after 1s but adding a margin of 0.5s to account for timing weirdness in CI to avoid flakes | ||
expect(spanDurationSeconds).toBeLessThan(1.5); | ||
}, | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -606,6 +606,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> { | |
) => void, | ||
): () => void; | ||
|
||
/** | ||
* A hook for the browser tracing integrations to trigger the end of a page load span. | ||
* @returns {() => void} A function that, when executed, removes the registered callback. | ||
*/ | ||
public on(hook: 'endPageloadSpan', callback: () => void): () => void; | ||
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. I decided to go with the client hooks approach instead of exposing a method on 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. IMHO this is a nice enough API for this anyhow, I like it! |
||
|
||
/** | ||
* A hook for the browser tracing integrations to trigger after the pageload span was started. | ||
* @returns {() => void} A function that, when executed, removes the registered callback. | ||
|
@@ -800,6 +806,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> { | |
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined }, | ||
): void; | ||
|
||
/** | ||
* Emit a hook event for browser tracing integrations to trigger the end of a page load span. | ||
*/ | ||
public emit(hook: 'endPageloadSpan'): void; | ||
|
||
/** | ||
* Emit a hook event for browser tracing integrations to trigger aafter the pageload span was started. | ||
*/ | ||
|
Uh oh!
There was an error while loading. Please reload this page.