fix: always use sum aggregator for sum & count

CHANGELOG.md

@@ -22,6 +22,8 @@ project adheres to [Semantic Versioning](http://semver.org/).
   Pushgateway's typedef were missing promise return type. That was
   causing vscode to think that push/pushAdd and delete didn't promise
   resulting in incorrect behavior.
+
+- fix: always using sum aggregator for sum & count in histogram & summary
 
 ### Added
 

lib/metricAggregators.js

@@ -2,6 +2,56 @@
 
 const { Grouper, hashObject } = require('./util');
 
+/**
+ * Returns the sum of all label values passed
+ * @param {Object} v Label values
+ * @return {Number} sum of values
+ */
+function sum(v) {
+	return v.reduce((p, c) => p + c.value, 0);
+}
+
+/**
+ * Returns the first label value
+ * @param {Object} v Label values
+ * @return {Number} first value.
+ */
+function first(v) {
+	return v[0].value;
+}
+
+/**
+ * @return {undefined} Undefined; omits the values.
+ */
+function omit() {}
+
+/**
+ * Returns the mean of all label values
+ * @param {Object} v Label values
+ * @return {Number} mean of values.
+ */
+function average(v) {
+	return v.reduce((p, c) => p + c.value, 0) / v.length;
+}
+
+/**
+ * Returns the minimum of all label values
+ * @param {Object} v Label values
+ * @return {Number} min of values.
+ */
+function min(v) {
+	return v.reduce((p, c) => Math.min(p, c.value), Infinity);
+}
+
+/**
+ * Returns the maximum of all label values
+ * @param {Object} v Label values
+ * @return {Number} max of values.
+ */
+function max(v) {
+	return v.reduce((p, c) => Math.max(p, c.value), -Infinity);
+}
+
 /**
  * Returns a new function that applies the `aggregatorFn` to the values.
  * @param {Function} aggregatorFn function to apply to values.
@@ -29,12 +79,20 @@ function AggregatorFactory(aggregatorFn) {
 		byLabels.forEach(values => {
 			if (values.length === 0) return;
 			const valObj = {
-				value: aggregatorFn(values),
 				labels: values[0].labels,
 			};
 			if (values[0].metricName) {
 				valObj.metricName = values[0].metricName;
 			}
+			// Should always `sum` if it's `sum` or `count` that's part of histogram & summary
+			if (
+				values[0].metricName === `${result.name}_sum` ||
+				values[0].metricName === `${result.name}_count`
+			) {
+				valObj.value = sum(values);
+			} else {
+				valObj.value = aggregatorFn(values);
+			}
 			// NB: Timestamps are omitted.
 			result.values.push(valObj);
 		});
@@ -48,34 +106,19 @@ exports.AggregatorFactory = AggregatorFactory;
  * Functions that can be used to aggregate metrics from multiple registries.
  */
 exports.aggregators = {
-	/**
-	 * @return The sum of values.
-	 */
-	sum: AggregatorFactory(v => v.reduce((p, c) => p + c.value, 0)),
-	/**
-	 * @return The first value.
-	 */
-	first: AggregatorFactory(v => v[0].value),
-	/**
-	 * @return {undefined} Undefined; omits the metric.
-	 */
-	omit: () => {},
-	/**
-	 * @return The arithmetic mean of the values.
-	 */
-	average: AggregatorFactory(
-		v => v.reduce((p, c) => p + c.value, 0) / v.length,
-	),
+	sum: AggregatorFactory(sum),
+
+	first: AggregatorFactory(first),
+
+	omit,
+
+	average: AggregatorFactory(average),
 	/**
 	 * @return The minimum of the values.
 	 */
-	min: AggregatorFactory(v =>
-		v.reduce((p, c) => Math.min(p, c.value), Infinity),
-	),
+	min: AggregatorFactory(min),
 	/**
 	 * @return The maximum of the values.
 	 */
-	max: AggregatorFactory(v =>
-		v.reduce((p, c) => Math.max(p, c.value), -Infinity),
-	),
+	max: AggregatorFactory(max),
 };

test/clusterTest.js

@@ -45,6 +45,28 @@ describe('AggregatorRegistry', () => {
 		const Registry = require('../lib/cluster');
 		// These mimic the output of `getMetricsAsJSON`.
 		const metricsArr1 = [
+			{
+				name: 'test_summary',
+				help: 'Example of a summary',
+				type: 'summary',
+				values: [
+					{
+						labels: { quantile: 0.5 },
+						value: 4,
+					},
+					{
+						metricName: 'test_summary_sum',
+						labels: {},
+						value: 10,
+					},
+					{
+						metricName: 'test_summary_count',
+						labels: {},
+						value: 3,
+					},
+				],
+				aggregator: 'average',
+			},
 			{
 				name: 'test_histogram',
 				help: 'Example of a histogram',
@@ -102,6 +124,28 @@ describe('AggregatorRegistry', () => {
 			},
 		];
 		const metricsArr2 = [
+			{
+				name: 'test_summary',
+				help: 'Example of a summary',
+				type: 'summary',
+				values: [
+					{
+						labels: { quantile: 0.5 },
+						value: 6,
+					},
+					{
+						metricName: 'test_summary_sum',
+						labels: {},
+						value: 20,
+					},
+					{
+						metricName: 'test_summary_count',
+						labels: {},
+						value: 2,
+					},
+				],
+				aggregator: 'average',
+			},
 			{
 				name: 'test_histogram',
 				help: 'Example of a histogram',
@@ -246,5 +290,31 @@ describe('AggregatorRegistry', () => {
 				aggregator: 'first',
 			});
 		});
+
+		it('uses `aggregate` method defined for test_summary and summation for `count` & `sum`', async () => {
+			const summary = aggregated.getSingleMetric('test_summary').get();
+			expect(summary).toEqual({
+				name: 'test_summary',
+				help: 'Example of a summary',
+				type: 'summary',
+				values: [
+					{
+						labels: { quantile: 0.5 },
+						value: 5,
+					},
+					{
+						labels: {},
+						value: 30,
+						metricName: 'test_summary_sum',
+					},
+					{
+						labels: {},
+						value: 5,
+						metricName: 'test_summary_count',
+					},
+				],
+				aggregator: 'average',
+			});
+		});
 	});
 });