-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
feat: Add Spotlight back through devservices #87295
Conversation
Adds and enables Spotlight by default which will now show up at the bottom right corner with all transactions etc. This also enables profiling as they would show up in Spotlight. I'll be following up enabling Spotlight for Snuba to get a more complete picture after this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we start with a partial rollout to ensure things go smoothly and we don't inadvertently interrupt development?
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #87295 +/- ##
=======================================
Coverage 87.73% 87.73%
=======================================
Files 9865 9865
Lines 559030 559035 +5
Branches 22041 22041
=======================================
+ Hits 490478 490489 +11
+ Misses 68121 68115 -6
Partials 431 431 |
@IanWoodard what's your main concern as this should not have any affect on development and can be turned off by setting |
@@ -15,6 +15,7 @@ class SdkConfig(TypedDict): | |||
send_default_pii: bool | |||
auto_enabling_integrations: bool | |||
keep_alive: NotRequired[bool] | |||
spotlight: NotRequired[str | bool | None] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I can remove None
from here now?
spotlight: NotRequired[str | bool | None] | |
spotlight: NotRequired[str | bool] |
It's a new piece of the system that can cause issues for developers if something breaks with spotlight internally. I hear you and am not saying this as a blocker, more so as a suggestion. |
Thanks for the review @IanWoodard. Everything around Spotlight is designed with graceful fallbacks and have been tested but I'll both send an announcement and keep an eye for reports. |
This is a follow up to getsentry/sentry#87295. Can be disabled by setting `SENTRY_SPOTLIGHT=0` env variable
This is a follow up to getsentry/sentry#87295. Can be disabled by setting `SENTRY_SPOTLIGHT=0` env variable
Adds and enables Spotlight by default which will now
show up at the bottom right corner with all transactions etc.
This also enables profiling as they would show up in Spotlight.
I'll be following up enabling Spotlight for Snuba to get a more
complete picture after this.