update deps, fix sumary factory method, add buckets param, extend unit tests, add coverage

.gitignore

@@ -1,2 +1,3 @@
 .npmrc
 node_modules
+coverage

.npmignore

@@ -3,3 +3,4 @@ docker-compose.yml
 spec
 .travis.yml
 .eslintrc
+coverage

.travis.yml

@@ -1,4 +1,5 @@
 language: node_js
 node_js:
+    - "6"
     - "5"
     - "4"

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.

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

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
+You **MUST** call `app.use(metricsMiddleware)` before the `use`-ing your middleware,
-const
+otherwise those won't count in `http_request_seconds` histogram
-    promBundle = require("express-prom-bundle"),
-    middleware = promBundle({/* options */ });
 
-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",
+    prefix: "demo_app:something"
-    blacklist: ["up"]
 }));
 
 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

advanced-example.js

@@ -1,28 +1,38 @@
 "use strict";
 
-const express = require("express"),
+const express = require("express");
-    app = express(),
+const app = express();
-    promBundle = require("express-prom-bundle"),
+const promBundle = require("express-prom-bundle");
-    promClient = require("prom-client");
 
 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
+*/

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",
+    "type": "git",
-    "url" : "https://github.com/jochen-schweizer/express-prom-bundle.git"
+    "url": "https://github.com/jochen-schweizer/express-prom-bundle.git"
   }
-}
+}

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]);
+    });
+
+});

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.status).toBe(500);
-                expect(res.text).toMatch(/hello:up\s1/);
                 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();
+                    });
+            });
+    });
 });

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(
+        applyParams[0] = realName;
-            realName, description, param
+        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) {
+    newCounter() {
-        return this.makeMetric(this.promClient.Counter, name, description, labels);
+        return this.makeMetric(this.promClient.Counter, arguments);
     }
 
-    newGauge(name, description, labels) {
+    newGauge() {
-        return this.makeMetric(this.promClient.Gauge, name, description, labels);
+        return this.makeMetric(this.promClient.Gauge, arguments);
     }
 
-    newHistogram(name, description, options) {
+    newHistogram() {
-        return this.makeMetric(this.promClient.Histogram, name, description, options);
+        return this.makeMetric(this.promClient.Histogram, arguments);
     }
 
-    newSummary(name, description, options) {
+    newSummary() {
-        return this.makeMetric(this.promClient.Histogram, name, description, options);
+        return this.makeMetric(this.promClient.Summary, arguments);
     }
 };

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("<h1>500 Error</h1>\n"
-                + "<p>Unexapected 3d param.\n"
+                + "<p>Unexapected 3d param in express-prom-bundle.\n"
                 + "<p>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;
-                    labels["status_code"] = res.statusCode;
+                timer();
-                    timer();
-                }
             });
         }