Skip to content

Commit

Permalink
Cover more uses.
Browse files Browse the repository at this point in the history
  • Loading branch information
@onurtemizkan
onurtemizkan committed Mar 15, 2024
1 parent 78b20f8 commit c0b8441
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 18 deletions.
4 changes: 4 additions & 0 deletions plugins/node/opentelemetry-instrumentation-express/src/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ export const asErrorAndMessage = (
* @returns The layer path
*/
export const getLayerPath = (args: unknown[]) => {
if (args[0] instanceof RegExp) {
return args[0].toString();
}

if (typeof args[0] === 'string') {
return args[0];
}
Expand Down
105 changes: 100 additions & 5 deletions plugins/node/opentelemetry-instrumentation-express/test/express.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,24 +567,23 @@ describe('ExpressInstrumentation', () => {
});
});

it('should work with ESM usage and multiple routes including regex', async () => {
it('should set a correct transaction name for routes specified in RegEx', async () => {
await testUtils.runTestFixture({
cwd: __dirname,
argv: ['fixtures/use-express-regex.mjs'],
env: {
NODE_OPTIONS:
'--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
NODE_NO_WARNINGS: '1',
TEST_REGEX_ROUTE: '/test/regex',
},
checkResult: (err, stdout, stderr) => {
assert.ifError(err);
},
checkCollector: (collector: testUtils.TestCollector) => {
const spans = collector.sortedSpans;

console.debug(spans);

assert.strictEqual(spans[0].name, 'GET /test/arr/:id,/\\\/test\\\/arr[0-9]*\\\/required(path)?(\\\/optionalPath)?\\\/(lastParam)?/');
assert.strictEqual(spans[0].name, 'GET /\\/test\\/regex/');
assert.strictEqual(spans[0].kind, SpanKind.CLIENT);
assert.strictEqual(spans[1].name, 'middleware - query');
assert.strictEqual(spans[1].kind, SpanKind.SERVER);
Expand All @@ -595,10 +594,106 @@ describe('ExpressInstrumentation', () => {
assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware');
assert.strictEqual(spans[3].kind, SpanKind.SERVER);
assert.strictEqual(spans[3].parentSpanId, spans[0].spanId);
assert.strictEqual(spans[4].name, 'request handler - /test/arr/:id,/\\\/test\\\/arr[0-9]*\\\/required(path)?(\\\/optionalPath)?\\\/(lastParam)?/');
assert.strictEqual(
spans[4].name,
'request handler - /\\/test\\/regex/'
);
assert.strictEqual(spans[4].kind, SpanKind.SERVER);
assert.strictEqual(spans[4].parentSpanId, spans[0].spanId);
},
});
});

for (const segment of ['array1', 'array5']) {
it('should set a correct transaction name for routes consisting of arrays of routes', async () => {
await testUtils.runTestFixture({
cwd: __dirname,
argv: ['fixtures/use-express-regex.mjs'],
env: {
NODE_OPTIONS:
'--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
NODE_NO_WARNINGS: '1',
TEST_REGEX_ROUTE: `/test/${segment}`,
},
checkResult: err => {
assert.ifError(err);
},
checkCollector: (collector: testUtils.TestCollector) => {
const spans = collector.sortedSpans;

assert.strictEqual(
spans[0].name,
'GET /test/array1,/\\/test\\/array[2-9]/'
);
assert.strictEqual(spans[0].kind, SpanKind.CLIENT);
assert.strictEqual(spans[1].name, 'middleware - query');
assert.strictEqual(spans[1].kind, SpanKind.SERVER);
assert.strictEqual(spans[1].parentSpanId, spans[0].spanId);
assert.strictEqual(spans[2].name, 'middleware - expressInit');
assert.strictEqual(spans[2].kind, SpanKind.SERVER);
assert.strictEqual(spans[2].parentSpanId, spans[0].spanId);
assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware');
assert.strictEqual(spans[3].kind, SpanKind.SERVER);
assert.strictEqual(spans[3].parentSpanId, spans[0].spanId);
assert.strictEqual(
spans[4].name,
'request handler - /test/array1,/\\/test\\/array[2-9]/'
);
assert.strictEqual(spans[4].kind, SpanKind.SERVER);
assert.strictEqual(spans[4].parentSpanId, spans[0].spanId);
},
});
});
}

for (const segment of [
'arr/545',
'arr/required',
'arr/required',
'arr/requiredPath',
'arr/required/lastParam',
'arr55/required/lastParam',
'arr/requiredPath/optionalPath/',
'arr/requiredPath/optionalPath/lastParam',
]) {
it('should handle more complex regexes in route arrays correctly', async () => {
await testUtils.runTestFixture({
cwd: __dirname,
argv: ['fixtures/use-express-regex.mjs'],
env: {
NODE_OPTIONS:
'--experimental-loader=@opentelemetry/instrumentation/hook.mjs',
NODE_NO_WARNINGS: '1',
TEST_REGEX_ROUTE: `/test/${segment}`,
},
checkResult: err => {
assert.ifError(err);
},
checkCollector: (collector: testUtils.TestCollector) => {
const spans = collector.sortedSpans;

assert.strictEqual(
spans[0].name,
'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/'
);
assert.strictEqual(spans[0].kind, SpanKind.CLIENT);
assert.strictEqual(spans[1].name, 'middleware - query');
assert.strictEqual(spans[1].kind, SpanKind.SERVER);
assert.strictEqual(spans[1].parentSpanId, spans[0].spanId);
assert.strictEqual(spans[2].name, 'middleware - expressInit');
assert.strictEqual(spans[2].kind, SpanKind.SERVER);
assert.strictEqual(spans[2].parentSpanId, spans[0].spanId);
assert.strictEqual(spans[3].name, 'middleware - simpleMiddleware');
assert.strictEqual(spans[3].kind, SpanKind.SERVER);
assert.strictEqual(spans[3].parentSpanId, spans[0].spanId);
assert.strictEqual(
spans[4].name,
'request handler - /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/'
);
assert.strictEqual(spans[4].kind, SpanKind.SERVER);
assert.strictEqual(spans[4].parentSpanId, spans[0].spanId);
},
});
});
}
});
40 changes: 27 additions & 13 deletions plugins/node/opentelemetry-instrumentation-express/test/fixtures/use-express-regex.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,8 @@ import { ExpressInstrumentation } from '../../build/src/index.js';

const sdk = createTestNodeSdk({
serviceName: 'use-express-regex',
instrumentations: [
new HttpInstrumentation(),
new ExpressInstrumentation()
]
})
instrumentations: [new HttpInstrumentation(), new ExpressInstrumentation()],
});
sdk.start();

import express from 'express';
Expand All @@ -46,21 +43,38 @@ app.use(async function simpleMiddleware(req, res, next) {
next();
});

app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/], (_req, res) => {
res.send({ response: 'response' });
app.get(
[
'/test/arr/:id',
/\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/,
],
(_req, res) => {
res.send({ response: 'response' });
}
);

app.get(/\/test\/regex/, (_req, res) => {
res.send({ response: 'response 2' });
});

app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => {
res.send({ response: 'response 3' });
});

const server = http.createServer(app);
await new Promise(resolve => server.listen(0, resolve));
const port = server.address().port;

await new Promise(resolve => {
http.get(`http://localhost:${port}/test/arr/requiredPath/optionalPath/lastParam`, (res) => {
res.resume();
res.on('end', () => {
resolve();
});
})
http.get(
`http://localhost:${port}${process.env.TEST_REGEX_ROUTE}`,
res => {
res.resume();
res.on('end', () => {
resolve();
});
}
);
});

await new Promise(resolve => server.close(resolve));
Expand Down

0 comments on commit c0b8441

Please sign in to comment.