Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
46 changes: 39 additions & 7 deletions packages/sdk-metrics/src/view/AttributesProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import type { Context, Attributes } from '@opentelemetry/api';
import { ExactPredicate, PatternPredicate, type Predicate } from './Predicate';

/**
* The {@link AttributesProcessor} is responsible for customizing which
Expand Down Expand Up @@ -41,18 +42,36 @@ class MultiAttributesProcessor implements IAttributesProcessor {
}
}

/**
* Builds one {@link Predicate} per entry in `attributeNames`: a
* {@link PatternPredicate} for entries containing `*`/`?` wildcards, or an
* {@link ExactPredicate} otherwise, avoiding regular expression overhead for
* plain literal names.
*/
function toPredicates(attributeNames: string[]): Predicate[] {
return attributeNames.map(name =>
PatternPredicate.hasWildcard(name)
? new PatternPredicate(name)
: new ExactPredicate(name)
);
}

function matchesAny(predicates: Predicate[], attributeName: string): boolean {
return predicates.some(predicate => predicate.match(attributeName));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we keep the exact names in a Set and only use predicates for entries that contain wildcards? This runs for every measurement, and as written the common all-literal case now scans the whole list for each attribute instead of doing a constant-time lookup.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I’ve updated the implementation to use a Set for exact-match names, making the common all-literal case a constant-time lookup. Entries containing * or ? are handled separately through PatternPredicate. Pushed the changes in 0f00faf.

}

