Skip to content

Conversation

@alwx
Copy link
Contributor

@alwx alwx commented Nov 24, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Fixes #5384

This PR does the following:

  • Adds Metrics API to React Native SDK;
  • Adds a few tests for Metrics (to make sure Sentry.metrics, enableMetrics flag and beforeSendMetric callback work;
  • Adds metrics to the sample apps.

💚 How did you test it?

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

alwx added 4 commits November 25, 2025 10:38
# Conflicts:
#	samples/expo/package.json
� Conflicts:
�	CHANGELOG.md
@alwx alwx force-pushed the alwx/feature/metrics branch from 5ce970d to d327971 Compare November 25, 2025 09:40
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

Fails
🚫 Pull request is not ready for merge, please add the "ready-to-merge" label to the pull request

Generated by 🚫 dangerJS against 0a27b75

@alwx alwx changed the title WIP: Metrics for React Native SDK Metrics for React Native SDK Nov 25, 2025
Sentry.logger.error('expo error log');

Sentry.logger.info('expo info log with data', { database: 'admin', number: 123, obj: { password: 'admin'} });
Sentry.logger.info('expo info log with data', { database: 'admin', number: 123, obj: { password: 'admin' } });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated auto-formatting change

@alwx alwx marked this pull request as ready for review November 25, 2025 10:16
@antonis
Copy link
Contributor

antonis commented Nov 25, 2025

@sentry review

Comment on lines +1 to +180
import { getClient, metrics, setCurrentClient } from '@sentry/core';
import { ReactNativeClient } from '../src/js';
import { getDefaultTestClientOptions } from './mocks/client';
import { NATIVE } from './mockWrapper';

jest.mock('../src/js/wrapper', () => jest.requireActual('./mockWrapper'));

const EXAMPLE_DSN = 'https://6890c2f6677340daa4804f8194804ea2@o19635.ingest.sentry.io/148053';

describe('Metrics', () => {
beforeEach(() => {
jest.clearAllMocks();
jest.useFakeTimers();
(NATIVE.isNativeAvailable as jest.Mock).mockImplementation(() => true);
});

afterEach(() => {
const client = getClient();
client?.close();
jest.clearAllTimers();
jest.useRealTimers();
});

describe('beforeSendMetric', () => {
it('is called when enableMetrics is true and a metric is sent', async () => {
const beforeSendMetric = jest.fn(metric => metric);

const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
enableMetrics: true,
beforeSendMetric,
}),
});

setCurrentClient(client);
client.init();

// Send a metric
metrics.count('test_metric', 1);

jest.advanceTimersByTime(10000);
expect(beforeSendMetric).toHaveBeenCalled();
});

it('is not called when enableMetrics is false', async () => {
const beforeSendMetric = jest.fn(metric => metric);

const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
enableMetrics: false,
beforeSendMetric,
}),
});

setCurrentClient(client);
client.init();

// Send a metric
metrics.count('test_metric', 1);

jest.advanceTimersByTime(10000);
expect(beforeSendMetric).not.toHaveBeenCalled();
});

it('is called when enableMetrics is undefined (metrics are enabled by default)', async () => {
const beforeSendMetric = jest.fn(metric => metric);

const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
beforeSendMetric,
}),
});

setCurrentClient(client);
client.init();

// Send a metric
metrics.count('test_metric', 1);

jest.advanceTimersByTime(10000);
expect(beforeSendMetric).toHaveBeenCalled();
});

it('allows beforeSendMetric to modify metrics when enableMetrics is true', async () => {
const beforeSendMetric = jest.fn(metric => {
// Modify the metric
return { ...metric, name: 'modified_metric' };
});

const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
enableMetrics: true,
beforeSendMetric,
}),
});

setCurrentClient(client);
client.init();

// Send a metric
metrics.count('test_metric', 1);

jest.advanceTimersByTime(10000);
expect(beforeSendMetric).toHaveBeenCalled();
const modifiedMetric = beforeSendMetric.mock.results[0]?.value;
expect(modifiedMetric).toBeDefined();
expect(modifiedMetric.name).toBe('modified_metric');
});

it('allows beforeSendMetric to drop metrics by returning null', async () => {
const beforeSendMetric = jest.fn(() => null);

const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
enableMetrics: true,
beforeSendMetric,
}),
});

setCurrentClient(client);
client.init();

// Send a metric
metrics.count('test_metric', 1);

