Skip to content

Commit 4c8ffbe

Browse files
andreiborzaandreiborza
committed
Add integration tests covering 401, 402, 403 filtering
1 parent 072ca80 commit 4c8ffbe

File tree

7 files changed

+120
-56
lines changed

7 files changed

+120
-56
lines changed

‎dev-packages/node-integration-tests/suites/express-v5/tracing/scenario-filterStatusCode.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@ app.get('/', (_req, res) => {
88
res.send({ response: 'response 0' });
99
});
1010

11+
app.get('/401', (_req, res) => {
12+
res.status(401).send({ response: 'response 401' });
13+
});
14+
15+
app.get('/402', (_req, res) => {
16+
res.status(402).send({ response: 'response 402' });
17+
});
18+
19+
app.get('/403', (_req, res) => {
20+
res.status(403).send({ response: 'response 403' });
21+
});
22+
1123
app.get('/499', (_req, res) => {
1224
res.status(499).send({ response: 'response 499' });
1325
});

‎dev-packages/node-integration-tests/suites/express-v5/tracing/scenario.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ app.get('/', (_req, res) => {
1515
res.send({ response: 'response 0' });
1616
});
1717

18+
app.get('/401', (_req, res) => {
19+
res.status(401).send({ response: 'response 401' });
20+
});
21+
22+
app.get('/402', (_req, res) => {
23+
res.status(402).send({ response: 'response 402' });
24+
});
25+
26+
app.get('/403', (_req, res) => {
27+
res.status(403).send({ response: 'response 403' });
28+
});
29+
1830
app.get('/test/express', (_req, res) => {
1931
res.send({ response: 'response 1' });
2032
});

‎dev-packages/node-integration-tests/suites/express-v5/tracing/test.ts

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,16 @@ describe('express v5 tracing', () => {
8989
await runner.completed();
9090
});
9191