class AllowListProcessor implements IAttributesProcessor {
private readonly _allowedAttributeNames: Set<string>;
private readonly _predicates: Predicate[];
constructor(allowedAttributeNames: string[]) {
this._allowedAttributeNames = new Set(allowedAttributeNames);
this._predicates = toPredicates(allowedAttributeNames);
}

process(incoming: Attributes, _context?: Context): Attributes {
const filteredAttributes: Attributes = {};
for (const attributeName in incoming) {
if (
Object.prototype.hasOwnProperty.call(incoming, attributeName) &&
this._allowedAttributeNames.has(attributeName)
matchesAny(this._predicates, attributeName)
) {
filteredAttributes[attributeName] = incoming[attributeName];
}
Expand All @@ -62,17 +81,17 @@ class AllowListProcessor implements IAttributesProcessor {
}

class DenyListProcessor implements IAttributesProcessor {
private readonly _deniedAttributeNames: Set<string>;
private readonly _predicates: Predicate[];
constructor(deniedAttributeNames: string[]) {
this._deniedAttributeNames = new Set(deniedAttributeNames);
this._predicates = toPredicates(deniedAttributeNames);
}

process(incoming: Attributes, _context?: Context): Attributes {
const filteredAttributes: Attributes = {};
for (const attributeName in incoming) {
if (
Object.prototype.hasOwnProperty.call(incoming, attributeName) &&
!this._deniedAttributeNames.has(attributeName)
!matchesAny(this._predicates, attributeName)
) {
filteredAttributes[attributeName] = incoming[attributeName];
}
Expand Down Expand Up @@ -106,6 +125,13 @@ export function createMultiAttributesProcessor(
/**
* Create an {@link IAttributesProcessor} that filters by allowed attribute names and drops any names that are not in the
* allow list.
*
* Entries may use `*` to match zero or more characters and `?` to match exactly
* one character, following the wildcard semantics of the OpenTelemetry
* `IncludeExclude` configuration type
* (https://opentelemetry.io/docs/specs/otel-config/types/#type-includeexclude).
* For example, `http.request.header.*` matches any attribute name starting
* with `http.request.header.`.
*/
export function createAllowListAttributesProcessor(
attributeAllowList: string[]
Expand All @@ -114,7 +140,13 @@ export function createAllowListAttributesProcessor(
}

/**
* Create an {@link IAttributesProcessor} that drops attributes based on the names provided in the deny list
* Create an {@link IAttributesProcessor} that drops attributes based on the names provided in the deny list.
*
* Entries may use `*` to match zero or more characters and `?` to match exactly
* one character, following the wildcard semantics of the OpenTelemetry
* `IncludeExclude` configuration type
* (https://opentelemetry.io/docs/specs/otel-config/types/#type-includeexclude).
* For example, `*.password` matches any attribute name ending in `.password`.
*/
export function createDenyListAttributesProcessor(
attributeDenyList: string[]
Expand Down
20 changes: 14 additions & 6 deletions packages/sdk-metrics/src/view/Predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,21 @@
*/

// https://tc39.es/proposal-regex-escaping
// escape ^ $ \ . + ? ( ) [ ] { } |
// do not need to escape * as we interpret it as wildcard
const ESCAPE = /[\^$\\.+?()[\]{}|]/g;
// escape ^ $ \ . + ( ) [ ] { } |
// do not need to escape * or ? as we interpret them as wildcards
const ESCAPE = /[\^$\\.+()[\]{}|]/g;

export interface Predicate {
match(str: string): boolean;
}

/**
* Wildcard pattern predicate, supports patterns like `*`, `foo*`, `*bar`.
* Wildcard pattern predicate, supports patterns like `*`, `foo*`, `*bar`,
* as well as `?` to match exactly one character, e.g. `foo?`, `?bar`.
*
* This follows the wildcard semantics of the OpenTelemetry `IncludeExclude`
* configuration type:
* https://opentelemetry.io/docs/specs/otel-config/types/#type-includeexclude
*/
export class PatternPredicate implements Predicate {
private _matchAll: boolean;
Expand All @@ -38,11 +43,14 @@ export class PatternPredicate implements Predicate {
}

static escapePattern(pattern: string): string {
return `^${pattern.replace(ESCAPE, '\\$&').replace('*', '.*')}$`;
return `^${pattern
.replace(ESCAPE, '\\$&')
.replace(/\*/g, '.*')
.replace(/\?/g, '.')}$`;
}

static hasWildcard(pattern: string): boolean {
return pattern.includes('*');
return pattern.includes('*') || pattern.includes('?');
}
}

Expand Down
78 changes: 78 additions & 0 deletions packages/sdk-metrics/test/view/AttributesProcessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,61 @@ describe('AllowListProcessor', () => {
}
);
});

it('should support * wildcard to match zero or more characters', () => {
const processor = createAllowListAttributesProcessor([
'http.request.header.*',
]);
assert.deepStrictEqual(
processor.process(
{
'http.request.header.content-type': 'application/json',
'http.request.header.': 'edgeCase',
'http.response.header.content-type': 'application/json',
},
context.active()
),
{
'http.request.header.content-type': 'application/json',
'http.request.header.': 'edgeCase',
}
);
});

it('should support ? wildcard to match exactly one character', () => {
const processor = createAllowListAttributesProcessor(['attr?']);
assert.deepStrictEqual(
processor.process(
{
attr1: 'a',
attr2: 'b',
attr10: 'c',
attr: 'd',
},
context.active()
),
{
attr1: 'a',
attr2: 'b',
}
);
});

it('should treat other regex meta characters as literal', () => {
const processor = createAllowListAttributesProcessor(['a.b+c']);
assert.deepStrictEqual(
processor.process(
{
'a.b+c': 'kept',
axbyc: 'dropped',
},
context.active()
),
{
'a.b+c': 'kept',
}
);
});
});

describe('DenyListProcessor', () => {
Expand All @@ -71,6 +126,29 @@ describe('DenyListProcessor', () => {
}
);
});

it('should support wildcards when denying attributes', () => {
const processor = createDenyListAttributesProcessor([
'*.password',
'temp_?',
]);
assert.deepStrictEqual(
processor.process(
{
'user.password': 'secret',
'db.password': 'secret',
username: 'kept',
temp_1: 'dropped',
temp_12: 'kept',
},
context.active()
),
{
username: 'kept',
temp_12: 'kept',
}
);
});
});

describe('MultiAttributesProcessor', () => {
Expand Down
27 changes: 25 additions & 2 deletions packages/sdk-metrics/test/view/Predicate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,27 @@ describe('PatternPredicate', () => {
});
});

describe('question-mark match', () => {
it('should match exactly one character', () => {
const predicate = new PatternPredicate('fo?');
assert.ok(predicate.match('foo'));
assert.ok(predicate.match('fob'));

assert.ok(!predicate.match('fo'));
assert.ok(!predicate.match('fooo'));
assert.ok(!predicate.match(''));
});

it('should combine with asterisk', () => {
const predicate = new PatternPredicate('fo?*');
assert.ok(predicate.match('foo'));
assert.ok(predicate.match('foobar'));

assert.ok(!predicate.match('fo'));
assert.ok(!predicate.match(''));
});
});

describe('exact match', () => {
it('should match exactly', () => {
const predicate = new PatternPredicate('foobar');
Expand All @@ -49,13 +70,15 @@ describe('PatternPredicate', () => {
describe('escapePattern', () => {
it('should escape regexp elements', () => {
assert.strictEqual(
PatternPredicate.escapePattern('^$\\.+?()[]{}|'),
'^\\^\\$\\\\\\.\\+\\?\\(\\)\\[\\]\\{\\}\\|$'
PatternPredicate.escapePattern('^$\\.+()[]{}|'),
'^\\^\\$\\\\\\.\\+\\(\\)\\[\\]\\{\\}\\|$'
);
assert.strictEqual(PatternPredicate.escapePattern('*'), '^.*$');
assert.strictEqual(PatternPredicate.escapePattern('foobar'), '^foobar$');
assert.strictEqual(PatternPredicate.escapePattern('foo*'), '^foo.*$');
assert.strictEqual(PatternPredicate.escapePattern('*bar'), '^.*bar$');
assert.strictEqual(PatternPredicate.escapePattern('fo?'), '^fo.$');
assert.strictEqual(PatternPredicate.escapePattern('?bar'), '^.bar$');
});
});
});
Expand Down