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

fix(mongo): rewrite Buffer as ? during serialization #14071

Merged
merged 7 commits into from
Nov 12, 2024
Merged

Conversation

bcoe
Copy link
Member

@bcoe bcoe commented Oct 23, 2024

Buffer, which Mongoose uses some instances rather than a string serializes as {"0": "?", "1": "?", "2": "?"}.

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

Copy link
Contributor

github-actions bot commented Oct 23, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 22.77 KB - -
@sentry/browser - with treeshaking flags 21.53 KB - -
@sentry/browser (incl. Tracing) 35.26 KB - -
@sentry/browser (incl. Tracing, Replay) 71.98 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.35 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 76.31 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 89.14 KB - -
@sentry/browser (incl. Feedback) 39.93 KB - -
@sentry/browser (incl. sendFeedback) 27.42 KB - -
@sentry/browser (incl. FeedbackAsync) 32.23 KB - -
@sentry/react 25.52 KB - -
@sentry/react (incl. Tracing) 38.22 KB - -
@sentry/vue 26.91 KB - -
@sentry/vue (incl. Tracing) 37.1 KB - -
@sentry/svelte 22.9 KB - -
CDN Bundle 24.13 KB - -
CDN Bundle (incl. Tracing) 37.04 KB - -
CDN Bundle (incl. Tracing, Replay) 71.7 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 77.05 KB - -
CDN Bundle - uncompressed 70.73 KB - -
CDN Bundle (incl. Tracing) - uncompressed 109.9 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 222.42 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 235.64 KB - -
@sentry/nextjs (client) 38.33 KB - -
@sentry/sveltekit (client) 35.84 KB - -
@sentry/node 133.5 KB +0.11% +137 B 🔺
@sentry/node - without tracing 95.67 KB - -
@sentry/aws-serverless 105.93 KB - -

View base workflow run

Comment on lines +55 to +59
let isBuffer = false;
if (typeof Buffer !== 'undefined') {
isBuffer = Buffer.isBuffer(value);
}
return isBuffer;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is necessary, all our node versions should support this so let's just check normally here :)

@mjq
Copy link
Member

mjq commented Nov 12, 2024

Have we considered trying to upstream this into OTel's mongo instrumentation? Seems like a nicer default than what they've got, and it helps share the maintenance of the code. (Doesn't have to block this PR though, we can always take it back out if/when it gets upstreamed).

@bcoe
Copy link
Member Author

bcoe commented Nov 12, 2024

@mjq yes, I will upstream and keep an eye on when we can roll this code back.

@bcoe bcoe merged commit 6d1a251 into develop Nov 12, 2024
130 checks passed
@bcoe bcoe deleted the handle-buffer branch November 12, 2024 19:36
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.

3 participants