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

feat(sdk): Automatically detect environment if not set by User #3362

Merged
merged 5 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- Automatically detect environment if not set ([#3362](https://github.com/getsentry/sentry-react-native/pull/3362))

### Fixes

- Add actual `activeThreadId` to Profiles ([#3338](https://github.com/getsentry/sentry-react-native/pull/3338))
Expand Down
3 changes: 2 additions & 1 deletion src/js/profiling/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Envelope, Event, Profile } from '@sentry/types';
import { forEachEnvelopeItem, logger } from '@sentry/utils';

import { getDefaultEnvironment } from '../utils/environment';
import type { RawThreadCpuProfile } from './types';

/**
Expand Down Expand Up @@ -64,7 +65,7 @@ export function createProfilingEvent(profile: RawThreadCpuProfile, event: Event)

return createProfilePayload(profile, {
release: event.release || '',
environment: event.environment || '',
environment: event.environment || getDefaultEnvironment(),
event_id: event.event_id || '',
transaction: event.transaction || '',
start_timestamp: event.start_timestamp ? event.start_timestamp * 1000 : Date.now(),
Expand Down
5 changes: 5 additions & 0 deletions src/js/sdk.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable complexity */
import type { Scope } from '@sentry/core';
import { getIntegrationsToSetup, hasTracingEnabled, Hub, initAndBind, makeMain, setExtra } from '@sentry/core';
import { HttpClient } from '@sentry/integrations';
Expand Down Expand Up @@ -33,6 +34,7 @@ import { TouchEventBoundary } from './touchevents';
import { ReactNativeProfiler, ReactNativeTracing } from './tracing';
import { DEFAULT_BUFFER_SIZE, makeNativeTransportFactory } from './transports/native';
import { makeUtf8TextEncoder } from './transports/TextEncoder';
import { getDefaultEnvironment } from './utils/environment';
import { safeFactory, safeTracesSampler } from './utils/safe';
import { NATIVE } from './wrapper';

Expand Down Expand Up @@ -95,6 +97,9 @@ export function init(passedOptions: ReactNativeOptions): void {
initialScope: safeFactory(passedOptions.initialScope, { loggerMessage: 'The initialScope threw an error' }),
tracesSampler: safeTracesSampler(passedOptions.tracesSampler),
};
if (!('environment' in options)) {
options.environment = getDefaultEnvironment();
}

const defaultIntegrations: Integration[] = passedOptions.defaultIntegrations || [];
if (passedOptions.defaultIntegrations === undefined) {
Expand Down
5 changes: 5 additions & 0 deletions src/js/utils/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,8 @@ export function getHermesVersion(): string | undefined {
RN_GLOBAL_OBJ.HermesInternal.getRuntimeProperties()['OSS Release Version']
);
}

/** Returns default environment based on __DEV__ */
export function getDefaultEnvironment(): 'development' | 'production' {
return __DEV__ ? 'development' : 'production';
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
}
114 changes: 108 additions & 6 deletions test/profiling/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import type { Envelope, Event, Profile, Transaction, Transport } from '@sentry/t

import * as Sentry from '../../src/js';
import { HermesProfiling } from '../../src/js/integrations';
import type { NativeDeviceContextsResponse } from '../../src/js/NativeRNSentry';
import type * as Hermes from '../../src/js/profiling/hermes';
import { isHermesEnabled } from '../../src/js/utils/environment';
import { getDefaultEnvironment, isHermesEnabled } from '../../src/js/utils/environment';
import { RN_GLOBAL_OBJ } from '../../src/js/utils/worldwide';
import { MOCK_DSN } from '../mockDsn';
import { envelopeItemPayload, envelopeItems } from '../testutils';
Expand All @@ -35,7 +36,7 @@ describe('profiling integration', () => {
});

test('should start profile if there is a transaction running when integration is created', () => {
mock = initTestClient(false);
mock = initTestClient({ withProfiling: false });
jest.runAllTimers();
jest.clearAllMocks();

Expand Down Expand Up @@ -65,6 +66,96 @@ describe('profiling integration', () => {
]);
});

describe('environment', () => {
beforeEach(() => {
(getDefaultEnvironment as jest.Mock).mockReturnValue('mocked');
mockWrapper.NATIVE.fetchNativeDeviceContexts.mockResolvedValue(<NativeDeviceContextsResponse>{
environment: 'native',
});
});

const expectTransactionWithEnvironment = (envelope: Envelope | undefined, env: string | undefined) => {
const transactionEvent = envelope?.[envelopeItems][0][envelopeItemPayload] as Event;
expect(transactionEvent).toEqual(
expect.objectContaining<Partial<Event>>({
environment: env,
}),
);
};

const expectProfileWithEnvironment = (envelope: Envelope | undefined, env: string | undefined) => {
const profileEvent = (envelope?.[envelopeItems][1] as [{ type: 'profile' }, Profile])[1];
expect(profileEvent).toEqual(
expect.objectContaining<Partial<Profile>>({
environment: env,
}),
);
};

test('should use default environment for transaction and profile', () => {
mock = initTestClient();

const transaction: Transaction = Sentry.startTransaction({
name: 'test-name',
});
transaction.finish();

jest.runAllTimers();

const envelope: Envelope | undefined = mock.transportSendMock.mock.lastCall?.[0];
expectTransactionWithEnvironment(envelope, 'mocked');
expectProfileWithEnvironment(envelope, 'mocked');
});

test('should use native environment for transaction and profile if user value is nullish', () => {
mock = initTestClient({ withProfiling: true, environment: '' });

const transaction: Transaction = Sentry.startTransaction({
name: 'test-name',
});
transaction.finish();

jest.runAllTimers();

const envelope: Envelope | undefined = mock.transportSendMock.mock.lastCall?.[0];
expectTransactionWithEnvironment(envelope, 'native');
expectProfileWithEnvironment(envelope, 'native');
});

test('should keep nullish for transaction and profile uses default', () => {
mockWrapper.NATIVE.fetchNativeDeviceContexts.mockResolvedValue(<NativeDeviceContextsResponse>{
environment: undefined,
});
mock = initTestClient({ withProfiling: true, environment: undefined });

const transaction: Transaction = Sentry.startTransaction({
name: 'test-name',
});
transaction.finish();

jest.runAllTimers();

const envelope: Envelope | undefined = mock.transportSendMock.mock.lastCall?.[0];
expectTransactionWithEnvironment(envelope, undefined);
expectProfileWithEnvironment(envelope, 'mocked');
});

test('should keep custom environment for transaction and profile', () => {
mock = initTestClient({ withProfiling: true, environment: 'custom' });

const transaction: Transaction = Sentry.startTransaction({
name: 'test-name',
});
transaction.finish();

jest.runAllTimers();

const envelope: Envelope | undefined = mock.transportSendMock.mock.lastCall?.[0];
expectTransactionWithEnvironment(envelope, 'custom');
expectProfileWithEnvironment(envelope, 'custom');
});
});

describe('with profiling enabled', () => {
beforeEach(() => {
mock = initTestClient();
Expand Down Expand Up @@ -231,17 +322,24 @@ function getCurrentHermesProfilingIntegration(): TestHermesIntegration {
return integration as unknown as TestHermesIntegration;
}

function initTestClient(withProfiling: boolean = true): {
function initTestClient(
testOptions: {
withProfiling?: boolean;
environment?: string;
} = {
withProfiling: true,
},
): {
transportSendMock: jest.Mock<ReturnType<Transport['send']>, Parameters<Transport['send']>>;
} {
const transportSendMock = jest.fn<ReturnType<Transport['send']>, Parameters<Transport['send']>>();
Sentry.init({
const options: Sentry.ReactNativeOptions = {
dsn: MOCK_DSN,
_experiments: {
profilesSampleRate: 1,
},
integrations: integrations => {
if (!withProfiling) {
if (!testOptions.withProfiling) {
return integrations.filter(i => i.name !== 'HermesProfiling');
}
return integrations;
Expand All @@ -250,7 +348,11 @@ function initTestClient(withProfiling: boolean = true): {
send: transportSendMock.mockResolvedValue(undefined),
flush: jest.fn().mockResolvedValue(true),
}),
});
};
if ('environment' in testOptions) {
options.environment = testOptions.environment;
}
Sentry.init(options);

// In production integrations are setup only once, but in the tests we want them to setup on every init
const integrations = Sentry.getCurrentHub().getClient()?.getOptions().integrations;
Expand Down
30 changes: 30 additions & 0 deletions test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,36 @@ describe('Tests the SDK functionality', () => {
});
});

describe('environment', () => {
it('detect development environment', () => {
init({
enableNative: true,
});
expect(usedOptions()?.environment).toBe('development');
});

it('uses custom environment', () => {
init({
environment: 'custom',
});
expect(usedOptions()?.environment).toBe('custom');
});

it('it keeps empty string environment', () => {
init({
environment: '',
});
expect(usedOptions()?.environment).toBe('');
});

it('it keeps undefined environment', () => {
init({
environment: undefined,
});
expect(usedOptions()?.environment).toBe(undefined);
});
});

describe('transport options buffer size', () => {
it('uses default transport options buffer size', () => {
init({
Expand Down