From 06727dfe5fc86a40002ff8f00d02db3af84d0008 Mon Sep 17 00:00:00 2001 From: Konstantin Pogorelov Date: Thu, 28 Jul 2016 02:09:27 +0200 Subject: [PATCH] update deps, fix sumary factory method, add buckets param, extend unit tests, add coverage --- .gitignore | 1 + .npmignore | 1 + .travis.yml | 1 + LICENSE | 21 +++++++++++ Makefile | 14 ++++++++ README.md | 27 ++++++++++----- advanced-example.js | 30 ++++++++++------ package.json | 16 +++++---- spec/PromFactorySpec.js | 57 ++++++++++++++++++++++++++++++ spec/indexSpec.js | 77 +++++++++++++++++++++++++++++++++++------ src/PromFactory.js | 29 +++++++++------- src/index.js | 21 +++++------ 12 files changed, 235 insertions(+), 60 deletions(-) create mode 100644 LICENSE create mode 100644 spec/PromFactorySpec.js diff --git a/.gitignore b/.gitignore index 3b2c247..2b46987 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .npmrc node_modules +coverage diff --git a/.npmignore b/.npmignore index c02dd96..1d2fbf9 100644 --- a/.npmignore +++ b/.npmignore @@ -3,3 +3,4 @@ docker-compose.yml spec .travis.yml .eslintrc +coverage diff --git a/.travis.yml b/.travis.yml index 29013be..3a9c633 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,5 @@ language: node_js node_js: + - "6" - "5" - "4" diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..d921781 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2016 Jochen Schweizer Technology Solutions GmbH + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. \ No newline at end of file diff --git a/Makefile b/Makefile index ffcb65e..0f1d4d7 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,18 @@ +.PHONY: coverage + test: ./node_modules/jasme/run.js lint: node_modules/eslint/bin/eslint.js src +coverage: + node_modules/istanbul/lib/cli.js cover \ + -i 'src/*' \ + --include-all-sources \ + --dir coverage \ + node_modules/jasme/run.js + +coveralls: coverage +ifndef COVERALLS_REPO_TOKEN + $(error COVERALLS_REPO_TOKEN is undefined) +endif + node_modules/coveralls/bin/coveralls.js < coverage/lcov.info diff --git a/README.md b/README.md index 186c3d3..54ecf32 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -[![build status](https://travis-ci.org/jochen-schweizer/express-prom-bundle.png)](https://travis-ci.org/jochen-schweizer/express-prom-bundle) +[![build status](https://travis-ci.org/jochen-schweizer/express-prom-bundle.png)](https://travis-ci.org/jochen-schweizer/express-prom-bundle) [![Coverage Status](https://coveralls.io/repos/github/jochen-schweizer/express-prom-bundle/badge.svg?branch=master)](https://coveralls.io/github/jochen-schweizer/express-prom-bundle?branch=master) [![license](https://img.shields.io/github/license/mashape/apistatus.svg?maxAge=2592000)](https://www.tldrlegal.com/l/mit) # express prometheus bundle @@ -20,18 +20,26 @@ npm install express-prom-bundle ## Usage -```javascript -const - promBundle = require("express-prom-bundle"), - middleware = promBundle({/* options */ }); +You **MUST** call `app.use(metricsMiddleware)` before the `use`-ing your middleware, +otherwise those won't count in `http_request_seconds` histogram -app.use(middleware); +```javascript +const promBundle = require("express-prom-bundle"), +const metricsMiddleware = promBundle({/* options */ }); + +app.use(metricsMiddleware); +app.use(/* your middleware */); +app.listen(3000); ``` +* call your endpoints +* see your metrics here: [http://localhost:3000/metrics]() + ## Options * **prefix**: prefix added to every metric name - * **whitelist**, **blacklist**: array of strings or regexp. These which metrics to include/exclude + * **whitelist**, **blacklist**: array of strings or regexp specifying which metrics to include/exclude + * **buckets**: buckets used for `http_request_seconds` histogram ## Example @@ -45,8 +53,7 @@ const express = require("express"), promBundle = require("express-prom-bundle"); app.use(promBundle({ - prefix: "demo_app:something", - blacklist: ["up"] + prefix: "demo_app:something" })); app.get("/hello", (req, res) => res.send("ok")); @@ -54,6 +61,8 @@ app.get("/hello", (req, res) => res.send("ok")); app.listen(3000); ``` +See an [advanced example on github](https://github.com/jochen-schweizer/express-prom-bundle/blob/master/advanced-example.js) + ## License MIT \ No newline at end of file diff --git a/advanced-example.js b/advanced-example.js index 8f50bf5..979780e 100644 --- a/advanced-example.js +++ b/advanced-example.js @@ -1,28 +1,38 @@ "use strict"; -const express = require("express"), - app = express(), - promBundle = require("express-prom-bundle"), - promClient = require("prom-client"); +const express = require("express"); +const app = express(); +const promBundle = require("express-prom-bundle"); const bundle = promBundle({ prefix: "demo_app:something:", - blacklist: [/up/] + blacklist: [/up/], + buckets: [0.1, 0.4, 0.7] }); app.use(bundle); -let c1 = new bundle.promClient.Counter("c1", "c1 help"); +// native prom-client metric (no prefix) +const c1 = new bundle.promClient.Counter("c1", "c1 help"); c1.inc(10); -let c2 = bundle.factory.newCounter("c2", "c2 help"); +// create metric using factory (w/ prefix) +const c2 = bundle.factory.newCounter("c2", "c2 help"); c2.inc(20); app.get("/foo", (req, res) => { setTimeout(() => { - res.send("foo response"); + res.send("foo response\n"); }, 500); }); -app.get("/bar", (req, res) => res.send("bar response")); +app.get("/bar", (req, res) => res.send("bar response\n")); -app.listen(3001); +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 diff --git a/package.json b/package.json index 2ea8633..001adaa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "express-prom-bundle", - "version": "1.1.3", + "version": "1.1.4", "description": "express middleware with popular prometheus metrics in one bundle", "main": "src/index.js", "keywords": [ @@ -16,16 +16,18 @@ "license": "MIT", "dependencies": { "on-finished": "^2.3.0", - "prom-client": "^3.4.0" + "prom-client": "^3.4.6" }, "devDependencies": { - "eslint": "^2.8.0", + "coveralls": "^2.11.12", + "eslint": "^2.13.1", "express": "^4.13.4", - "jasme": "^4.1.1", + "istanbul": "^0.4.4", + "jasme": "^4.1.2", "supertest": "^1.2.0" }, "repository": { - "type" : "git", - "url" : "https://github.com/jochen-schweizer/express-prom-bundle.git" + "type": "git", + "url": "https://github.com/jochen-schweizer/express-prom-bundle.git" } -} \ No newline at end of file +} diff --git a/spec/PromFactorySpec.js b/spec/PromFactorySpec.js new file mode 100644 index 0000000..326403e --- /dev/null +++ b/spec/PromFactorySpec.js @@ -0,0 +1,57 @@ +"use strict"; +/* eslint-env jasmine */ +const PromFactory = require("../src/PromFactory"); + +describe("PromFactory", () => { + let factory; + beforeEach(() => { + factory = new PromFactory(); + }); + it("creates Counter", () => { + const metric = factory.newCounter( + "test1", + "help for test1", + ["label1", "label2"] + ); + expect(metric.name).toBe("test1"); + 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", + "help for test2", + ["label1", "label2"] + ); + expect(metric.name).toBe("test2"); + expect(metric.help).toBe("help for test2"); + expect(metric.labelNames).toEqual(["label1", "label2"]); + }); + it("creates Histogram with labels", () => { + const metric = factory.newHistogram( + "test3", + "help for test3", + ["label1", "label2"], + {buckets: [1, 2, 3]} + ); + expect(metric.name).toBe("test3"); + expect(metric.help).toBe("help for test3"); + expect(metric.labelNames).toEqual(["label1", "label2"]); + expect(metric.bucketValues).toEqual({"1": 0, "2": 0, "3": 0}); + }); + it("creates Summary without labels", () => { + const metric = factory.newSummary( + "test4", + "help for test4", + {percentiles: [0.1, 0.5]} + ); + expect(metric.name).toBe("test4"); + expect(metric.help).toBe("help for test4"); + expect(metric.percentiles).toEqual([0.1, 0.5]); + }); + +}); diff --git a/spec/indexSpec.js b/spec/indexSpec.js index 3ade247..c791b6b 100644 --- a/spec/indexSpec.js +++ b/spec/indexSpec.js @@ -2,23 +2,80 @@ /* eslint-env jasmine */ let express = require("express"), - request = require("supertest"), + supertest = require("supertest"), bundle = require("../"); describe("index", () => { - const app = express(); - - app.use(bundle({ - prefix: "hello:" - })); - it("/metrics returns up=1", done => { - request(app) + const app = express(); + app.use(bundle({ + prefix: "hello:", + whitelist: ["up"] + })); + 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); + expect(res.text).toMatch(/hello:up\s1/); + done(); + }); + }); + }); + it("metrics can be filtered using exect match", () => { + const instance = bundle({blacklist: ["up"]}); + expect(instance.metrics.up).not.toBeDefined(); + expect(instance.metrics.nodejs_memory_heap_total_bytes).toBeDefined(); + }); + it("metrics can be filtered using regex", () => { + const instance = bundle({blacklist: [/memory/]}); + expect(instance.metrics.up).toBeDefined(); + expect(instance.metrics.nodejs_memory_heap_total_bytes).not.toBeDefined(); + }); + it("metrics can be whitelisted", () => { + const instance = bundle({whitelist: [/^up$/]}); + expect(instance.metrics.up).toBeDefined(); + expect(instance.metrics.nodejs_memory_heap_total_bytes).not.toBeDefined(); + expect(instance.metrics.http_request_seconds).not.toBeDefined(); + }); + it("throws on both white and blacklist", () => { + expect(() => { + bundle({whitelist: [/up/], blacklist: [/up/]}); + }).toThrow(); + }); + it("returns error 500 on incorrect middleware usage", done => { + const app = express(); + app.use(bundle); + supertest(app) .get("/metrics") .end((err, res) => { - expect(res.status).toBe(200); - expect(res.text).toMatch(/hello:up\s1/); + expect(res.status).toBe(500); done(); }); }); + it("http latency gets counted", done => { + const app = express(); + const instance = bundle(); + app.use(instance); + app.use("/test", (req, res) => res.send("it worked")); + const agent = supertest(app); + agent + .get("/test") + .end(() => { + const metricHashMap = instance.metrics.http_request_seconds.hashMap; + expect(metricHashMap["status_code:200"]).toBeDefined(); + const labeled = metricHashMap["status_code:200"]; + expect(labeled.count).toBe(1); + + agent + .get("/metrics") + .end((err, res) => { + expect(res.status).toBe(200); + done(); + }); + }); + }); }); \ No newline at end of file diff --git a/src/PromFactory.js b/src/PromFactory.js index b598619..ce71134 100644 --- a/src/PromFactory.js +++ b/src/PromFactory.js @@ -21,28 +21,33 @@ module.exports = class { return (this.opts.prefix || "") + name; } - makeMetric(TheClass, name, description, param) { + makeMetric(TheClass, args) { + // convert pseudo-array + const applyParams = Array.prototype.slice.call(args); + const name = applyParams[0]; this.checkDuplicate(name); const realName = this.makeRealName(name); - this.metrics[name] = new TheClass( - realName, description, param - ); + 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]; } - newCounter(name, description, labels) { - return this.makeMetric(this.promClient.Counter, name, description, labels); + newCounter() { + return this.makeMetric(this.promClient.Counter, arguments); } - newGauge(name, description, labels) { - return this.makeMetric(this.promClient.Gauge, name, description, labels); + newGauge() { + return this.makeMetric(this.promClient.Gauge, arguments); } - newHistogram(name, description, options) { - return this.makeMetric(this.promClient.Histogram, name, description, options); + newHistogram() { + return this.makeMetric(this.promClient.Histogram, arguments); } - newSummary(name, description, options) { - return this.makeMetric(this.promClient.Histogram, name, description, options); + 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 b73d6b7..26f6328 100644 --- a/src/index.js +++ b/src/index.js @@ -20,7 +20,7 @@ function filterArrayByRegExps(array, regexps) { } function prepareMetricNames(opts, metricTemplates) { - let names = Object.keys(metricTemplates); + const names = Object.keys(metricTemplates); if (opts.whitelist) { if (opts.blacklist) { throw new Error("you cannot have whitelist and blacklist at the same time"); @@ -35,16 +35,17 @@ function prepareMetricNames(opts, metricTemplates) { } function main(opts) { + opts = opts === undefined ? {} : opts; if (arguments[2] && arguments[1] && arguments[1].send) { arguments[1].status(500) .send("

500 Error

\n" - + "

Unexapected 3d param.\n" + + "

Unexapected 3d param in express-prom-bundle.\n" + "

Did you just put express-prom-bundle into app.use " + "without calling it as a function first?"); return; } - let factory = new PromFactory(opts); + const factory = new PromFactory(opts); const metricTemplates = { "up": () => factory.newGauge( @@ -63,11 +64,11 @@ function main(opts) { const metric = factory.newHistogram( "http_request_seconds", "number of http responses labeled with status code", + ["status_code"], { - buckets: [0.003, 0.03, 0.1, 0.3, 1.5, 10] + buckets: opts.buckets || [0.003, 0.03, 0.1, 0.3, 1.5, 10] } ); - metric.labelNames = ["status_code"]; return metric; } }; @@ -85,14 +86,12 @@ function main(opts) { metrics.up.set(1); } - let middleware = function (req, res, next) { + const middleware = function (req, res, next) { let timer, labels; - if (metrics["http_request_seconds"]) { labels = {"status_code": 0}; timer = metrics["http_request_seconds"].startTimer(labels); } - if (req.path == "/metrics") { let memoryUsage = process.memoryUsage(); if (metrics["nodejs_memory_heap_total_bytes"]) { @@ -109,10 +108,8 @@ function main(opts) { if (timer) { onFinished(res, () => { - if (res.statusCode) { - labels["status_code"] = res.statusCode; - timer(); - } + labels["status_code"] = res.statusCode; + timer(); }); }