-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: http endpoint filtering according to design review (#248)
* feat: span endpoint filtering for client and server spans separately * test: http endpoint filtering * test: more readable test cases * docs: document http endpoint filtering
- Loading branch information
1 parent
f741013
commit c1d8928
Showing
4 changed files
with
172 additions
and
296 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,217 +1,64 @@ | ||
| import { extractUrl, shouldSkipSpanOnRouteMatch } from './lumigoSampler'; | ||
| import { doesEndpointMatchRegexes, extractEndpoint, parseStringToArray } from './lumigoSampler'; | ||
| import { SpanKind } from '@opentelemetry/api'; | ||
|
|
||
| describe('lumigo sampler', () => { | ||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| [ | ||
| // Test happy flow - regex matches url | ||
| { | ||
| url: 'https://example.com', | ||
| regex: '.*example.*', | ||
| shouldSkip: true, | ||
| }, | ||
| { | ||
| url: 'https://example.com', | ||
| regex: 'https://example.com', | ||
| shouldSkip: true, | ||
| }, | ||
| { | ||
| url: 'https://example.com/about', | ||
| regex: 'https://example.com/about', | ||
| shouldSkip: true, | ||
| }, | ||
| { | ||
| url: 'https://example.com/about', | ||
| regex: 'https?://.+/about', | ||
| shouldSkip: true, | ||
| }, | ||
|
|
||
| // Test url doesn't match regex | ||
| { | ||
| url: 'https://example.com', | ||
| regex: '.*not-example.*', | ||
| shouldSkip: false, | ||
| }, | ||
| { | ||
| url: 'https://example.com', | ||
| regex: 'https://not-example.com', | ||
| shouldSkip: false, | ||
| }, | ||
| { | ||
| url: 'https://example.com/about', | ||
| regex: 'https://example.com/not-about', | ||
| shouldSkip: false, | ||
| }, | ||
|
|
||
| // Test regex is invalid | ||
| { | ||
| url: 'https://example.com', | ||
| // The regex is invalid because a char must come before the * operator | ||
| regex: '*', | ||
| shouldSkip: false, | ||
| }, | ||
| test.each` | ||
| rawArrayString | expectedArray | ||
| ${'["a"]'} | ${['a']} | ||
| ${'["a", "b"]'} | ${['a', 'b']} | ||
| ${'[]'} | ${[]} | ||
| ${'"a","b"'} | ${[]} | ||
| ${'Not an array'} | ${[]} | ||
| ${null} | ${[]} | ||
| ${undefined} | ${[]} | ||
| ${'["a", 2]'} | ${[]} | ||
| ${'["a", true]'} | ${[]} | ||
| `('test parse array string', ({ rawArrayString, expectedArray }) => { | ||
| expect(parseStringToArray(rawArrayString)).toEqual(expectedArray); | ||
| }); | ||
|
|
||
| // Test regex is undefined | ||
| { | ||
| url: 'https://example.com', | ||
| regex: undefined, | ||
| shouldSkip: false, | ||
| }, | ||
| { | ||
| url: null, | ||
| regex: '.*', | ||
| shouldSkip: false, | ||
| }, | ||
| { | ||
| url: undefined, | ||
| regex: '.*', | ||
| shouldSkip: false, | ||
| }, | ||
| ].map(({ url, regex, shouldSkip }) => { | ||
| test('test should skip span with url', () => { | ||
| if (regex) { | ||
| process.env.LUMIGO_AUTO_FILTER_HTTP_ENDPOINTS_REGEX = regex; | ||
| } | ||
| expect(shouldSkipSpanOnRouteMatch(url)).toEqual(shouldSkip); | ||
| describe('test when there is a match', () => { | ||
| const endpoint = 'https://example.com'; | ||
| const regexes = ['.*example.*']; | ||
| const expected = true; | ||
| test('test regex match endpoint', () => { | ||
| expect(doesEndpointMatchRegexes(endpoint, regexes)).toEqual(expected); | ||
| }); | ||
| }); | ||
|
|
||
| [ | ||
| { | ||
| description: 'happy flow - full url field exists', | ||
| cases: [ | ||
| { | ||
| attributes: { | ||
| 'http.url': 'https://example.com', | ||
| }, | ||
| expectedUrl: 'https://example.com', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.url': 'https://example.com?page=1', | ||
| }, | ||
| expectedUrl: 'https://example.com?page=1', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| description: 'happy flow - schema host and target fields exist', | ||
| cases: [ | ||
| { | ||
| attributes: { | ||
| 'http.scheme': 'https', | ||
| 'http.host': 'example.com', | ||
| 'http.target': '/', | ||
| }, | ||
| expectedUrl: 'https://example.com', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.scheme': 'https', | ||
| 'http.host': 'example.com', | ||
| 'http.target': '/about', | ||
| }, | ||
| expectedUrl: 'https://example.com/about', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.scheme': 'https', | ||
| 'http.host': 'example.com', | ||
| 'http.target': '/about?page=1', | ||
| }, | ||
| expectedUrl: 'https://example.com/about?page=1', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| description: 'http endpoint standard port', | ||
| cases: [ | ||
| { | ||
| attributes: { | ||
| 'http.url': 'https://example.com:443', | ||
| }, | ||
| expectedUrl: 'https://example.com', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.url': 'http://example.com:80', | ||
| }, | ||
| expectedUrl: 'http://example.com', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| description: 'http endpoint non standard port', | ||
| cases: [ | ||
| { | ||
| attributes: { | ||
| 'http.url': 'https://example.com:80', | ||
| }, | ||
| expectedUrl: 'https://example.com:80', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.url': 'https://example.com:80/about', | ||
| }, | ||
| expectedUrl: 'https://example.com:80/about', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.url': 'http://example.com:443', | ||
| }, | ||
| expectedUrl: 'http://example.com:443', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.url': 'http://example.com:443/about', | ||
| }, | ||
| expectedUrl: 'http://example.com:443/about', | ||
| }, | ||
| { | ||
| attributes: { | ||
| 'http.scheme': 'https', | ||
| 'http.host': 'example.com:80', | ||
| 'http.target': '/about', | ||
| }, | ||
| expectedUrl: 'https://example.com:80/about', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| description: 'http root url', | ||
| cases: [ | ||
| { | ||
| attributes: { | ||
| 'http.url': 'https://example.com/', | ||
| }, | ||
| expectedUrl: 'https://example.com', | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| description: 'missing values', | ||
| cases: [ | ||
| { | ||
| attributes: null, | ||
| expectedUrl: null, | ||
| }, | ||
| { | ||
| attributes: undefined, | ||
| expectedUrl: null, | ||
| }, | ||
| { | ||
| attributes: {}, | ||
| expectedUrl: null, | ||
| }, | ||
| ], | ||
| }, | ||
| ].map(({ description, cases }) => { | ||
| return cases.map(({ attributes, expectedUrl }) => { | ||
| test(`test extract url from span - ${description}`, () => { | ||
| expect(extractUrl(attributes)).toEqual(expectedUrl); | ||
| }); | ||
| }); | ||
| test.each` | ||
| endpoint | regexes | shouldMatch | ||
| ${'https://example.com'} | ${['.*example.*']} | ${true} | ||
| ${'/orders/123'} | ${['.*orders.*']} | ${true} | ||
| ${'/orders/123'} | ${['.*will-not-match.*', '.*orders.*']} | ${true} | ||
| ${'/orders/123'} | ${[]} | ${false} | ||
| ${'/orders/123'} | ${['no-match-1', 'no-match-2']} | ${false} | ||
| ${''} | ${['.*']} | ${false} | ||
| ${null} | ${['.*']} | ${false} | ||
| ${undefined} | ${['.*']} | ${false} | ||
| `('test regex match endpoint', ({ endpoint, regexes, shouldMatch }) => { | ||
| expect(doesEndpointMatchRegexes(endpoint, regexes)).toEqual(shouldMatch); | ||
| }); | ||
|
|
||
| test.each` | ||
| attributes | spanKind | expectedEndpoint | ||
| ${{ 'url.path': 'urlPath', 'http.target': 'httpTarget' }} | ${SpanKind.SERVER} | ${'urlPath'} | ||
| ${{ a: 'a', 'http.target': 'httpTarget' }} | ${SpanKind.SERVER} | ${'httpTarget'} | ||
| ${{ 'url.full': 'fullUrl', 'http.url': 'httpUrl' }} | ${SpanKind.CLIENT} | ${'fullUrl'} | ||
| ${{ a: 'a', 'http.url': 'httpUrl' }} | ${SpanKind.CLIENT} | ${'httpUrl'} | ||
| ${{ | ||
| 'url.path': 'urlPath', | ||
| 'http.target': 'httpTarget', | ||
| 'url.full': 'fullUrl', | ||
| 'http.url': 'httpUrl', | ||
| }} | ${SpanKind.INTERNAL} | ${null} | ||
| ${{}} | ${SpanKind.SERVER} | ${null} | ||
| ${{}} | ${SpanKind.CLIENT} | ${null} | ||
| `('test extract endpoint', ({ attributes, spanKind, expectedEndpoint }) => { | ||
| expect(extractEndpoint(attributes, spanKind)).toEqual(expectedEndpoint); | ||
| }); | ||
| }); |
Oops, something went wrong.