search

getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.95k stars 1.57k forks source link

Auto instrument `JSON.parse` #11795

Open timfish opened 6 months ago

timfish commented 6 months ago

Problem Statement

While instrumenting one of my projects I wrapped JSON.parse and JSON.stringify so they would be automatically included in traces so I could see their impact in requests. Then today I saw @anonrig post this.

Since these are synchronous I guess they have quite an impact if they're blocking for a long time.

Solution Brainstorm

Wrap parse and maybe even stringify with proxies that automatically add spans.

AbhiPrasad commented 6 months ago

So we attempted this on the frontend but it got really noisy so I ended up reverting it. I wonder if there's a good way to reduce the noise 🤔

Maybe require min duration?

https://github.com/getsentry/sentry/pull/63503

timfish commented 6 months ago

Noisy, as in just too many json spans?

We could make an integration with a configurable threshold and then only enable it be default for Node? Or enabled by default for all SDKs and vary the threshold by SDK?

AbhiPrasad commented 6 months ago

Noisy, as in just too many json spans?

Yes too many spans - which is a problem of itself with our app, but something we ran into.

I'd start with making it opt-in everywhere, and then rolling it out in Node first. configurable threshold makes sense to me!