From 08d98b450c63cc2308ce869039531f6c34cdec1c Mon Sep 17 00:00:00 2001 From: Konstantin Pogorelov Date: Sat, 26 Nov 2016 21:00:58 +0100 Subject: [PATCH] upgrade prom-client to v6, add options: includeMethod, includePath, keepDefaultMetrics, bump 1.2.0 --- README.md | 26 +++++++++++++++++++++++++- advanced-example.js | 29 ++++++++++++++++++----------- package.json | 9 +++++---- spec/PromFactorySpec.js | 4 ---- spec/indexSpec.js | 39 +++++++++++++++++++++++++++++++++------ spec/normalizePathSpec.js | 28 ++++++++++++++++++++++++++++ src/PromFactory.js | 17 ++--------------- src/index.js | 26 ++++++++++++++++++++++++-- src/normalizePath.js | 18 ++++++++++++++++++ 9 files changed, 153 insertions(+), 43 deletions(-) create mode 100644 spec/normalizePathSpec.js create mode 100644 src/normalizePath.js diff --git a/README.md b/README.md index 0bdc641..8ece367 100644 --- a/README.md +++ b/README.md @@ -46,8 +46,26 @@ See the example below. * **prefix**: prefix added to every metric name * **whitelist**, **blacklist**: array of strings or regexp specifying which metrics to include/exclude * **buckets**: buckets used for `http_request_seconds` histogram + * **includeMethod**: include HTTP method (GET, PUT, ...) as a label to `http_request_seconds` + * **includePath**: include URL path as a label - **EXPERIMENTAL!** (see below) + * **normalizePath**: boolean or `function(req)` - path normalization for `includePath` option * **excludeRoutes**: array of strings or regexp specifying which routes should be skipped for `http_request_seconds` metric. It uses `req.path` as subject when checking - * **autoregister**: boolean. If `/metrics` endpoint should be registered. It is **true** by default + * **autoregister**: if `/metrics` endpoint should be registered. It is (Default: **true**) + * **keepDefaultMetrics**: if default metrics provided by **prom-client** should be probed and delivered. (Default: **false**) + +### includePath option + +The goal is to have separate latency statistics by URL path, e.g. `/my-app/user/`, `/products/by-category` etc. + +But just taking `req.path` as a label value won't work as IDs are often part of the URL, like `/user/12352/profile`. So what we actually need is a path template. The automatically module tries to figure out what parts of the path are values or IDs, and what is an actual path. The example mentioned before would be normalized to `/user/#val/profile` and that will become the value for the label. + +You can override this magical behavior and create define your own function by providing an optional callback **normalizePath**. + +For more details: + * [url-value-parser](https://www.npmjs.com/package/url-value-parser) - magic behind automatic path normalization + * [normalizePath.js](https://github.com/jochen-schweizer/express-prom-bundle/blob/master/src/normalizePath.js) - source code for path processing, for you + + ## express example @@ -96,6 +114,12 @@ app.use(/* your middleware */); app.listen(3000); ``` +## Changelog + + * **1.2.0** + * upgrade prom-client to 6.1.2 + * add options: includeMethod, includePath, keepDefaultMetrics + ## License MIT diff --git a/advanced-example.js b/advanced-example.js index 979780e..d833a3d 100644 --- a/advanced-example.js +++ b/advanced-example.js @@ -7,7 +7,10 @@ const promBundle = require("express-prom-bundle"); const bundle = promBundle({ prefix: "demo_app:something:", blacklist: [/up/], - buckets: [0.1, 0.4, 0.7] + buckets: [0.1, 0.4, 0.7], + includeMethod: true, + includePath: true, + keepDefaultMetrics: false }); app.use(bundle); @@ -20,19 +23,23 @@ c1.inc(10); const c2 = bundle.factory.newCounter("c2", "c2 help"); c2.inc(20); -app.get("/foo", (req, res) => { +app.get("/foo/:id", (req, res) => { setTimeout(() => { res.send("foo response\n"); }, 500); }); +app.delete("/foo/:id", (req, res) => { + setTimeout(() => { + res.send("foo deleted\n"); + }, 300); +}); app.get("/bar", (req, res) => res.send("bar response\n")); -app.listen(3000, () => console.log("listening on 3000")); // eslint-disable-line - -/* -test in shell console: - -curl localhost:3000/foo -curl localhost:3000/bar -curl localhost:3000/metrics -*/ \ No newline at end of file +app.listen(3000, () => console.info( // eslint-disable-line + "listening on 3000\n" + + "test in shell console\n\n" + + "curl localhost:3000/foo/1234\n" + + "curl -X DELETE localhost:3000/foo/5432\n" + + "curl localhost:3000/bar\n" + + "curl localhost:3000/metrics\n" +)); diff --git a/package.json b/package.json index 128eff1..51ab89f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "express-prom-bundle", - "version": "1.1.8", + "version": "1.2.0", "description": "express middleware with popular prometheus metrics in one bundle", "main": "src/index.js", "keywords": [ @@ -16,17 +16,18 @@ "license": "MIT", "dependencies": { "on-finished": "^2.3.0", - "prom-client": "^3.4.6" + "prom-client": "^6.1.2", + "url-value-parser": "^1.0.0" }, "devDependencies": { "coveralls": "^2.11.12", - "eslint": "^2.13.1", + "eslint": "^3.11.0", "express": "^4.13.4", "istanbul": "^0.4.4", "jasme": "^5.2.0", "koa": "^1.2.4", "koa-connect": "^1.0.0", - "supertest": "^1.2.0", + "supertest": "^2.0.1", "supertest-koa-agent": "^0.3.0" }, "repository": { diff --git a/spec/PromFactorySpec.js b/spec/PromFactorySpec.js index 326403e..b6d7531 100644 --- a/spec/PromFactorySpec.js +++ b/spec/PromFactorySpec.js @@ -17,10 +17,6 @@ describe("PromFactory", () => { expect(metric.help).toBe("help for test1"); expect(metric.labelNames).toEqual(["label1", "label2"]); }); - it("throws on duplicate names", () => { - factory.newCounter("n","h"); - expect(() => factory.newCounter("n","h2")).toThrow(); - }); it("creates Gauge", () => { const metric = factory.newGauge( "test2", diff --git a/spec/indexSpec.js b/spec/indexSpec.js index e77a3a1..8e8bf3a 100644 --- a/spec/indexSpec.js +++ b/spec/indexSpec.js @@ -1,14 +1,19 @@ "use strict"; /* eslint-env jasmine */ -let express = require("express"), - supertest = require("supertest"), - bundle = require("../"), - koa = require("koa"), - c2k = require("koa-connect"), - supertestKoa = require("supertest-koa-agent"); +const express = require("express"); +const supertest = require("supertest"); +const bundle = require("../"); +const koa = require("koa"); +const c2k = require("koa-connect"); +const supertestKoa = require("supertest-koa-agent"); +const promClient = require("prom-client"); describe("index", () => { + beforeEach(() => { + promClient.register.clear(); + }); + it("metrics returns up=1", done => { const app = express(); const bundled = bundle({ @@ -141,6 +146,28 @@ describe("index", () => { }); }); + it("tolerates includePath, includeMethod and keepDefaultMetrics", done => { + const app = express(); + const instance = bundle({ + includePath: true, + includeMethod: true, + keepDefaultMetrics: true + }); + app.use(instance); + app.use("/test", (req, res) => res.send("it worked")); + const agent = supertest(app); + agent + .get("/test") + .end(() => { + agent + .get("/metrics") + .end((err, res) => { + expect(res.status).toBe(200); + done(); + }); + }); + }); + it("Koa: metrics returns up=1", done => { const app = koa(); const bundled = bundle({ diff --git a/spec/normalizePathSpec.js b/spec/normalizePathSpec.js new file mode 100644 index 0000000..5e1e6c6 --- /dev/null +++ b/spec/normalizePathSpec.js @@ -0,0 +1,28 @@ +"use strict"; +/* eslint-env jasmine */ + +const normalizePath = require("../src/normalizePath"); + +describe("normalizePath", () => { + it("returns original if disabled in opts", () => { + expect( + normalizePath({path: "/a/12345"}, {normalizePath: false}) + ).toBe("/a/12345"); + }); + + it("returns run callback if configured", () => { + expect( + normalizePath( + {path: "/a/12345"}, + { + normalizePath: req => req.path + "-ok" + } + ) + ).toBe("/a/12345-ok"); + }); + + it("uses UrlValueParser by default", () => { + expect(normalizePath({path: "/a/12345"})) + .toBe("/a/#val"); + }); +}); diff --git a/src/PromFactory.js b/src/PromFactory.js index ce71134..44f6647 100644 --- a/src/PromFactory.js +++ b/src/PromFactory.js @@ -4,17 +4,6 @@ module.exports = class { constructor(opts) { this.opts = opts || {}; this.promClient = this.opts.promClient || require("prom-client"); - this.metrics = {}; - } - - metricExists(name) { - return !!this.metrics[name]; - } - - checkDuplicate(name) { - if (this.metricExists(name)) { - throw new Error("trying to add already existing metric: " + name); - } } makeRealName(name) { @@ -25,14 +14,12 @@ module.exports = class { // convert pseudo-array const applyParams = Array.prototype.slice.call(args); const name = applyParams[0]; - this.checkDuplicate(name); const realName = this.makeRealName(name); applyParams[0] = realName; applyParams.unshift(null); // add some dummy context for apply // call constructor with variable params - this.metrics[name] = new (Function.prototype.bind.apply(TheClass, applyParams)); - return this.metrics[name]; + return new (Function.prototype.bind.apply(TheClass, applyParams)); // eslint-disable-line } newCounter() { @@ -50,4 +37,4 @@ module.exports = class { newSummary() { return this.makeMetric(this.promClient.Summary, arguments); } -}; \ No newline at end of file +}; diff --git a/src/index.js b/src/index.js index 1261abd..c30a4b3 100644 --- a/src/index.js +++ b/src/index.js @@ -3,6 +3,8 @@ const PromFactory = require("./PromFactory"); const onFinished = require("on-finished"); const url = require("url"); +const promClient = require("prom-client"); +const normalizePath = require("./normalizePath"); function matchVsRegExps(element, regexps) { for (let regexp of regexps) { @@ -49,6 +51,12 @@ function main(opts) { return; } + // remove default metrics provided by prom-client + if (!opts.keepDefaultMetrics) { + clearInterval(promClient.defaultMetrics()); + promClient.register.clear(); + } + const factory = new PromFactory(opts); const metricTemplates = { @@ -65,10 +73,17 @@ function main(opts) { "value of process.memoryUsage().heapUsed" ), "http_request_seconds": () => { + const labels = ["status_code"]; + if (opts.includeMethod) { + labels.push("method"); + } + if (opts.includePath) { + labels.push("path"); + } const metric = factory.newHistogram( "http_request_seconds", "number of http responses labeled with status code", - ["status_code"], + labels, { buckets: opts.buckets || [0.003, 0.03, 0.1, 0.3, 1.5, 10] } @@ -118,7 +133,13 @@ function main(opts) { labels = {"status_code": 0}; let timer = metrics["http_request_seconds"].startTimer(labels); onFinished(res, () => { - labels["status_code"] = res.statusCode; + labels.status_code = res.statusCode; + if (opts.includeMethod) { + labels.method = req.method; + } + if (opts.includePath) { + labels.path = normalizePath(req, opts); + } timer(); }); } @@ -134,4 +155,5 @@ function main(opts) { return middleware; } +main.promClient = promClient; module.exports = main; diff --git a/src/normalizePath.js b/src/normalizePath.js new file mode 100644 index 0000000..e27a2a3 --- /dev/null +++ b/src/normalizePath.js @@ -0,0 +1,18 @@ +"use strict"; + +const UrlValueParser = require("url-value-parser"); +let urlValueParser; + +module.exports = function(req, opts) { + opts = opts || {}; + if (opts.normalizePath !== undefined && !opts.normalizePath) { + return req.path; + } + if (typeof opts.normalizePath === "function") { + return opts.normalizePath(req, opts); + } + if (!urlValueParser) { + urlValueParser = new UrlValueParser(); + } + return urlValueParser.replacePathValues(req.path); +};