#21 node-fast should support fixed buckets and artedi v2

Reviewed by: Kody A Kantor <kody@kkantor.com>
Approved by: Kody A Kantor <kody@kkantor.com>

CHANGES.md

@@ -4,6 +4,21 @@
 
 None yet.
 
+## v2.6.0
+
+* `fast_client_request_time_ms` became `fast_client_request_time_seconds` and is
+  now measured in seconds rather than milliseconds.
+* `fast_request_time_ms` became `fast_server_request_time_seconds` and is
+  now measured in seconds rather than milliseconds.
+* Add support for artedi v2 which requires histograms to use fixed buckets.
+  When the collector passed is an artedi v2 collector, default buckets are
+  generated via artedi.logLinearBuckets(10, -1, 3, 5). See: joyent/node-artedi#17
+  for more details. Data generated with artedi v1 collectors are likely
+  invalid (and have been with previous releases as well). Dashboards/queries for
+  services which pass v2 collectors to node-fast, should restrict results using
+  the `{buckets_version="1"}` label when performing Prometheus queries on this
+  data.
+
 ## v2.5.0
 
 * #17 node-fast could track client metrics

lib/fast_client.js

@@ -26,6 +26,19 @@ var mod_subr = require('./subr');
 
 exports.FastClient = FastClient;
 
+/*
+ * Generated via artedi.logLinearBuckets(10, -4, 1, 5). Chosen only because it
+ * covers the range of observed requests.
+ */
+var DEFAULT_BUCKETS = [
+	0.0002, 0.0004, 0.0006, 0.0008, 0.001,
+	0.002, 0.004, 0.006, 0.008, 0.01,
+	0.02, 0.04, 0.06, 0.08, 0.1,
+	0.2, 0.4, 0.6, 0.8, 1,
+	2, 4, 6, 8, 10,
+	20, 40, 60, 80, 100
+];
+
 /*
  * There's one DTrace provider for all clients using this copy of this module.
  */
@@ -70,6 +83,7 @@ fastNclients = 0;
 function FastClient(args)
 {
 	var self = this;
+	var fixed_buckets = false;
 
 	mod_assertplus.object(args, 'args');
 	mod_assertplus.optionalObject(args.collector, 'args.collector');
@@ -109,13 +123,20 @@ function FastClient(args)
 	this.fc_transport_ended = false; /* transport detached us */
 
 	if (this.fc_collector) {
+		if (this.fc_collector.FIXED_BUCKETS === true) {
+			fixed_buckets = true;
+		}
 		this.fc_request_counter = this.fc_collector.counter({
 			name: 'fast_client_requests_completed',
 			help: 'count of fast client requests completed'
 		});
 		this.fc_latency_histogram = this.fc_collector.histogram({
-			name: 'fast_client_request_time_ms',
-			help: 'end-to-end fast client request duration'
+			name: 'fast_client_request_time_seconds',
+			help: 'end-to-end fast client request duration',
+			buckets: (fixed_buckets === true) ?
+			    DEFAULT_BUCKETS : undefined,
+			labels: (fixed_buckets === true) ?
+			    { buckets_version: '1' } : undefined
 		});
 	}
 
@@ -677,7 +698,7 @@ FastClient.prototype.requestComplete = function (request)
 		/* Track the requested RPC methoad. */
 		labels = { 'rpcMethod': request.frq_rpcmethod };
 		this.fc_request_counter.increment(labels);
-		this.fc_latency_histogram.observe(latency, labels);
+		this.fc_latency_histogram.observe((latency / 1000), labels);
 	}
 
 	this.fc_recentrpc.push(request);

lib/fast_server.js

@@ -131,6 +131,19 @@ var mod_subr = require('./subr');
 
 exports.FastServer = FastServer;
 
+/*
+ * Generated via artedi.logLinearBuckets(10, -4, 1, 5). Chosen only because it
+ * covers the range of observed requests.
+ */
+var DEFAULT_BUCKETS = [
+	0.0002, 0.0004, 0.0006, 0.0008, 0.001,
+	0.002, 0.004, 0.006, 0.008, 0.01,
+	0.02, 0.04, 0.06, 0.08, 0.1,
+	0.2, 0.4, 0.6, 0.8, 1,
+	2, 4, 6, 8, 10,
+	20, 40, 60, 80, 100
+];
+
 /*
  * This maximum is chosen pretty arbitrarily.
  */
@@ -167,6 +180,7 @@ fastNservers = 0;
 function FastServer(args)
 {
 	var self = this;
+	var fixed_buckets = false;
 
 	mod_assertplus.object(args, 'args');
 	mod_assertplus.object(args.log, 'args.log');
@@ -213,13 +227,20 @@ function FastServer(args)
 	this.fs_nrequests_failed = 0;		/* count of reqs failed */
 
 	if (this.fs_collector) {
+		if (this.fc_collector.FIXED_BUCKETS === true) {
+			fixed_buckets = true;
+		}
 		this.fs_request_counter = this.fs_collector.counter({
 			name: 'fast_requests_completed',
 			help: 'count of fast requests completed'
 		});
 		this.fs_latency_histogram = this.fs_collector.histogram({
-			name: 'fast_request_time_ms',
-			help: 'total time to process fast requests'
+			name: 'fast_server_request_time_seconds',
+			help: 'total time to process fast requests',
+			buckets: (fixed_buckets === true) ?
+			    DEFAULT_BUCKETS : undefined,
+			labels: (fixed_buckets === true) ?
+			    { buckets_version: '1' } : undefined
 		});
 	}
 
@@ -910,7 +931,7 @@ FastServer.prototype.requestCleanup = function (request)
 		/* Track the requested RPC methoad. */
 		labels = { 'rpcMethod': request.fsr_rpcmethod };
 		this.fs_request_counter.increment(labels);
-		this.fs_latency_histogram.observe(latency, labels);
+		this.fs_latency_histogram.observe((latency / 1000), labels);
 	}
 
 	if (request.fsr_handler !== null) {

package.json

@@ -1,7 +1,7 @@
 {
 	"name": "fast",
 	"description": "streaming JSON RPC over TCP",
-	"version": "2.5.0",
+	"version": "2.6.0",
 	"main": "./lib/fast.js",
 	"repository": {
 		"type": "git",