92-
test('ignores 404 routes by default', async () => {
92+
test.each(['/401', '/402', '/403', '/does-not-exist'])('ignores %s route by default', async (url: string) => {
9393
const runner = createRunner()
9494
.expect({
95-
// No transaction is sent for the 404 route
95+
// No transaction is sent for the 401, 402, 403, 404 routes
9696
transaction: {
9797
transaction: 'GET /',
9898
},
9999
})
100100
.start();
101-
runner.makeRequest('get', '/does-not-exist', { expectError: true });
101+
runner.makeRequest('get', url, { expectError: true });
102102
runner.makeRequest('get', '/');
103103
await runner.completed();
104104
});
@@ -284,33 +284,41 @@ describe('express v5 tracing', () => {
284284
'scenario-filterStatusCode.mjs',
285285
'instrument-filterStatusCode.mjs',
286286
(createRunner, test) => {
287-
// We opt-out of the default 404 filtering in order to test how 404 spans are handled
288-
test('handles 404 route correctly', async () => {
289-
const runner = createRunner()
290-
.expect({
291-
transaction: {
292-
transaction: 'GET /does-not-exist',
293-
contexts: {
294-
trace: {
295-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
296-
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
297-
data: {
298-
'http.response.status_code': 404,
299-
url: expect.stringMatching(/\/does-not-exist$/),
300-
'http.method': 'GET',
301-
'http.url': expect.stringMatching(/\/does-not-exist$/),
302-
'http.target': '/does-not-exist',
287+
// We opt-out of the default [401, 404] fitering in order to test how these spans are handled
288+
test.each([
289+
{ status_code: 401, url: '/401', status: 'unauthenticated' },
290+
{ status_code: 402, url: '/402', status: 'invalid_argument' },
291+
{ status_code: 403, url: '/403', status: 'permission_denied' },
292+
{ status_code: 404, url: '/does-not-exist', status: 'not_found' },
293+
])(
294+
'handles %s route correctly',
295+
async ({ status_code, url, status }: { status_code: number; url: string; status: string }) => {
296+
const runner = createRunner()
297+
.expect({
298+
transaction: {
299+
transaction: `GET ${url}`,
300+
contexts: {
301+
trace: {
302+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
303+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
304+
data: {
305+
'http.response.status_code': status_code,
306+
url: expect.stringMatching(url),
307+
'http.method': 'GET',
308+
'http.url': expect.stringMatching(url),
309+
'http.target': url,
310+
},
311+
op: 'http.server',
312+
status,
303313
},
304-
op: 'http.server',
305-
status: 'not_found',
306314
},
307315
},
308-
},
309-
})
310-
.start();
311-
runner.makeRequest('get', '/does-not-exist', { expectError: true });
312-
await runner.completed();
313-
});
316+
})
317+
.start();
318+
runner.makeRequest('get', url, { expectError: true });
319+
await runner.completed();
320+
},
321+
);
314322

315323
test('filters defined status codes', async () => {
316324
const runner = createRunner()

‎dev-packages/node-integration-tests/suites/express/tracing/scenario-filterStatusCode.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,18 @@ app.get('/', (_req, res) => {
88
res.send({ response: 'response 0' });
99
});
1010

11+
app.get('/401', (_req, res) => {
12+
res.status(401).send({ response: 'response 401' });
13+
});
14+
15+
app.get('/402', (_req, res) => {
16+
res.status(402).send({ response: 'response 402' });
17+
});
18+
19+
app.get('/403', (_req, res) => {
20+
res.status(403).send({ response: 'response 403' });
21+
});
22+
1123
app.get('/499', (_req, res) => {
1224
res.status(499).send({ response: 'response 499' });
1325
});

‎dev-packages/node-integration-tests/suites/express/tracing/scenario.mjs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,18 @@ app.get('/', (_req, res) => {
1515
res.send({ response: 'response 0' });
1616
});
1717

18+
app.get('/401', (_req, res) => {
19+
res.status(401).send({ response: 'response 401' });
20+
});
21+
22+
app.get('/402', (_req, res) => {
23+
res.status(402).send({ response: 'response 402' });
24+
});
25+
26+
app.get('/403', (_req, res) => {
27+
res.status(403).send({ response: 'response 403' });
28+
});
29+
1830
app.get('/test/express', (_req, res) => {
1931
res.send({ response: 'response 1' });
2032
});

‎dev-packages/node-integration-tests/suites/express/tracing/test.ts

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,16 @@ describe('express tracing', () => {
9090
await runner.completed();
9191
});
9292

93-
test('ignores 404 routes by default', async () => {
93+
test.each(['/401', '/402', '/403', '/does-not-exist'])('ignores %s route by default', async (url: string) => {
9494
const runner = createRunner()
9595
.expect({
96-
// No transaction is sent for the 404 route
96+
// No transaction is sent for the 401, 402, 403, 404 routes
9797
transaction: {
9898
transaction: 'GET /',
9999
},
100100
})
101101
.start();
102-
runner.makeRequest('get', '/does-not-exist', { expectError: true });
102+
runner.makeRequest('get', url, { expectError: true });
103103
runner.makeRequest('get', '/');
104104
await runner.completed();
105105
});
@@ -315,34 +315,42 @@ describe('express tracing', () => {
315315
'scenario-filterStatusCode.mjs',
316316
'instrument-filterStatusCode.mjs',
317317
(createRunner, test) => {
318-
// We opt-out of the default 404 filtering in order to test how 404 spans are handled
319-
test('handles 404 route correctly', async () => {
320-
const runner = createRunner()
321-
.expect({
322-
transaction: {
323-
// FIXME: This is incorrect, sadly :(
324-
transaction: 'GET /',
325-
contexts: {
326-
trace: {
327-
span_id: expect.stringMatching(/[a-f0-9]{16}/),
328-
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
329-
data: {
330-
'http.response.status_code': 404,
331-
url: expect.stringMatching(/\/does-not-exist$/),
332-
'http.method': 'GET',
333-
'http.url': expect.stringMatching(/\/does-not-exist$/),
334-
'http.target': '/does-not-exist',
318+
// We opt-out of the default [401, 404] filtering in order to test how these spans are handled
319+
test.each([
320+
{ status_code: 401, url: '/401', status: 'unauthenticated' },
321+
{ status_code: 402, url: '/402', status: 'invalid_argument' },
322+
{ status_code: 403, url: '/403', status: 'permission_denied' },
323+
{ status_code: 404, url: '/does-not-exist', status: 'not_found' },
324+
])(
325+
'handles %s route correctly',
326+
async ({ status_code, url, status }: { status_code: number; url: string; status: string }) => {
327+
const runner = createRunner()
328+
.expect({
329+
transaction: {
330+
// TODO(v10): This is incorrect on OpenTelemetry v1 but can be fixed in v2
331+
transaction: `GET ${status_code === 404 ? '/' : url}`,
332+
contexts: {
333+
trace: {
334+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
335+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
336+
data: {
337+
'http.response.status_code': status_code,
338+
url: expect.stringMatching(url),
339+
'http.method': 'GET',
340+
'http.url': expect.stringMatching(url),
341+
'http.target': url,
342+
},
343+
op: 'http.server',
344+
status,
335345
},
336-
op: 'http.server',
337-
status: 'not_found',
338346
},
339347
},
340-
},
341-
})
342-
.start();
343-
runner.makeRequest('get', '/does-not-exist', { expectError: true });
344-
await runner.completed();
345-
});
348+
})
349+
.start();
350+
runner.makeRequest('get', url, { expectError: true });
351+
await runner.completed();
352+
},
353+
);
346354

347355
test('filters defined status codes', async () => {
348356
const runner = createRunner()

‎dev-packages/rollup-utils/npmHelpers.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export function makeBaseNPMConfig(options = {}) {
9393
}
9494

9595
return true;
96-
}
96+
},
9797
},
9898

9999
plugins: [nodeResolvePlugin, sucrasePlugin, debugBuildStatementReplacePlugin, rrwebBuildPlugin, cleanupPlugin],

0 commit comments

Comments
 (0)