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(integrations): Add Spotlight integration #3550

Merged
merged 8 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,21 @@

## Unreleased

### Features

- Add [`@spotlightjs/spotlight`](https://spotlightjs.com/) support ([#3550](https://github.com/getsentry/sentry-react-native/pull/3550))

Download the `Spotlight` desktop application and add the integration to your `Sentry.init`.

```javascript
import * as Sentry from '@sentry/react-native';

Sentry.init({
dsn: '___DSN___',
enableSpotlight: __DEV__,
});
```

### Fixes

- Prevent pod install crash when visionos is not present ([#3548](https://github.com/getsentry/sentry-react-native/pull/3548))
Expand Down
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module.exports = {
collectCoverage: true,
preset: 'react-native',
setupFilesAfterEnv: ['jest-extended/all', '<rootDir>/test/mockConsole.ts'],
setupFilesAfterEnv: ['jest-extended/all', '<rootDir>/test/mockConsole.ts', '<rootDir>/test/mockFetch.ts'],
globals: {
__DEV__: true,
'ts-jest': {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
"expo-module-scripts": "^3.1.0",
"jest": "^29.6.2",
"jest-environment-jsdom": "^29.6.2",
"jest-fetch-mock": "^3.0.3",
"jest-extended": "^4.0.2",
"madge": "^6.1.0",
"metro": "0.76",
Expand Down
1 change: 1 addition & 0 deletions samples/expo/app/(tabs)/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Sentry.init({
_experiments: {
profilesSampleRate: 0,
},
enableSpotlight: true,
});

export default function TabOneScreen() {
Expand Down
1 change: 1 addition & 0 deletions samples/react-native/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Sentry.init({
_experiments: {
profilesSampleRate: 0,
},
enableSpotlight: true,
});

const Stack = createStackNavigator();
Expand Down
9 changes: 9 additions & 0 deletions src/js/integrations/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { Release } from './release';
import { createReactNativeRewriteFrames } from './rewriteframes';
import { Screenshot } from './screenshot';
import { SdkInfo } from './sdkinfo';
import { Spotlight } from './spotlight';
import { ViewHierarchy } from './viewhierarchy';

/**
Expand Down Expand Up @@ -94,5 +95,13 @@ export function getDefaultIntegrations(options: ReactNativeClientOptions): Integ
integrations.push(new ExpoContext());
}

if (options.enableSpotlight) {
integrations.push(
Spotlight({
sidecarUrl: options.spotlightSidecarUrl,
}),
);
}

return integrations;
}
1 change: 1 addition & 0 deletions src/js/integrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ export { SdkInfo } from './sdkinfo';
export { ReactNativeInfo } from './reactnativeinfo';
export { ModulesLoader } from './modulesloader';
export { HermesProfiling } from '../profiling/integration';
export { Spotlight } from './spotlight';
98 changes: 98 additions & 0 deletions src/js/integrations/spotlight.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import type { Client, Envelope, EventProcessor, Integration } from '@sentry/types';
import { logger, serializeEnvelope } from '@sentry/utils';

import { makeUtf8TextEncoder } from '../transports/TextEncoder';
import { ReactNativeLibraries } from '../utils/rnlibraries';

type SpotlightReactNativeIntegrationOptions = {
/**
* The URL of the Sidecar instance to connect and forward events to.
* If not set, Spotlight will try to connect to the Sidecar running on localhost:8969.
*
* @default "http://localhost:8969/stream"
*/
sidecarUrl?: string;
};

/**
* Use this integration to send errors and transactions to Spotlight.
*
* Learn more about spotlight at https://spotlightjs.com
*/
export function Spotlight({
sidecarUrl = getDefaultSidecarUrl(),
}: SpotlightReactNativeIntegrationOptions = {}): Integration {
logger.info('[Spotlight] Using Sidecar URL', sidecarUrl);

return {
name: 'Spotlight',

setupOnce(_: (callback: EventProcessor) => void, getCurrentHub) {
const client = getCurrentHub().getClient();
if (client) {
setup(client, sidecarUrl);
} else {
logger.warn('[Spotlight] Could not initialize Sidecar integration due to missing Client');
}
},
};
}

function setup(client: Client, sidecarUrl: string): void {
sendEnvelopesToSidecar(client, sidecarUrl);
}

function sendEnvelopesToSidecar(client: Client, sidecarUrl: string): void {
if (!client.on) {
return;
}

client.on('beforeEnvelope', (originalEnvelope: Envelope) => {
// TODO: This is a workaround for spotlight/sidecar not supporting images
const spotlightEnvelope: Envelope = [...originalEnvelope];
const envelopeItems = [...originalEnvelope[1]].filter(
item => typeof item[0].content_type !== 'string' || !item[0].content_type.startsWith('image'),
);
Comment on lines +52 to +55
Copy link
Member

Choose a reason for hiding this comment

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

Hmm this is a good point. I think we just support text data at the moment.

I think we should handle this in the sidecar somehow because we don't want SDKs to have too much of that logic. Ideally, Spotlight (similarly to Relay lol) accepts all kinds of payloads and just deals with removing incompatible stuff on its end.

Additionally, WDYT, would it be valuable to render the images in the Spotlight UI?

Copy link
Member

Choose a reason for hiding this comment

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

Btw, no change required from my end. The workaround is fine for now, I just think we should handle this in the spotlight sidecar in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

I think rendering the screenshots would be especially useful for crashes/uncaught errors. So the developer doesn't have to try to crash the app again and carefully watch what was on the screen at that moment (or create a screenrecording).


spotlightEnvelope[1] = envelopeItems as Envelope[1];

fetch(sidecarUrl, {
method: 'POST',
body: serializeEnvelope(spotlightEnvelope, makeUtf8TextEncoder()),
headers: {
'Content-Type': 'application/x-sentry-envelope',
},
mode: 'cors',
}).catch(err => {
logger.error(
"[Spotlight] Sentry SDK can't connect to Spotlight is it running? See https://spotlightjs.com to download it.",
err,
);
});
});
}

function getDefaultSidecarUrl(): string {
try {
const { url } = ReactNativeLibraries.Devtools?.getDevServer();
return `http://${getHostnameFromString(url)}:8969/stream`;
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
} catch (_oO) {
// We can't load devserver URL
}
return 'http://localhost:8969/stream';
}

/**
* React Native implementation of the URL class is missing the `hostname` property.
*/
function getHostnameFromString(urlString: string): string | null {
const regex = /^(?:\w+:)?\/\/([^/:]+)(:\d+)?(.*)$/;
const matches = urlString.match(regex);

if (matches && matches[1]) {
return matches[1];
} else {
// Invalid URL format
return null;
}
}
21 changes: 21 additions & 0 deletions src/js/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,27 @@ export interface BaseReactNativeOptions {
* @default false
*/
enableCaptureFailedRequests?: boolean;

/**
* This option will enable forwarding captured Sentry events to Spotlight.
*
* More details: https://spotlightjs.com/
*
* IMPORTANT: Only set this option to `true` while developing, not in production!
*/
enableSpotlight?: boolean;

/**
* This option changes the default Spotlight Sidecar URL.
*
* By default, the SDK expects the Sidecar to be running
* on the same host as React Native Metro Dev Server.
*
* More details: https://spotlightjs.com/
*
* @default "http://localhost:8969/stream"
*/
spotlightSidecarUrl?: string;
}

export interface ReactNativeTransportOptions extends BrowserTransportOptions {
Expand Down
97 changes: 97 additions & 0 deletions test/integrations/spotlight.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import type { Envelope, Hub } from '@sentry/types';
import fetchMock from 'jest-fetch-mock';

import { Spotlight } from '../../src/js/integrations/spotlight';

describe('spotlight', () => {
it('should not change the original envelope', () => {
const mockHub = createMockHub();

const spotlight = Spotlight();
spotlight.setupOnce(
() => {},
() => mockHub as unknown as Hub,
);

const spotlightBeforeEnvelope = mockHub.getClient().on.mock.calls[0]?.[1] as
| ((envelope: Envelope) => void)
| undefined;

const originalEnvelopeReference = createMockEnvelope();
spotlightBeforeEnvelope?.(originalEnvelopeReference);

expect(spotlightBeforeEnvelope).toBeDefined();
expect(originalEnvelopeReference).toEqual(createMockEnvelope());
});

it('should remove image attachments from spotlight envelope', () => {
fetchMock.mockOnce();
const mockHub = createMockHub();

const spotlight = Spotlight();
spotlight.setupOnce(
() => {},
() => mockHub as unknown as Hub,
);

const spotlightBeforeEnvelope = mockHub.getClient().on.mock.calls[0]?.[1] as
| ((envelope: Envelope) => void)
| undefined;

spotlightBeforeEnvelope?.(createMockEnvelope());

expect(spotlightBeforeEnvelope).toBeDefined();
expect(fetchMock.mock.lastCall?.[1]?.body?.toString().includes('image/png')).toBe(false);
});
});

function createMockHub() {
const client = {
on: jest.fn(),
};

return {
getClient: jest.fn().mockReturnValue(client),
};
}

function createMockEnvelope(): Envelope {
return [
{
event_id: 'event_id',
sent_at: 'sent_at',
sdk: {
name: 'sdk_name',
version: 'sdk_version',
},
},
[
[
{
type: 'event',
length: 0,
},
{
event_id: 'event_id',
},
],
[
{
type: 'attachment',
length: 10,
filename: 'filename',
},
'attachment',
],
[
{
type: 'attachment',
length: 8,
filename: 'filename2',
content_type: 'image/png',
},
Uint8Array.from([137, 80, 78, 71, 13, 10, 26, 10]), // PNG header
],
],
];
}
2 changes: 2 additions & 0 deletions test/mockFetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import { enableFetchMocks } from 'jest-fetch-mock';
enableFetchMocks();
18 changes: 18 additions & 0 deletions test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,24 @@ describe('Tests the SDK functionality', () => {
);
});

it('no spotlight integration by default', () => {
init({});

const actualOptions = mockedInitAndBind.mock.calls[0][secondArg] as ReactNativeClientOptions;
const actualIntegrations = actualOptions.integrations;
expect(actualIntegrations).toEqual(expect.not.arrayContaining([expect.objectContaining({ name: 'Spotlight' })]));
});

it('adds spotlight integration', () => {
init({
enableSpotlight: true,
});

const actualOptions = mockedInitAndBind.mock.calls[0][secondArg] as ReactNativeClientOptions;
const actualIntegrations = actualOptions.integrations;
expect(actualIntegrations).toEqual(expect.arrayContaining([expect.objectContaining({ name: 'Spotlight' })]));
});

it('no default integrations', () => {
init({
defaultIntegrations: false,
Expand Down
15 changes: 14 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5908,7 +5908,7 @@ cosmiconfig@^5.0.5, cosmiconfig@^5.1.0:
js-yaml "^3.13.1"
parse-json "^4.0.0"

cross-fetch@^3.1.5:
cross-fetch@^3.0.4, cross-fetch@^3.1.5:
version "3.1.8"
resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-3.1.8.tgz#0327eba65fd68a7d119f8fb2bf9334a1a7956f82"
integrity sha512-cvA+JwZoU0Xq+h6WkMvAUqPEYy92Obet6UdKLfW60qn99ftItKjB5T+BkyWOFWe2pUyfQ+IJHmpOTznqk1M6Kg==
Expand Down Expand Up @@ -9020,6 +9020,14 @@ jest-extended@^4.0.2:
jest-diff "^29.0.0"
jest-get-type "^29.0.0"

jest-fetch-mock@^3.0.3:
version "3.0.3"
resolved "https://registry.yarnpkg.com/jest-fetch-mock/-/jest-fetch-mock-3.0.3.tgz#31749c456ae27b8919d69824f1c2bd85fe0a1f3b"
integrity sha512-Ux1nWprtLrdrH4XwE7O7InRY6psIi3GOsqNESJgMJ+M5cv4A8Lh7SN9d2V2kKRZ8ebAfcd1LNyZguAOb6JiDqw==
dependencies:
cross-fetch "^3.0.4"
promise-polyfill "^8.1.3"

jest-get-type@^29.0.0, jest-get-type@^29.6.3:
version "29.6.3"
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-29.6.3.tgz#36f499fdcea197c1045a127319c0481723908fd1"
Expand Down Expand Up @@ -11562,6 +11570,11 @@ promise-inflight@^1.0.1:
resolved "https://registry.yarnpkg.com/promise-inflight/-/promise-inflight-1.0.1.tgz#98472870bf228132fcbdd868129bad12c3c029e3"
integrity sha512-6zWPyEOFaQBJYcGMHBKTKJ3u6TBsnMFOIZSa6ce1e/ZrrsOlnHRHbabMjLiBYKp+n44X9eUI6VUPaukCXHuG4g==

promise-polyfill@^8.1.3:
version "8.3.0"
resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-8.3.0.tgz#9284810268138d103807b11f4e23d5e945a4db63"
integrity sha512-H5oELycFml5yto/atYqmjyigJoAo3+OXwolYiH7OfQuYlAqhxNvTfiNMbV9hsC6Yp83yE5r2KTVmtrG6R9i6Pg==

promise@^7.1.1:
version "7.3.1"
resolved "https://registry.yarnpkg.com/promise/-/promise-7.3.1.tgz#064b72602b18f90f29192b8b1bc418ffd1ebd3bf"
Expand Down
Loading