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

ref: Remove performance clear entry calls #2490

Merged
merged 4 commits into from
Mar 12, 2020
Merged

ref: Remove performance clear entry calls #2490

merged 4 commits into from
Mar 12, 2020

Conversation

HazAT
Copy link
Member

@HazAT HazAT commented Mar 12, 2020

User code or other libraries may be using the performance entries, therefore we do not clear entries.

Note: there is a limit on the number of "resource timing" entries, we do not change that, leaving it up to the browser default.

@HazAT HazAT requested a review from rhcarvalho March 12, 2020 07:49
@HazAT HazAT requested a review from kamilogorek as a code owner March 12, 2020 07:49
@HazAT HazAT self-assigned this Mar 12, 2020
@getsentry-bot
Copy link
Contributor

getsentry-bot commented Mar 12, 2020

Messages
📖

@sentry/browser bundle gzip'ed minified size: (ES5: 16.876 kB) (ES6: 15.9102 kB)

📖 ✅ TSLint passed

Generated by 🚫 dangerJS against 538b60c

Copy link
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

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

We cannot guarantee that 300 is actually an increase. Specially not without looking at the number of entries.

packages/apm/src/integrations/tracing.ts Outdated Show resolved Hide resolved
packages/apm/src/integrations/tracing.ts Outdated Show resolved Hide resolved
Comment on lines -606 to -613
// The Performance object has a limited buffer size, often 150 entries. At some point the buffer may overflow, in
// which case we would not be able to use it to create/update spans. Therefore, after we have processed entries to
// report to Sentry, we clear the buffer in an attempt to allow for more entries to be added in the future.
// https://developer.mozilla.org/en-US/docs/Web/API/Performance
logger.log('[Tracing] Clearing most performance marks');
performance.clearMarks();
performance.clearMeasures();
performance.clearResourceTimings();
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@HazAT HazAT changed the title ref: Use onresourcetimingbufferfull ref: Remove performance clear entry calls Mar 12, 2020
Copy link
Member

@dashed dashed left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@HazAT HazAT merged commit b6dd2c7 into master Mar 12, 2020
@HazAT HazAT deleted the apm/resource-buffer branch March 12, 2020 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants