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

Laravel note on performance overhead potentially misleading #11337

Open
kosty-sentry-dw-eval opened this issue Sep 11, 2024 · 3 comments · May be fixed by #11533
Open

Laravel note on performance overhead potentially misleading #11337

kosty-sentry-dw-eval opened this issue Sep 11, 2024 · 3 comments · May be fixed by #11533

Comments

@kosty-sentry-dw-eval
Copy link

kosty-sentry-dw-eval commented Sep 11, 2024

SDK

PHP SDK

Description

https://docs.sentry.io/platforms/php/guides/laravel/#local-development-and-testing
Mentions negative effect of Sentry SDK on performance. I believe it may be a bit misleading

  1. This may lead customers to generalize that to Sentry overall, however this is something specific to only PHP platform that doesn't allow sending events in the background. Overhead on all other platforms is negligible as background threads (or equivalent) are used.
  2. AFAIK even for PHP (in production environment) it can be effectively mitigated by running a local relay in the same datacenter that reduces latency. We only mention that in the deep fine print of [Performance Monitoring] Performance Overhead and only in the context of AWS Lambda (I believe it should apply universally). So customer may conclude overhead locally => overhead in production and not be aware of mitigation. And this Laravel passage in question doesn't link to that doc.

Context: this section seems to be picked up a lot by RAG as it matches certain use cases well and other platforms don't have an equivalent doc. It's on RAG that it wasn't able to tell it's SDK-specific, but maybe we can meet it halfway? 🙂

Suggested Solution

add something along the lines of "because PHP unlike other platforms doesn't have background threads"? (@cleptric correct me if this is wrong please)

@getsantry
Copy link
Contributor

getsantry bot commented Sep 11, 2024

Assigning to @getsentry/support for routing ⏲️

@cleptric
Copy link
Member

We got PR open at #11533. I just want to make sure that we fully understand how our SDKs work. Just because an SDK uses an async transport, it does not mean that the overhead magically disappears.

@realkosty
Copy link
Contributor

I just want to make sure that we fully understand how our SDKs work. Just because an SDK uses an async transport, it does not mean that the overhead magically disappears.

yes, also 'overhead' is not very precise, the answer is different depending on what you mean by it :)

You most likely don't want errors to be sent to Sentry when you are developing or running tests

curious why is SDK overhead a bigger problem when developing or testing rather than production? I understand you want quick local iterations but still there's something off in that logic...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

6 participants