Named rule support; v1.4.0

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Divvy Changelog
 
+## v1.4.0 (2019-05-08)
+
+* Feature: Support named rules.
+
 ## v1.3.0 (2018-06-06)
 
 * Feature: Add support for JSON-based configuration.

README.md

@@ -25,7 +25,7 @@ Divvy is a quota / rate limiter service, implemented in NodeJS and backed by Red
 With Divvy you can express policies like:
 
 * Limit `GET /printer/supplies` to 100 requests per minute globally.
-* Limit `POST /printer/print` to 
+* Limit `POST /printer/print` to
     * 60 requests/minute for authorized users.
     * 5 requests/minute for everyone else, by IP address.
 * No limit for `GET /printer/status`
@@ -101,7 +101,7 @@ Let's look at what each field means:
 * **Status**: `OK`: The server understood the command, yay!
 * **Allowed**: `true`: We have enough quota for this operation.
 * **Credit remaining**: `999`: We have a bunch of quota left, too.
-* **Next reset (seconds)**: `60`: Our quota will be set back to `1000` in 60 seconds. 
+* **Next reset (seconds)**: `60`: Our quota will be set back to `1000` in 60 seconds.
 
 Let's try a few more and watch our quota decrease:
 
@@ -219,6 +219,7 @@ Bucket order is signficant: Quota is determined for a `HIT` request by finding t
 
 The following optional fields are also supported:
 
