Skip to content

Commit 5208161

Browse files
committed
union type options object, improve filtering performance
1 parent 1c50529 commit 5208161

File tree

5 files changed

+49
-24
lines changed

5 files changed

+49
-24
lines changed

packages/core/src/client.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,7 +1296,7 @@ function processBeforeSend(
12961296
const rootSpanJson = convertTransactionEventToSpanJson(processedEvent);
12971297

12981298
// 1.1 If the root span should be ignored, drop the whole transaction
1299-
if (shouldIgnoreSpan(rootSpanJson, ignoreSpans)) {
1299+
if (ignoreSpans?.length && shouldIgnoreSpan(rootSpanJson, ignoreSpans)) {
13001300
// dropping the whole transaction!
13011301
return null;
13021302
}
@@ -1320,7 +1320,7 @@ function processBeforeSend(
13201320

13211321
for (const span of initialSpans) {
13221322
// 2.a If the child span should be ignored, reparent it to the root span
1323-
if (shouldIgnoreSpan(span, ignoreSpans)) {
1323+
if (ignoreSpans?.length && shouldIgnoreSpan(span, ignoreSpans)) {
13241324
reparentChildSpans(initialSpans, span);
13251325
continue;
13261326
}

packages/core/src/envelope.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ export function createSpanEnvelope(spans: [SentrySpan, ...SentrySpan[]], client?
125125

126126
const { beforeSendSpan, ignoreSpans } = client?.getOptions() || {};
127127

128-
const filteredSpans = ignoreSpans ? spans.filter(span => !shouldIgnoreSpan(spanToJSON(span), ignoreSpans)) : spans;
128+
const filteredSpans = ignoreSpans?.length
129+
? spans.filter(span => !shouldIgnoreSpan(spanToJSON(span), ignoreSpans))
130+
: spans;
129131
const droppedSpans = spans.length - filteredSpans.length;
130132

131133
if (droppedSpans) {

packages/core/src/types-hoist/options.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,31 @@ import type { StackLineParser, StackParser } from './stacktrace';
1010
import type { TracePropagationTargets } from './tracing';
1111
import type { BaseTransportOptions, Transport } from './transport';
1212

13-
interface IgnoreSpanFilter {
14-
name?: string | RegExp;
15-
op?: string | RegExp;
16-
}
13+
/**
14+
* A filter object for ignoring spans.
15+
* At least one of the properties (`op` or `name`) must be set.
16+
*/
17+
type IgnoreSpanFilter =
18+
| {
19+
/**
20+
* Spans with a name matching this pattern will be ignored.
21+
*/
22+
name: string | RegExp;
23+
/**
24+
* Spans with an op matching this pattern will be ignored.
25+
*/
26+
op?: string | RegExp;
27+
}
28+
| {
29+
/**
30+
* Spans with a name matching this pattern will be ignored.
31+
*/
32+
name?: string | RegExp;
33+
/**
34+
* Spans with an op matching this pattern will be ignored.
35+
*/
36+
op: string | RegExp;
37+
};
1738

1839
export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOptions> {
1940
/**
@@ -216,6 +237,9 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
216237
/**
217238
* A list of span names or patterns to ignore.
218239
*
240+
* If you specify a pattern {@link IgnoreSpanFilter}, at least one
241+
* of the properties (`op` or `name`) must be set.
242+
*
219243
* @default []
220244
*/
221245
ignoreSpans?: (string | RegExp | IgnoreSpanFilter)[];

packages/core/src/utils/should-ignore-span.ts

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,37 @@
11
import type { ClientOptions } from '../types-hoist/options';
22
import type { SpanJSON } from '../types-hoist/span';
3-
import { isMatchingPattern, stringMatchesSomePattern } from './string';
3+
import { isMatchingPattern } from './string';
44

55
/**
66
* Check if a span should be ignored based on the ignoreSpans configuration.
77
*/
88
export function shouldIgnoreSpan(
99
span: Pick<SpanJSON, 'description' | 'op'>,
10-
ignoreSpans: ClientOptions['ignoreSpans'],
10+
ignoreSpans: Required<ClientOptions>['ignoreSpans'],
1111
): boolean {
12-
if (!ignoreSpans?.length) {
12+
if (!ignoreSpans?.length || !span.description) {
1313
return false;
1414
}
1515

16-
if (!span.description) {
17-
return false;
18-
}
19-
20-
// First we check the simple string/regex patterns - if the name matches any of them, we ignore the span
21-
const simplePatterns = ignoreSpans.filter(isStringOrRegExp);
22-
if (simplePatterns.length && stringMatchesSomePattern(span.description, simplePatterns)) {
23-
return true;
24-
}
25-
26-
// Then we check the more complex patterns, where both parts must match
2716
for (const pattern of ignoreSpans) {
28-
// Have already checked for simple patterns, so we can skip these
29-
if (isStringOrRegExp(pattern) || (!pattern.name && !pattern.op)) {
17+
if (isStringOrRegExp(pattern)) {
18+
if (isMatchingPattern(span.description, pattern)) {
19+
return true;
20+
}
21+
continue;
22+
}
23+
24+
if (!pattern.name && !pattern.op) {
3025
continue;
3126
}
3227

3328
const nameMatches = pattern.name ? isMatchingPattern(span.description, pattern.name) : true;
3429
const opMatches = pattern.op ? span.op && isMatchingPattern(span.op, pattern.op) : true;
3530

31+
// This check here is only correct because we can guarantee that we ran `isMatchingPattern`
32+
// for at least one of `nameMatches` and `opMatches`. So in contrary to how this looks,
33+
// not both op and name actually have to match. This is the most efficient way to check
34+
// for all combinations of name and op patterns.
3635
if (nameMatches && opMatches) {
3736
return true;
3837
}

packages/core/test/lib/utils/should-ignore-span.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { reparentChildSpans, shouldIgnoreSpan } from '../../../src/utils/should-
55
describe('shouldIgnoreSpan', () => {
66
it('should not ignore spans with empty ignoreSpans', () => {
77
const span = { description: 'test', op: 'test' };
8-
const ignoreSpans = [] as ClientOptions['ignoreSpans'];
8+
const ignoreSpans = [] as Required<ClientOptions>['ignoreSpans'];
99
expect(shouldIgnoreSpan(span, ignoreSpans)).toBe(false);
1010
});
1111

0 commit comments

Comments
 (0)