From bff28958b92d81cbbcc351ea0ee57a86cc59cd9a Mon Sep 17 00:00:00 2001 From: Bernardo Guerreiro Date: Tue, 2 Jul 2024 11:55:44 +0100 Subject: [PATCH] feat(cli): set nontemplated url as req name by default in metrics --- .../index.js | 47 ++++++-- .../test/fixtures/scenario-templated-url.yml | 18 +++ .../test/index.spec.js | 104 ++++++++++++++++ .../test/index.unit.js | 111 ++++++++++++++++-- 4 files changed, 256 insertions(+), 24 deletions(-) create mode 100644 packages/artillery-plugin-metrics-by-endpoint/test/fixtures/scenario-templated-url.yml diff --git a/packages/artillery-plugin-metrics-by-endpoint/index.js b/packages/artillery-plugin-metrics-by-endpoint/index.js index 55f503ebec..b1f14bfee4 100644 --- a/packages/artillery-plugin-metrics-by-endpoint/index.js +++ b/packages/artillery-plugin-metrics-by-endpoint/index.js @@ -1,7 +1,6 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - const url = require('url'); module.exports = { Plugin: MetricsByEndpoint }; @@ -12,6 +11,7 @@ let useOnlyRequestNames; let stripQueryString; let ignoreUnnamedRequests; let metricsPrefix; +let useDefaultName; // NOTE: Will not work with `parallel` - need request UIDs for that function MetricsByEndpoint(script, events) { @@ -43,20 +43,26 @@ function MetricsByEndpoint(script, events) { metricsPrefix = script.config.plugins['metrics-by-endpoint'].metricsNamespace || 'plugins.metrics-by-endpoint'; + useDefaultName = + script.config.plugins['metrics-by-endpoint'].useDefaultName ?? true; script.config.processor.metricsByEndpoint_afterResponse = metricsByEndpoint_afterResponse; script.config.processor.metricsByEndpoint_onError = metricsByEndpoint_onError; + script.config.processor.metricsByEndpoint_beforeRequest = + metricsByEndpoint_beforeRequest; script.scenarios.forEach(function (scenario) { scenario.afterResponse = [].concat(scenario.afterResponse || []); scenario.afterResponse.push('metricsByEndpoint_afterResponse'); scenario.onError = [].concat(scenario.onError || []); scenario.onError.push('metricsByEndpoint_onError'); + scenario.beforeRequest = [].concat(scenario.beforeRequest || []); + scenario.beforeRequest.push('metricsByEndpoint_beforeRequest'); }); } -function getReqName(target, originalRequestUrl, requestName) { +function calculateBaseUrl(target, originalRequestUrl) { const targetUrl = target && url.parse(target); const requestUrl = url.parse(originalRequestUrl); @@ -73,20 +79,33 @@ function getReqName(target, originalRequestUrl, requestName) { } baseUrl += stripQueryString ? requestUrl.pathname : requestUrl.path; - let reqName = ''; - if (useOnlyRequestNames && requestName) { - reqName += requestName; - } else if (requestName) { - reqName += `${baseUrl} (${requestName})`; - } else if (!ignoreUnnamedRequests) { - reqName += baseUrl; + return decodeURIComponent(baseUrl); +} + +function getReqName(target, originalRequestUrl, requestName) { + const baseUrl = calculateBaseUrl(target, originalRequestUrl); + + if (!requestName) { + return ignoreUnnamedRequests ? '' : baseUrl; } - return reqName; + return useOnlyRequestNames ? requestName : `${baseUrl} (${requestName})`; +} + +function metricsByEndpoint_beforeRequest(req, userContext, events, done) { + if (useDefaultName) { + req.defaultName = getReqName(userContext.vars.target, req.url, req.name); + } + + return done(); } function metricsByEndpoint_onError(err, req, userContext, events, done) { - const reqName = getReqName(userContext.vars.target, req.url, req.name); + //if useDefaultName is true, then req.defaultName is set in beforeRequest + //otherwise, we must calculate the reqName here as req.url is the non-templated version + const reqName = useDefaultName + ? req.defaultName + : getReqName(userContext.vars.target, req.url, req.name); if (reqName === '') { return done(); @@ -102,7 +121,11 @@ function metricsByEndpoint_onError(err, req, userContext, events, done) { } function metricsByEndpoint_afterResponse(req, res, userContext, events, done) { - const reqName = getReqName(userContext.vars.target, req.url, req.name); + //if useDefaultName is true, then req.defaultName is set in beforeRequest + //otherwise, we must calculate the reqName here as req.url is the non-templated version + const reqName = useDefaultName + ? req.defaultName + : getReqName(userContext.vars.target, req.url, req.name); if (reqName === '') { return done(); diff --git a/packages/artillery-plugin-metrics-by-endpoint/test/fixtures/scenario-templated-url.yml b/packages/artillery-plugin-metrics-by-endpoint/test/fixtures/scenario-templated-url.yml new file mode 100644 index 0000000000..9d91daafc4 --- /dev/null +++ b/packages/artillery-plugin-metrics-by-endpoint/test/fixtures/scenario-templated-url.yml @@ -0,0 +1,18 @@ +config: + target: http://asciiart.artillery.io:8080 + phases: + - duration: 2 + arrivalRate: 2 + plugins: + metrics-by-endpoint: + stripQueryString: true + +scenarios: + - flow: + - get: + url: "/dino/{{ $randomString() }}?potato=1&tomato=2" + name: "GET /dino" + - get: + url: "/armadillo/{{ $randomString() }}" + - get: + url: "/pony" \ No newline at end of file diff --git a/packages/artillery-plugin-metrics-by-endpoint/test/index.spec.js b/packages/artillery-plugin-metrics-by-endpoint/test/index.spec.js index 517d95c18c..294f95f00b 100644 --- a/packages/artillery-plugin-metrics-by-endpoint/test/index.spec.js +++ b/packages/artillery-plugin-metrics-by-endpoint/test/index.spec.js @@ -78,3 +78,107 @@ test("Reports correctly when 'parallel' is used", async (t) => { ); } }); + +test('Reports correctly when `useDefaultName` is set to true (default)', async (t) => { + const reportPath = + os.tmpdir() + '/artillery-plugin-metrics-by-endpoint-use-path-as-name.json'; + const output = + await $`../artillery/bin/run run ./test/fixtures/scenario-templated-url.yml -o ${reportPath}`; + + const report = require(reportPath); + + t.equal(output.exitCode, 0, 'CLI Exit Code should be 0'); + t.equal( + report.aggregate.counters[ + 'plugins.metrics-by-endpoint./armadillo/{{ $randomString() }}.codes.200' + ], + 4, + 'should have counter metrics including templated url and no query strings' + ); + t.equal( + report.aggregate.counters[ + 'plugins.metrics-by-endpoint./dino/{{ $randomString() }} (GET /dino).codes.200' + ], + 4, + 'should have counter metrics including templated url with request name specified' + ); + t.equal( + report.aggregate.counters['plugins.metrics-by-endpoint./pony.codes.200'], + 4 + ), + 'should display counter metrics for /pony as normal'; + + t.ok( + Object.keys(report.aggregate.summaries).includes( + 'plugins.metrics-by-endpoint.response_time./armadillo/{{ $randomString() }}' + ), + 'should have summary metrics including templated url' + ); + t.ok( + Object.keys(report.aggregate.summaries).includes( + 'plugins.metrics-by-endpoint.response_time./dino/{{ $randomString() }} (GET /dino)' + ), + 'should have summary metrics including templated url with request name specified' + ); + t.ok( + Object.keys(report.aggregate.summaries).includes( + 'plugins.metrics-by-endpoint.response_time./pony' + ), + 'should display summary metrics for /pony as normal' + ); +}); + +test('Reports correctly when `useDefaultName` is explicitly set to false', async (t) => { + const reportPath = + os.tmpdir() + + '/artillery-plugin-metrics-by-endpoint-use-path-without-name-test.json'; + const overrides = { + config: { + plugins: { + 'metrics-by-endpoint': { + useDefaultName: false, + stripQueryString: false + } + } + } + }; + const output = + await $`../artillery/bin/run run ./test/fixtures/scenario-templated-url.yml -o ${reportPath} --overrides ${JSON.stringify( + overrides + )}`; + + const report = require(reportPath); + + t.equal(output.exitCode, 0, 'CLI Exit Code should be 0'); + + const aggregateCounters = Object.keys(report.aggregate.counters); + + const countersWithName = aggregateCounters.filter((counter) => { + return new RegExp( + /plugins\.metrics-by-endpoint\.\/dino\/[a-zA-Z0-9]+\.?\w+\?potato=1&tomato=2 \(GET \/dino\)\.codes\.200/ + ).test(counter); + }); + + const countersWithoutName = aggregateCounters.filter((counter) => { + return new RegExp( + /plugins\.metrics-by-endpoint\.\/armadillo\/[a-zA-Z0-9]+\.?\w+\.codes\.200/ + ).test(counter); + }); + + const regularPonyCounter = aggregateCounters.filter( + (counter) => counter == 'plugins.metrics-by-endpoint./pony.codes.200' + ); + + t.ok( + countersWithName.length > 0, + `should have counter metrics without the templated url, got ${countersWithName}` + ); + t.ok( + countersWithoutName.length > 0, + `should have counter metrics without the templated url and request name specified, got ${countersWithoutName}` + ); + t.ok( + regularPonyCounter.length == 1, + `should display counter metrics for /pony as normal, got ${regularPonyCounter}` + ); +}); diff --git a/packages/artillery-plugin-metrics-by-endpoint/test/index.unit.js b/packages/artillery-plugin-metrics-by-endpoint/test/index.unit.js index 57af9cf49e..5712bcc33e 100644 --- a/packages/artillery-plugin-metrics-by-endpoint/test/index.unit.js +++ b/packages/artillery-plugin-metrics-by-endpoint/test/index.unit.js @@ -33,7 +33,7 @@ const baseScript = { ] }; -test('afterResponse', async (t) => { +test('beforeRequest and afterResponse', async (t) => { let defaultPluginPrefix = 'plugins.metrics-by-endpoint'; let script; let hookArgs; @@ -85,18 +85,28 @@ test('afterResponse', async (t) => { }; }); - t.test('sets up afterResponse hook correctly', async (t) => { - new Plugin(script, hookArgs.events); - - // check afterResponse is in processor - t.hasProp(script.config.processor, 'metricsByEndpoint_afterResponse'); + t.test( + 'sets up beforeRequest and afterResponse hook correctly', + async (t) => { + new Plugin(script, hookArgs.events); - // check afterResponse is each scenario - script.scenarios.forEach((scenario) => { - t.equal(scenario.afterResponse.length, 1); - t.equal(scenario.afterResponse[0], 'metricsByEndpoint_afterResponse'); - }); - }); + // check afterResponse is in processor + t.hasProp(script.config.processor, 'metricsByEndpoint_afterResponse'); + t.hasProp(script.config.processor, 'metricsByEndpoint_beforeRequest'); + + // check afterResponse is each scenario + script.scenarios.forEach((scenario) => { + t.equal(scenario.afterResponse.length, 1); + t.equal(scenario.afterResponse[0], 'metricsByEndpoint_afterResponse'); + }); + + // check beforeRequest is each scenario + script.scenarios.forEach((scenario) => { + t.equal(scenario.beforeRequest.length, 1); + t.equal(scenario.beforeRequest[0], 'metricsByEndpoint_beforeRequest'); + }); + } + ); t.test('only runs plugin inside workers', async (t) => { delete process.env.LOCAL_WORKER_ID; @@ -111,6 +121,13 @@ test('afterResponse', async (t) => { async (t) => { new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -145,6 +162,13 @@ test('afterResponse', async (t) => { hookArgs.req.url = `http://${requestUrlWithoutProtocol}`; new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -178,6 +202,13 @@ test('afterResponse', async (t) => { hookArgs.req.url = requestWithPort; new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -207,6 +238,13 @@ test('afterResponse', async (t) => { const serverTiming = 105; hookArgs.res.headers['server-timing'] = `total;dur=${serverTiming}`; + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -237,6 +275,13 @@ test('afterResponse', async (t) => { const serverTiming = 105; hookArgs.res.headers['server-timing'] = `total;potatoes=${serverTiming}`; + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -269,6 +314,13 @@ test('afterResponse', async (t) => { const reqName = 'bunnyRequest123'; hookArgs.req.name = reqName; + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -301,6 +353,13 @@ test('afterResponse', async (t) => { const reqName = 'bunnyRequest123'; hookArgs.req.name = reqName; + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -335,6 +394,13 @@ test('afterResponse', async (t) => { }; new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -365,6 +431,13 @@ test('afterResponse', async (t) => { }; new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -387,6 +460,13 @@ test('afterResponse', async (t) => { hookArgs.req.name = 'iAmNamed'; new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res, @@ -421,6 +501,13 @@ test('afterResponse', async (t) => { hookArgs.req.url = '/dino?query=stringy&another=one'; new Plugin(script, hookArgs.events); + script.config.processor.metricsByEndpoint_beforeRequest( + hookArgs.req, + hookArgs.userContext, + hookArgs.events, + hookArgs.done + ); + script.config.processor.metricsByEndpoint_afterResponse( hookArgs.req, hookArgs.res,