+* `label`: A short, slug-like name for the rule. If set, Prometheus metrics for hits matching the rule will be labeled with `rule_label` as this value. Labels have no effect on statsd metrics.
 * `comment`: A diagnostic comment, printed when running server with `DEBUG=divvy`.
 * `actorField`: Described in _"Actors and multi-tenancy"_.
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@button/divvy",
-  "version": "1.3.0",
+  "version": "1.4.0",
   "description": "Redis-backed rate limit service.",
   "main": "index.js",
   "directories": {

src/config.js

@@ -16,13 +16,19 @@ const Utils = require('./utils');
  */
 const REGEX_ESCAPE_CHARACTERS = /[-[\]{}()+?.,\\^$|#]/g;
 
+/**
+ * Allowed pattern for the "label" field of a rule.
+ */
+const RULE_LABEL_REGEX = /^[a-zA-Z0-9_-]{1,255}$/;
+
 function isGlobValue(v) {
   return v.endsWith('*');
 }
 
 class Config {
   constructor() {
     this.rules = [];
+    this.ruleLabels = new Set();
   }
 
   /**
@@ -52,6 +58,7 @@ class Config {
         rule.creditLimit,
         rule.resetSeconds,
         rule.actorField,
+        rule.label,
         rule.comment);
     });
 
@@ -87,8 +94,9 @@ class Config {
       // Optional fields.
       const actorField = rulegroupConfig.actorField || '';
       const comment = rulegroupConfig.comment || '';
+      const label = rulegroupConfig.label || '';
 
-      config.addRule(operation, creditLimit, resetSeconds, actorField, comment);
+      config.addRule(operation, creditLimit, resetSeconds, actorField, label, comment);
     }
 
     return config;
@@ -115,9 +123,10 @@ class Config {
    * @param {number} creditLimit  Number of operations to permit every `resetSeconds`
    * @param {number} resetSeconds Credit renewal interval.
    * @param {string} actorField   Name of the actor field (optional).
+   * @param {string} label        Optional name for this rule.
    * @param {string} comment      Optional diagnostic name for this rule.
    */
-  addRule(operation, creditLimit, resetSeconds, actorField, comment) {
+  addRule(operation, creditLimit, resetSeconds, actorField, label, comment) {
     const foundRule = this.findRule(operation);
 
     if (foundRule !== null) {
@@ -133,11 +142,21 @@ class Config {
       throw new Error(`Invalid resetSeconds for operation=${operation} (${resetSeconds})`);
     }
 
+    if (label) {
+      if (!RULE_LABEL_REGEX.test(label)) {
+        throw new Error(`Invalid rule label "${label}"; must match ${RULE_LABEL_REGEX}`);
+      } else if (this.ruleLabels.has(label)) {
+        throw new Error(`A rule with label "${label}" already exists; labels must be unique.`);
+      }
+      this.ruleLabels.add(label);
+    }
+
     const rule = {
       operation,
       creditLimit,
       resetSeconds,
       actorField,
+      label: label || null,
       comment: comment || null,
     };
     this.rules.push(rule);

src/instrumenter.js

@@ -57,7 +57,7 @@ class Instrumenter {
     this.hitCounter = new PrometheusClient.Counter({
       name: 'divvy_hits_total',
       help: 'Counter of total HITs to Divvy.',
-      labelNames: ['status', 'type'],
+      labelNames: ['status', 'type', 'rule_label'],
     });
 
     this.errorCounter = new PrometheusClient.Counter({
@@ -92,11 +92,12 @@ class Instrumenter {
    * Record a HIT operation.
    * @param {string} status The status of the hit, either "accepted" or "rejected".
    * @param {string} type   The match type, either "rule", "default", or "none".
+   * @param {string} label   The matching rule label, or null.
    */
-  countHit(status, type) {
+  countHit(status, type, label) {
     this.statsd.increment(`hit.${status}`);
     this.statsd.increment(`hit.${status}.${type}`);
-    this.hitCounter.labels(status, type).inc();
+    this.hitCounter.labels(status, type, label || '').inc();
   }
 
   /**

src/server.js

@@ -133,7 +133,8 @@ class Server extends EventEmitter {
       this.instrumenter.timeHit(startDate);
       const result = status.isAllowed ? 'accepted' : 'rejected';
       const matchType = Server.getMatchType(rule);
-      this.instrumenter.countHit(result, matchType);
+      const ruleLabel = (rule && rule.label) ? rule.label : '';
+      this.instrumenter.countHit(result, matchType, ruleLabel);
     }).catch((err) => {
       this.sendError(conn, `Server error: ${err}`);
     });

tests/config-test.js

@@ -29,6 +29,7 @@ describe('src/config', function () {
           creditLimit: 100,
           resetSeconds: 60,
           actorField: 'ip',
+          label: null,
           comment: '100 rpm for /ping for authenticated users, by ip',
         }, rule);
 
@@ -47,6 +48,7 @@ describe('src/config', function () {
           creditLimit: 10,
           resetSeconds: 60,
           actorField: 'ip',
+          label: 'get-ping-by-ip',
           comment: '10 rpm for /ping for non-authenticated users, by ip',
         }, rule);
 
@@ -64,6 +66,7 @@ describe('src/config', function () {
           creditLimit: 5,
           resetSeconds: 60,
           actorField: 'ip',
+          label: 'post-by-ip',
           comment: '5 rpm for any POST, by ip',
         }, rule);
 
@@ -75,6 +78,7 @@ describe('src/config', function () {
           creditLimit: 1,
           resetSeconds: 60,
           actorField: '',
+          label: null,
           comment: 'Default quota',
         }, rule);
       });
@@ -102,6 +106,22 @@ describe('src/config', function () {
         }, /Invalid creditLimit/);
       });
 
+      it('with a rule containing an invalid label', function () {
+        const config = new Config();
+        config.addRule({ service: 'myservice', method: 'GET' }, 0, 60, null, 'nice-rule');
+        assert.throws(() => {
+          config.addRule({ service: 'myservice', method: 'POST' }, 0, 60, null, 'this is fine');
+        }, /Invalid rule label "this is fine"/);
+      });
+
+      it('with a rule containing a duplicate label', function () {
+        const config = new Config();
+        config.addRule({ service: 'myservice', method: 'GET' }, 0, 60, null, 'nice-rule');
+        assert.throws(() => {
+          config.addRule({ service: 'myservice', method: 'POST' }, 0, 60, null, 'nice-rule');
+        }, /A rule with label "nice-rule" already exists/);
+      });
+
       it('with a rule where resetSeconds < 1', function () {
         const config = new Config();
         config.addRule({ service: 'myservice', method: 'GET' }, 20, 1);
@@ -119,8 +139,8 @@ describe('src/config', function () {
       it('handles simple glob keys', function () {
         const config = new Config();
 
-        config.addRule({ service: 'my*', method: 'GET' }, 100, 60, 'actor', 'a');
-        config.addRule({ service: 'your*', method: 'GET' }, 200, 40, 'jim', 'b');
+        config.addRule({ service: 'my*', method: 'GET' }, 100, 60, 'actor', 'rule-a', 'a');
+        config.addRule({ service: 'your*', method: 'GET' }, 200, 40, 'jim', 'rule-b', 'b');
 
         const rule = config.findRule({ service: 'myget', method: 'GET' });
         assert.deepEqual({
@@ -131,6 +151,7 @@ describe('src/config', function () {
           creditLimit: 100,
           resetSeconds: 60,
           actorField: 'actor',
+          label: 'rule-a',
           comment: 'a',
         }, rule);
 
@@ -143,6 +164,7 @@ describe('src/config', function () {
           creditLimit: 200,
           resetSeconds: 40,
           actorField: 'jim',
+          label: 'rule-b',
           comment: 'b',
         }, other);
       });
@@ -164,6 +186,7 @@ describe('src/config', function () {
           creditLimit: 1,
           resetSeconds: 60,
           actorField: 'ip',
+          label: null,
           comment: '1 rpm for POST /account*, by ip',
         }, rule);
 
@@ -181,6 +204,7 @@ describe('src/config', function () {
           creditLimit: 5,
           resetSeconds: 60,
           actorField: 'ip',
+          label: 'post-by-ip',
           comment: '5 rpm for any POST, by ip',
         }, rule);
       });

tests/instrumenter-test.js

@@ -62,8 +62,8 @@ describe('src/instrumenter', function () {
     sinon.assert.notCalled(this.instrumenter.statsd.increment);
     assert.deepEqual(this.instrumenter.hitCounter.hashMap, {});
 
-    this.instrumenter.countHit('accepted', 'rule');
-    this.instrumenter.countHit('accepted', 'rule');
+    this.instrumenter.countHit('accepted', 'rule', 'a');
+    this.instrumenter.countHit('accepted', 'rule', 'b');
     this.instrumenter.countHit('accepted', 'none');
     this.instrumenter.countHit('accepted', 'default');
     this.instrumenter.countHit('rejected', 'none');
@@ -76,9 +76,15 @@ describe('src/instrumenter', function () {
     sinon.assert.calledWith(this.instrumenter.statsd.increment, 'hit.accepted.default');
     sinon.assert.calledWith(this.instrumenter.statsd.increment, 'hit.rejected.none');
 
-    assert.equal(this.instrumenter.hitCounter.hashMap['status:accepted,type:rule'].value, 2);
-    assert.equal(this.instrumenter.hitCounter.hashMap['status:accepted,type:none'].value, 1);
-    assert.equal(this.instrumenter.hitCounter.hashMap['status:accepted,type:default'].value, 1);
-    assert.equal(this.instrumenter.hitCounter.hashMap['status:rejected,type:none'].value, 1);
+    assert.equal(this.instrumenter.hitCounter.hashMap[
+      'rule_label:a,status:accepted,type:rule'].value, 1);
+    assert.equal(this.instrumenter.hitCounter.hashMap[
+      'rule_label:b,status:accepted,type:rule'].value, 1);
+    assert.equal(this.instrumenter.hitCounter.hashMap[
+      'rule_label:,status:accepted,type:none'].value, 1);
+    assert.equal(
+      this.instrumenter.hitCounter.hashMap['rule_label:,status:accepted,type:default'].value, 1);
+    assert.equal(this.instrumenter.hitCounter.hashMap[
+      'rule_label:,status:rejected,type:none'].value, 1);
   });
 });

tests/server-test.js

@@ -101,7 +101,7 @@ describe('src/server', function () {
         });
 
         sinon.assert.callCount(instrumenter.countHit, 1);
-        sinon.assert.calledWith(instrumenter.countHit, 'accepted', 'rule');
+        sinon.assert.calledWith(instrumenter.countHit, 'accepted', 'rule', '');
 
         sinon.assert.callCount(instrumenter.timeHit, 1);
         // Since we've installed sinon's fake timers we can safely compare to new Date()
@@ -140,7 +140,7 @@ describe('src/server', function () {
         });
 
         sinon.assert.callCount(instrumenter.countHit, 1);
-        sinon.assert.calledWith(instrumenter.countHit, 'accepted', 'rule');
+        sinon.assert.calledWith(instrumenter.countHit, 'accepted', 'rule', 'get-ping-by-ip');
 
         sinon.assert.callCount(instrumenter.timeHit, 1);
         // Since we've installed sinon's fake timers we can safely compare to new Date()
@@ -175,7 +175,7 @@ describe('src/server', function () {
         });
 
         sinon.assert.callCount(instrumenter.countHit, 1);
-        sinon.assert.calledWith(instrumenter.countHit, 'accepted', 'rule');
+        sinon.assert.calledWith(instrumenter.countHit, 'accepted', 'rule', '');
 
         sinon.assert.callCount(instrumenter.timeHit, 1);
         // Since we've installed sinon's fake timers we can safely compare to new Date()

tests/test-config.ini

@@ -8,6 +8,7 @@ comment = '100 rpm for /ping for authenticated users, by ip'
 creditLimit = 10
 resetSeconds = 60
 actorField = ip
+label = 'get-ping-by-ip'
 comment = '10 rpm for /ping for non-authenticated users, by ip'
 
 [method=POST ip=* path=/account* isAuthenticated=true]
@@ -20,9 +21,11 @@ comment = '1 rpm for POST /account*, by ip'
 creditLimit = 5
 resetSeconds = 60
 actorField = ip
+label = 'post-by-ip'
 comment = '5 rpm for any POST, by ip'
 
 [default]
 creditLimit = 1
 resetSeconds = 60
-comment = 'Default quota'
+label = null
+comment = 'Default quota'

tests/test-config.json

@@ -21,6 +21,7 @@
       "creditLimit": 10,
       "resetSeconds": 60,
       "actorField": "ip",
+      "label": "get-ping-by-ip",
       "comment": "10 rpm for /ping for non-authenticated users, by ip"
     },
     {
@@ -43,6 +44,7 @@
       "creditLimit": 5,
       "resetSeconds": 60,
       "actorField": "ip",
+      "label": "post-by-ip",
       "comment": "5 rpm for any POST, by ip"
     }
   ],
@@ -51,6 +53,7 @@
     "creditLimit": 1,
     "resetSeconds": 60,
     "actorField": "",
+    "label": null,
     "comment": "Default quota"
   }
 }