Skip to content
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
8 changes: 1 addition & 7 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ export const API_VERSION = 4;
*/
const DEFAULT_BREADCRUMBS = 100;

/**
* Absolute maximum number of breadcrumbs added to an event. The
* `maxBreadcrumbs` option cannot be higher than this value.
*/
const MAX_BREADCRUMBS = 100;

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this could break downstream dependencies. Could we leave it in?

/**
* A layer in the process stack.
* @hidden
Expand Down Expand Up @@ -289,7 +283,7 @@ export class Hub implements HubInterface {

if (finalBreadcrumb === null) return;

scope.addBreadcrumb(finalBreadcrumb, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS));
scope.addBreadcrumb(finalBreadcrumb, maxBreadcrumbs);
}

/**
Expand Down
20 changes: 15 additions & 5 deletions packages/hub/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ import { dateTimestampInSeconds, getGlobalObject, isPlainObject, isThenable, Syn

import { Session } from './session';

/**
* Absolute maximum number of breadcrumbs added to an event.
* The `maxBreadcrumbs` option cannot be higher than this value.
*/
const MAX_BREADCRUMBS = 100;

/**
* Holds additional event information. {@link Scope.applyToEvent} will be
* called by the client before an event will be sent.
Expand Down Expand Up @@ -366,16 +372,20 @@ export class Scope implements ScopeInterface {
* @inheritDoc
*/
public addBreadcrumb(breadcrumb: Breadcrumb, maxBreadcrumbs?: number): this {
const maxCrumbs = typeof maxBreadcrumbs === 'number' ? Math.min(maxBreadcrumbs, MAX_BREADCRUMBS) : MAX_BREADCRUMBS;

// No data has been changed, so don't notify scope listeners
if (maxCrumbs <= 0) {
return this;
}

const mergedBreadcrumb = {
timestamp: dateTimestampInSeconds(),
...breadcrumb,
};

this._breadcrumbs =
maxBreadcrumbs !== undefined && maxBreadcrumbs >= 0
? [...this._breadcrumbs, mergedBreadcrumb].slice(-maxBreadcrumbs)
: [...this._breadcrumbs, mergedBreadcrumb];
this._breadcrumbs = [...this._breadcrumbs, mergedBreadcrumb].slice(-maxCrumbs);
this._notifyScopeListeners();

return this;
}

Expand Down
26 changes: 21 additions & 5 deletions packages/hub/test/scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,26 @@ describe('Scope', () => {

test('addBreadcrumb', () => {
const scope = new Scope();
scope.addBreadcrumb({ message: 'test' }, 100);
scope.addBreadcrumb({ message: 'test' });
expect((scope as any)._breadcrumbs[0]).toHaveProperty('message', 'test');
});

test('addBreadcrumb can be limited to hold up to N breadcrumbs', () => {
const scope = new Scope();
for (let i = 0; i < 10; i++) {
scope.addBreadcrumb({ message: 'test' }, 5);
}
expect((scope as any)._breadcrumbs).toHaveLength(5);
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 also specifically check for which 5 breadcrumbs we want: last, first, etc. I think it should be last N, right?

});

test('addBreadcrumb cannot go over MAX_BREADCRUMBS value', () => {
const scope = new Scope();
for (let i = 0; i < 111; i++) {
scope.addBreadcrumb({ message: 'test' }, 111);
}
expect((scope as any)._breadcrumbs).toHaveLength(100);
});

test('setLevel', () => {
const scope = new Scope();
scope.setLevel(Severity.Critical);
Expand Down Expand Up @@ -181,7 +197,7 @@ describe('Scope', () => {
scope.setFingerprint(['abcd']);
scope.setLevel(Severity.Warning);
scope.setTransactionName('/abc');
scope.addBreadcrumb({ message: 'test' }, 100);
scope.addBreadcrumb({ message: 'test' });
scope.setContext('os', { id: '1' });
const event: Event = {};
return scope.applyToEvent(event).then(processedEvent => {
Expand All @@ -203,7 +219,7 @@ describe('Scope', () => {
scope.setTag('a', 'b');
scope.setUser({ id: '1' });
scope.setFingerprint(['abcd']);
scope.addBreadcrumb({ message: 'test' }, 100);
scope.addBreadcrumb({ message: 'test' });
scope.setContext('server', { id: '2' });
const event: Event = {
breadcrumbs: [{ message: 'test1' }],
Expand Down Expand Up @@ -358,7 +374,7 @@ describe('Scope', () => {
scope.setTag('a', 'b');
scope.setUser({ id: '1' });
scope.setFingerprint(['abcd']);
scope.addBreadcrumb({ message: 'test' }, 100);
scope.addBreadcrumb({ message: 'test' });
scope.setRequestSession({ status: RequestSessionStatus.Ok });
expect((scope as any)._extra).toEqual({ a: 2 });
scope.clear();
Expand All @@ -368,7 +384,7 @@ describe('Scope', () => {

test('clearBreadcrumbs', () => {
const scope = new Scope();
scope.addBreadcrumb({ message: 'test' }, 100);
scope.addBreadcrumb({ message: 'test' });
expect((scope as any)._breadcrumbs).toHaveLength(1);
scope.clearBreadcrumbs();
expect((scope as any)._breadcrumbs).toHaveLength(0);
Expand Down