Blocked for now: Enable logging by default#6084
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 82 more 🤖 This preview updates automatically when you update the PR. |
|
|
||
| - `enableLogs` defaults to `true` ([#6084](https://github.com/getsentry/sentry-react-native/pull/6084)) | ||
| - `consoleLoggingIntegration` is no longer added by default. To forward `console.*` calls to Sentry logs, add it explicitly: `integrations: [Sentry.consoleLoggingIntegration()]` ([#6084](https://github.com/getsentry/sentry-react-native/pull/6084)) | ||
|
|
There was a problem hiding this comment.
I added it to "Breaking Changes" because it's a breaking change :) But not sure it should be done this way.
There was a problem hiding this comment.
The implementation changes LGTM @alwx 👍
Looping in @christophaigner and @dingsdax for more context on this since normally a breaking change would require a major version bump.
There was a problem hiding this comment.
this is right, a breaking change like this requires a major version, whenever the next major is planned, incl. the change there
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5f5e5d2. Configure here.
| options.environment = getDefaultEnvironment(); | ||
| } | ||
|
|
||
| options.enableLogs = options.enableLogs ?? options._experiments?.enableLogs ?? true; |
There was a problem hiding this comment.
Duplicated defaulting logic for enableLogs across two files
Low Severity
The identical defaulting expression options.enableLogs = options.enableLogs ?? options._experiments?.enableLogs ?? true appears in both sdk.tsx and client.ts. Unlike other option defaults (e.g. parentSpanIsAlwaysRootSpan, which is only set in the client constructor), enableLogs is set in two places because sdk.tsx needs it before getDefaultIntegrations() runs. This duplication risks the two locations diverging if the fallback chain or default is updated in only one place. Flagging per review rules on redundant logic.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 5f5e5d2. Configure here.
alwx
changed the title
3 participants
This PR is finished but blocked because we decided to only include it to the next major.
📢 Type of change
📜 Description
Enables logging by default, fixes #6083
💡 Motivation and Context
See #6083
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps