ref(core): Add weight tracking logic to browser logs/metrics #17901
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
chargome
approved these changes
Oct 10, 2025
Lms24
reviewed
Oct 10, 2025
packages/core/src/client.ts
Outdated
| ): void { | ||
| // @ts-expect-error - TypeScript can't narrow generic hook types to match specific overloads, but we know this is type-safe | ||
| client.on(flushHook, () => { | ||
| client[weightProperty] = 0; |
There was a problem hiding this comment.
m/h: does still work in our CDN bundles where we mangle private _-prefixed names?
l: Not particularly a fan of extracting this to a function and then accessing private members. Is there a reason for this over adding a private method to the Client class?
There was a problem hiding this comment.
alternatively, could we keep the weight property (+whatever else) as variables within the closure of setupWeightBasedFlushing?
There was a problem hiding this comment.
yeah using closure variables is way nicer. Lemme refactor.
andreiborza
pushed a commit
to thedanchez/sentry-javascript
that referenced
this pull request
Oct 15, 2025
…ry#17901) We've seen some cases where our browser logs are hitting size limits. I suspect this is because we don't have any robust size tracking mechanisms in the browser sdk. <img width="1251" height="513" alt="image" src="https://github.com/user-attachments/assets/2364b984-2b53-4c6a-89e5-0a0e20fa3246" /> This refactors our log flushing mechanisms in the SDK to unify everything between the browser client and server runtime client. This also means the browser SDK gets a weight tracking mechanism for buffering, which should help with making sure we don't run into size issues with logs. Given metrics has the same issue, I included it in this refactor.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We've seen some cases where our browser logs are hitting size limits. I suspect this is because we don't have any robust size tracking mechanisms in the browser sdk.
This refactors our log flushing mechanisms in the SDK to unify everything between the browser client and server runtime client. This also means the browser SDK gets a weight tracking mechanism for buffering, which should help with making sure we don't run into size issues with logs.
Given metrics has the same issue, I included it in this refactor.