Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
> make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
<!-- prettier-ignore-end -->

## Unreleased

### Breaking Changes

- `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))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added it to "Breaking Changes" because it's a breaking change :) But not sure it should be done this way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is right, a breaking change like this requires a major version, whenever the next major is planned, incl. the change there

## 8.10.0

### Features
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/js/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ export class ReactNativeClient extends Client<ReactNativeClientOptions> {
options.parentSpanIsAlwaysRootSpan =
options.parentSpanIsAlwaysRootSpan === undefined ? true : options.parentSpanIsAlwaysRootSpan;

// Logs are opt-out now: calling `Sentry.logger.*` is the user's opt-in.
options.enableLogs = options.enableLogs ?? options._experiments?.enableLogs ?? true;
Comment thread
cursor[bot] marked this conversation as resolved.

// enableLogs must be disabled before calling super() to avoid logs being captured.
// This makes a copy of the user defined value, so we can restore it later for the native usaege.
const originalEnableLogs = options.enableLogs;
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/js/integrations/default.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* oxlint-disable eslint(complexity) */
import type { Integration } from '@sentry/core';

import { browserSessionIntegration, consoleLoggingIntegration } from '@sentry/browser';
import { browserSessionIntegration } from '@sentry/browser';

import type { ReactNativeClientOptions } from '../options';

Expand Down Expand Up @@ -92,7 +92,6 @@ export function getDefaultIntegrations(options: ReactNativeClientOptions): Integ
integrations.push(modulesLoaderIntegration());
if (options.enableLogs && options.logsOrigin !== 'native') {
integrations.push(logEnricherIntegration());
integrations.push(consoleLoggingIntegration());
}
if (options.attachScreenshot) {
integrations.push(screenshotIntegration());
Expand Down
7 changes: 7 additions & 0 deletions packages/core/src/js/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,13 @@ export interface BaseReactNativeOptions {
*/
propagateTraceparent?: boolean;

/**
* Acts as the kill switch for Sentry logs. When set to `false`, no logs are sent to Sentry.
*
* @default true
*/
enableLogs?: boolean;

/**
* Controls which log origin is captured when `enableLogs` is set to true.
* 'all' will log all origins.
Expand Down
26 changes: 26 additions & 0 deletions packages/core/test/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,32 @@ describe('Tests ReactNativeClient', () => {
expect(flushLogsSpy).toHaveBeenCalledTimes(1);
expect(flushLogsSpy).toHaveBeenLastCalledWith(client);
});

test('defaults enableLogs to true when not provided', () => {
jest.useFakeTimers();
const flushLogsSpy = jest.spyOn(SentryCore, '_INTERNAL_flushLogsBuffer').mockImplementation(jest.fn());

const { client } = createClientWithSpy({});

expect(client.getOptions().enableLogs).toBe(true);

client.emit('afterCaptureLog', { message: 'test', attributes: {} } as unknown);
jest.advanceTimersByTime(5000);

expect(flushLogsSpy).toHaveBeenCalledTimes(1);
});

test('respects user-provided enableLogs: false over the default', () => {
const { client } = createClientWithSpy({ enableLogs: false });
expect(client.getOptions().enableLogs).toBe(false);
});

test('backfills enableLogs from _experiments.enableLogs when top-level is undefined', () => {
const { client } = createClientWithSpy({
_experiments: { enableLogs: false },
} as Partial<ReactNativeClientOptions>);
expect(client.getOptions().enableLogs).toBe(false);
});
});
});

Expand Down
16 changes: 11 additions & 5 deletions packages/core/test/integrations/defaultLogs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,26 @@ describe('getDefaultIntegrations - logging integrations', () => {
return integrations.map((integration: Integration) => integration.name);
};

it('does not add logging integrations when enableLogs is falsy', () => {
it('does not add logEnricher when enableLogs is false', () => {
const names = getIntegrationNames(createOptions({ enableLogs: false }));

expect(names).not.toContain(logEnricherIntegrationName);
expect(names).not.toContain(consoleLoggingIntegrationName);
});

it('adds logging integrations when enableLogs is true and logsOrigin is not native', () => {
it('adds logEnricher when enableLogs is true and logsOrigin is not native', () => {
const names = getIntegrationNames(createOptions({ enableLogs: true }));

expect(names).toContain(logEnricherIntegrationName);
expect(names).toContain(consoleLoggingIntegrationName);
});

it('does not add logging integrations when logsOrigin is native', () => {
it('never adds consoleLoggingIntegration by default — it must be opt-in', () => {
const names = getIntegrationNames(createOptions({ enableLogs: true }));

expect(names).not.toContain(consoleLoggingIntegrationName);
});

it('does not add logEnricher when logsOrigin is native', () => {
const names = getIntegrationNames(
createOptions({
enableLogs: true,
Expand All @@ -76,6 +81,7 @@ describe('getDefaultIntegrations - logging integrations', () => {
);

expect(names.includes(logEnricherIntegrationName)).toBe(shouldInclude);
expect(names.includes(consoleLoggingIntegrationName)).toBe(shouldInclude);
// consoleLoggingIntegration is always opt-in regardless of logsOrigin
expect(names).not.toContain(consoleLoggingIntegrationName);
});
});
Loading