Skip to content

Commit d0accc0

Browse files
mydeamydea
authored
feat(core): Update Sentry.addBreadcrumb to skip hub (#10601)
Build on top of #10600, as that relies on this to ensure replay still works. This basically goes a step further than https://github.com/getsentry/sentry-javascript/pull/10586/files to actually change the implementation to skip the hub completely.
1 parent 93db2dc commit d0accc0

File tree

15 files changed

+163
-474
lines changed

15 files changed

+163
-474
lines changed

‎packages/core/src/breadcrumbs.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import type { Breadcrumb, BreadcrumbHint } from '@sentry/types';
2+
import { consoleSandbox, dateTimestampInSeconds } from '@sentry/utils';
3+
import { getClient } from './exports';
4+
import { getIsolationScope } from './hub';
5+
6+
/**
7+
* Default maximum number of breadcrumbs added to an event. Can be overwritten
8+
* with {@link Options.maxBreadcrumbs}.
9+
*/
10+
const DEFAULT_BREADCRUMBS = 100;
11+
12+
/**
13+
* Records a new breadcrumb which will be attached to future events.
14+
*
15+
* Breadcrumbs will be added to subsequent events to provide more context on
16+
* user's actions prior to an error or crash.
17+
*/
18+
export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): void {
19+
const client = getClient();
20+
const isolationScope = getIsolationScope();
21+
22+
if (!client) return;
23+
24+
const { beforeBreadcrumb = null, maxBreadcrumbs = DEFAULT_BREADCRUMBS } = client.getOptions();
25+
26+
if (maxBreadcrumbs <= 0) return;
27+
28+
const timestamp = dateTimestampInSeconds();
29+
const mergedBreadcrumb = { timestamp, ...breadcrumb };
30+
const finalBreadcrumb = beforeBreadcrumb
31+
? (consoleSandbox(() => beforeBreadcrumb(mergedBreadcrumb, hint)) as Breadcrumb | null)
32+
: mergedBreadcrumb;
33+
34+
if (finalBreadcrumb === null) return;
35+
36+
if (client.emit) {
37+
client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
38+
}
39+
40+
isolationScope.addBreadcrumb(finalBreadcrumb, maxBreadcrumbs);
41+
}

‎packages/core/src/exports.ts

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import type {
2-
Breadcrumb,
3-
BreadcrumbHint,
42
CaptureContext,
53
CheckIn,
64
Client,
@@ -77,19 +75,6 @@ export function captureEvent(event: Event, hint?: EventHint): string {
7775
return getCurrentHub().captureEvent(event, hint);
7876
}
7977

80-
/**
81-
* Records a new breadcrumb which will be attached to future events.
82-
*
83-
* Breadcrumbs will be added to subsequent events to provide more context on
84-
* user's actions prior to an error or crash.
85-
*
86-
* @param breadcrumb The breadcrumb to record.
87-
*/
88-
export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): ReturnType<Hub['addBreadcrumb']> {
89-
// eslint-disable-next-line deprecation/deprecation
90-
getCurrentHub().addBreadcrumb(breadcrumb, hint);
91-
}
92-
9378
/**
9479
* Sets context data with the given name.
9580
* @param name of the context

‎packages/core/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ export * from './tracing';
99
export * from './semanticAttributes';
1010
export { createEventEnvelope, createSessionEnvelope } from './envelope';
1111
export {
12-
addBreadcrumb,
1312
captureCheckIn,
1413
withMonitor,
1514
captureException,
@@ -95,6 +94,7 @@ export { RequestData } from './integrations/requestdata';
9594
export { InboundFilters } from './integrations/inboundfilters';
9695
export { FunctionToString } from './integrations/functiontostring';
9796
export { LinkedErrors } from './integrations/linkederrors';
97+
export { addBreadcrumb } from './breadcrumbs';
9898
/* eslint-enable deprecation/deprecation */
9999
import * as INTEGRATIONS from './integrations';
100100
export { functionToStringIntegration } from './integrations/functiontostring';

‎packages/core/test/lib/base.test.ts

Lines changed: 65 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
import type { Client, Envelope, Event, Span, Transaction } from '@sentry/types';
22
import { SentryError, SyncPromise, dsnToString, logger } from '@sentry/utils';
33