// Advance timers
jest.advanceTimersByTime(10000);
expect(beforeSendMetric).toHaveBeenCalled();
expect(beforeSendMetric.mock.results[0]?.value).toBeNull();
});
});

describe('metrics API', () => {
it('metrics.count works when enableMetrics is true', () => {
const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
enableMetrics: true,
}),
});

setCurrentClient(client);
client.init();

expect(() => {
metrics.count('test_metric', 1);
}).not.toThrow();
});

it('metrics can be sent with tags', async () => {
const beforeSendMetric = jest.fn(metric => metric);

const client = new ReactNativeClient({
...getDefaultTestClientOptions({
dsn: EXAMPLE_DSN,
enableMetrics: true,
beforeSendMetric,
}),
});

setCurrentClient(client);
client.init();

// Send a metric with tags
metrics.count('test_metric', 1, {
attributes: { environment: 'test' },
});

jest.advanceTimersByTime(10000);
expect(beforeSendMetric).toHaveBeenCalled();
const sentMetric = beforeSendMetric.mock.calls[0]?.[0];
expect(sentMetric).toBeDefined();
});
});
});
Copy link

Choose a reason for hiding this comment

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

The test suite advances timers by 10 seconds (line 40, 62, etc.) expecting metrics to be sent, but there is no verification that the actual metric data reaches the transport layer or that the metrics aggregation completes. Consider adding assertions on the transport mock or verifying that NATIVE.sendEnvelope was called with the expected metric envelope. Additionally, ensure that ReactNativeClient properly inherits metrics support from @sentry/core Client.
Severity: MEDIUM

🤖 Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/core/test/metrics.test.ts#L1-L180

Potential issue: The test suite advances timers by 10 seconds (line 40, 62, etc.)
expecting metrics to be sent, but there is no verification that the actual metric data
reaches the transport layer or that the metrics aggregation completes. Consider adding
assertions on the transport mock or verifying that `NATIVE.sendEnvelope` was called with
the expected metric envelope. Additionally, ensure that `ReactNativeClient` properly
inherits metrics support from `@sentry/core` Client.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3443368

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point actually

beforeSendMetric(metric: Sentry.Metric) {
logWithoutTracing('Metric beforeSend:', metric.name, metric.value);
return metric;
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Wdyt of enabling metrics for the sample app and adding an example in the app (e.g. a button or screen that uses metrics)

Suggested change
},
},
enableMetrics: true,

Copy link
Collaborator

@lucas-zimerman lucas-zimerman Nov 25, 2025

Choose a reason for hiding this comment

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

I am surprised it is not on the experimental field
EDIT: it no longer is experimental


### Features

- Adds metrics ([#5402](https://github.com/getsentry/sentry-react-native/pull/5402))
Copy link
Contributor

Choose a reason for hiding this comment

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

We could mention that the feature is beta (similar to feedback and SR in the past)

Suggested change
- Adds metrics ([#5402](https://github.com/getsentry/sentry-react-native/pull/5402))
- Adds Metrics Beta ([#5402](https://github.com/getsentry/sentry-react-native/pull/5402))

Copy link
Collaborator

Choose a reason for hiding this comment

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

also a small snippet on how to use it would be nice here

Copy link
Collaborator

Choose a reason for hiding this comment

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

@antonis do we want to keep it as beta since it no longer is in beta for JavaScript?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would advocate towards shipping as beta since the product is still labeled as beta in JS sidebar

Choose a reason for hiding this comment

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

You can always differentiate between the state of the product, and the state of the SDK. The product is currently in Open Beta.

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Awesome work @alwx 🎸
Left a few comments/suggestions but overall looks great!

@lucas-zimerman
Copy link
Collaborator

lucas-zimerman commented Nov 25, 2025

Sentry docs mention this for JavaScript
image
Shouldn't we limit it to the experimental field?

EDIT: getsentry/sentry-docs#15632 Lets keep it as is

@antonis
Copy link
Contributor

antonis commented Nov 25, 2025

Shouldn't we limit it to the experimental field?

This looks like an oversight in the docs since we don't mention the experimental elsewhere https://docs.sentry.io/platforms/javascript/metrics/ 🤔

@lucas-zimerman
Copy link
Collaborator

Thank you for the PR!
The PR looks good! before merging, we should enable the ready to ready-to-merge label, and add a basic description on how to enable it and generate a metric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentry Metrics for React Native

5 participants