4-
import { Hub, Scope, makeSession, setGlobalScope } from '../../src';
4+
import {
5+
Scope,
6+
addBreadcrumb,
7+
getCurrentScope,
8+
getIsolationScope,
9+
makeSession,
10+
setCurrentClient,
11+
setGlobalScope,
12+
} from '../../src';
513
import * as integrationModule from '../../src/integration';
614
import { TestClient, getDefaultTestClientOptions } from '../mocks/client';
715
import { AdHocIntegration, TestIntegration } from '../mocks/integration';
@@ -55,6 +63,8 @@ describe('BaseClient', () => {
5563
TestClient.sendEventCalled = undefined;
5664
TestClient.instance = undefined;
5765
setGlobalScope(undefined);
66+
getCurrentScope().clear();
67+
getIsolationScope().clear();
5868
});
5969

6070
afterEach(() => {
@@ -114,133 +124,108 @@ describe('BaseClient', () => {
114124

115125
describe('getBreadcrumbs() / addBreadcrumb()', () => {
116126
test('adds a breadcrumb', () => {
117-
expect.assertions(1);
118-
119127
const options = getDefaultTestClientOptions({});
120128
const client = new TestClient(options);
129+
setCurrentClient(client);
130+
client.init();
131+
121132
const scope = new Scope();
122-
// eslint-disable-next-line deprecation/deprecation
123-
const hub = new Hub(client, scope);
124133

125134
scope.addBreadcrumb({ message: 'hello' }, 100);
126-
// eslint-disable-next-line deprecation/deprecation
127-
hub.addBreadcrumb({ message: 'world' });
135+
addBreadcrumb({ message: 'world' });
128136

129-
expect((scope as any)._breadcrumbs[1].message).toEqual('world');
130-
});
137+
const breadcrumbs = scope.getScopeData().breadcrumbs;
138+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
131139

132-
test('adds a timestamp to new breadcrumbs', () => {
133-
expect.assertions(1);
140+
expect(breadcrumbs).toEqual([{ message: 'hello', timestamp: expect.any(Number) }]);
141+
expect(isolationScopeBreadcrumbs).toEqual([{ message: 'world', timestamp: expect.any(Number) }]);
142+
});
134143

144+
test('accepts a timestamp for new breadcrumbs', () => {
135145
const options = getDefaultTestClientOptions({});
136146
const client = new TestClient(options);
147+
setCurrentClient(client);
148+
client.init();
149+
137150
const scope = new Scope();
138-
// eslint-disable-next-line deprecation/deprecation
139-
const hub = new Hub(client, scope);
140151

141-
scope.addBreadcrumb({ message: 'hello' }, 100);
142-
// eslint-disable-next-line deprecation/deprecation
143-
hub.addBreadcrumb({ message: 'world' });
152+
scope.addBreadcrumb({ message: 'hello', timestamp: 1234 }, 100);
153+
addBreadcrumb({ message: 'world', timestamp: 12345 });
144154

145-
expect((scope as any)._breadcrumbs[1].timestamp).toBeGreaterThan(1);
155+
const breadcrumbs = scope.getScopeData().breadcrumbs;
156+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
157+
158+
expect(breadcrumbs).toEqual([{ message: 'hello', timestamp: 1234 }]);
159+
expect(isolationScopeBreadcrumbs).toEqual([{ message: 'world', timestamp: 12345 }]);
146160
});
147161

148162
test('discards breadcrumbs beyond `maxBreadcrumbs`', () => {
149-
expect.assertions(2);
150-
151163
const options = getDefaultTestClientOptions({ maxBreadcrumbs: 1 });
152164
const client = new TestClient(options);
153-
const scope = new Scope();
154-
// eslint-disable-next-line deprecation/deprecation
155-
const hub = new Hub(client, scope);
156-
157-
scope.addBreadcrumb({ message: 'hello' }, 100);
158-
// eslint-disable-next-line deprecation/deprecation
159-
hub.addBreadcrumb({ message: 'world' });
160-
161-
expect((scope as any)._breadcrumbs.length).toEqual(1);
162-
expect((scope as any)._breadcrumbs[0].message).toEqual('world');
163-
});
164-
165-
test('allows concurrent updates', () => {
166-
expect.assertions(1);
165+
setCurrentClient(client);
166+
client.init();
167167

168-
const options = getDefaultTestClientOptions({});
169-
const client = new TestClient(options);
170-
const scope = new Scope();
171-
// eslint-disable-next-line deprecation/deprecation
172-
const hub = new Hub(client, scope);
168+
addBreadcrumb({ message: 'hello1' });
169+
addBreadcrumb({ message: 'hello2' });
170+
addBreadcrumb({ message: 'hello3' });
173171

174-
scope.addBreadcrumb({ message: 'hello' });
175-
// eslint-disable-next-line deprecation/deprecation
176-
hub.addBreadcrumb({ message: 'world' });
172+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
177173

178-
expect((scope as any)._breadcrumbs).toHaveLength(2);
174+
expect(isolationScopeBreadcrumbs).toEqual([{ message: 'hello3', timestamp: expect.any(Number) }]);
179175
});
180176

181177
test('calls `beforeBreadcrumb` and adds the breadcrumb without any changes', () => {
182-
expect.assertions(1);
183-
184178
const beforeBreadcrumb = jest.fn(breadcrumb => breadcrumb);
185179
const options = getDefaultTestClientOptions({ beforeBreadcrumb });
186180
const client = new TestClient(options);
187-
const scope = new Scope();
188-
// eslint-disable-next-line deprecation/deprecation
189-
const hub = new Hub(client, scope);
181+
setCurrentClient(client);
182+
client.init();
190183

191-
// eslint-disable-next-line deprecation/deprecation
192-
hub.addBreadcrumb({ message: 'hello' });
184+
addBreadcrumb({ message: 'hello' });
193185

194-
expect((scope as any)._breadcrumbs[0].message).toEqual('hello');
186+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
187+
expect(isolationScopeBreadcrumbs).toEqual([{ message: 'hello', timestamp: expect.any(Number) }]);
195188
});
196189

197190
test('calls `beforeBreadcrumb` and uses the new one', () => {
198-
expect.assertions(1);
199-
200191
const beforeBreadcrumb = jest.fn(() => ({ message: 'changed' }));
201192
const options = getDefaultTestClientOptions({ beforeBreadcrumb });
202193
const client = new TestClient(options);
203-
const scope = new Scope();
204-
// eslint-disable-next-line deprecation/deprecation
205-
const hub = new Hub(client, scope);
194+
setCurrentClient(client);
195+
client.init();
206196

207-
// eslint-disable-next-line deprecation/deprecation
208-
hub.addBreadcrumb({ message: 'hello' });
197+
addBreadcrumb({ message: 'hello' });
209198

210-
expect((scope as any)._breadcrumbs[0].message).toEqual('changed');
199+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
200+
expect(isolationScopeBreadcrumbs).toEqual([{ message: 'changed', timestamp: expect.any(Number) }]);
211201
});
212202

213203
test('calls `beforeBreadcrumb` and discards the breadcrumb when returned `null`', () => {
214-
expect.assertions(1);
215-
216204
const beforeBreadcrumb = jest.fn(() => null);
217205
const options = getDefaultTestClientOptions({ beforeBreadcrumb });
218206
const client = new TestClient(options);
219-
const scope = new Scope();
220-
// eslint-disable-next-line deprecation/deprecation
221-
const hub = new Hub(client, scope);
207+
setCurrentClient(client);
208+
client.init();
222209

223-
// eslint-disable-next-line deprecation/deprecation
224-
hub.addBreadcrumb({ message: 'hello' });
210+
addBreadcrumb({ message: 'hello' });
225211

226-
expect((scope as any)._breadcrumbs.length).toEqual(0);
212+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
213+
expect(isolationScopeBreadcrumbs).toEqual([]);
227214
});
228215

229216
test('`beforeBreadcrumb` gets an access to a hint as a second argument', () => {
230-
expect.assertions(2);
231-
232217
const beforeBreadcrumb = jest.fn((breadcrumb, hint) => ({ ...breadcrumb, data: hint.data }));
233218
const options = getDefaultTestClientOptions({ beforeBreadcrumb });
234219
const client = new TestClient(options);
235-
const scope = new Scope();
236-
// eslint-disable-next-line deprecation/deprecation
237-
const hub = new Hub(client, scope);
220+
setCurrentClient(client);
221+
client.init();
238222

239-
// eslint-disable-next-line deprecation/deprecation
240-
hub.addBreadcrumb({ message: 'hello' }, { data: 'someRandomThing' });
223+
addBreadcrumb({ message: 'hello' }, { data: 'someRandomThing' });
241224

242-
expect((scope as any)._breadcrumbs[0].message).toEqual('hello');
243-
expect((scope as any)._breadcrumbs[0].data).toEqual('someRandomThing');
225+
const isolationScopeBreadcrumbs = getIsolationScope().getScopeData().breadcrumbs;
226+
expect(isolationScopeBreadcrumbs).toEqual([
227+
{ message: 'hello', data: 'someRandomThing', timestamp: expect.any(Number) },
228+
]);
244229
});
245230
});
246231

@@ -627,13 +612,12 @@ describe('BaseClient', () => {
627612

628613
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, maxBreadcrumbs: 1 });
629614
const client = new TestClient(options);
615+
setCurrentClient(client);
616+
client.init();
630617
const scope = new Scope();
631-
// eslint-disable-next-line deprecation/deprecation
632-
const hub = new Hub(client, scope);
633-
// eslint-disable-next-line deprecation/deprecation
634-
hub.addBreadcrumb({ message: '1' });
635-
// eslint-disable-next-line deprecation/deprecation
636-
hub.addBreadcrumb({ message: '2' });
618+
619+
addBreadcrumb({ message: '1' });
620+
addBreadcrumb({ message: '2' });
637621

638622
client.captureEvent({ message: 'message' }, undefined, scope);
639623

‎packages/node-experimental/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ export { startSpan, startSpanManual, startInactiveSpan, getActiveSpan } from '@s
3030
export {
3131
getClient,
3232
isInitialized,
33-
addBreadcrumb,
3433
captureException,
3534
captureEvent,
3635
captureMessage,
@@ -55,6 +54,7 @@ export { getCurrentHub, makeMain } from './sdk/hub';
5554
export { Scope } from './sdk/scope';
5655

5756
export {
57+
addBreadcrumb,
5858
makeNodeTransport,
5959
defaultStackParser,
6060
getSentryRelease,

‎packages/node-experimental/src/sdk/api.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
import type { Span } from '@opentelemetry/api';
44
import { context, trace } from '@opentelemetry/api';
55
import type {
6-
Breadcrumb,
7-
BreadcrumbHint,
86
CaptureContext,
97
Event,
108
EventHint,
@@ -15,7 +13,6 @@ import type {
1513
SeverityLevel,
1614
User,
1715
} from '@sentry/types';
18-
import { consoleSandbox, dateTimestampInSeconds } from '@sentry/utils';
1916
import { getContextFromScope, getScopesFromContext, setScopesOnContext } from '../utils/contextData';
2017

2118
import type { ExclusiveEventHintOrCaptureContext } from '../utils/prepareEvent';
@@ -116,29 +113,6 @@ export function captureEvent(event: Event, hint?: EventHint): string {
116113
return getCurrentScope().captureEvent(event, hint);
117114
}
118115

119-
/**
120-
* Add a breadcrumb to the current isolation scope.
121-
*/
122-
export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): void {
123-
const client = getClient();
124-
125-
const { beforeBreadcrumb, maxBreadcrumbs } = client.getOptions();
126-
127-
if (maxBreadcrumbs && maxBreadcrumbs <= 0) return;
128-
129-
const timestamp = dateTimestampInSeconds();
130-
const mergedBreadcrumb = { timestamp, ...breadcrumb };
131-
const finalBreadcrumb = beforeBreadcrumb
132-
? (consoleSandbox(() => beforeBreadcrumb(mergedBreadcrumb, hint)) as Breadcrumb | null)
133-
: mergedBreadcrumb;
134-
135-
if (finalBreadcrumb === null) return;
136-
137-
client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
138-
139-
getIsolationScope().addBreadcrumb(finalBreadcrumb, maxBreadcrumbs);
140-
}
141-
142116
/**
143117
* Add a global event processor.
144118
*/

‎packages/node-experimental/src/sdk/hub.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@ import type {
99
TransactionContext,
1010
} from '@sentry/types';
1111

12-
import { endSession, startSession } from '@sentry/core';
12+
import { addBreadcrumb, endSession, startSession } from '@sentry/core';
1313
import {
14-
addBreadcrumb,
1514
captureEvent,
1615
getClient,
1716
getCurrentScope,

0 commit comments

Comments
 (0)