From eee34858d2ef4198ff94f56a278d7b81f65e9c63 Mon Sep 17 00:00:00 2001
From: Romeu Pires <rilpires@gmail.com>
Date: Sat, 4 Mar 2023 07:16:30 -0300
Subject: [PATCH] 'pruneAgedBuckets' config option for summary (#540)

---
 CHANGELOG.md                    |  3 +++
 README.md                       |  5 ++++-
 lib/summary.js                  | 22 +++++++++++++++-------
 lib/timeWindowQuantiles.js      |  5 +++++
 test/summaryTest.js             | 30 +++++++++++++++++++++++++++++-
 test/timeWindowQuantilesTest.js |  2 +-
 6 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 95e92e35..38d2765c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,9 @@ project adheres to [Semantic Versioning](http://semver.org/).
   instead of loop of string concatenations.
 - Also use `Array.prototype.map`, and object spread instead of an explicit `for` loop
 - changed: updated the sample output in `example/default-metrics.js`
+- `summary` metrics now has a `pruneAgedBuckets` config parameter
+  to remove entries without any new values in the last `maxAgeSeconds`.
+  Default is `false` (old behavior)
 
 ### Added
 
diff --git a/README.md b/README.md
index 4f4faa30..436eb50f 100644
--- a/README.md
+++ b/README.md
@@ -265,12 +265,15 @@ new client.Summary({
   help: 'metric_help',
   maxAgeSeconds: 600,
   ageBuckets: 5,
+  pruneAgedBuckets: false,
 });
 ```
 
 The `maxAgeSeconds` will tell how old a bucket can be before it is reset and
 `ageBuckets` configures how many buckets we will have in our sliding window for
-the summary.
+the summary. If `pruneAgedBuckets` is `false` (default), the metric value will
+always be present, even when empty (its percentile values will be `0`). Set
+`pruneAgedBuckets` to `true` if you don't want to export it when it is empty.
 
 ##### Examples
 
diff --git a/lib/summary.js b/lib/summary.js
index 9ca3e096..eeb4bf2f 100644
--- a/lib/summary.js
+++ b/lib/summary.js
@@ -52,14 +52,22 @@ class Summary extends Metric {
 			const v = this.collect();
 			if (v instanceof Promise) await v;
 		}
-		const data = Object.values(this.hashMap);
+		const hashKeys = Object.keys(this.hashMap);
 		const values = [];
-		data.forEach(s => {
-			extractSummariesForExport(s, this.percentiles).forEach(v => {
-				values.push(v);
-			});
-			values.push(getSumForExport(s, this));
-			values.push(getCountForExport(s, this));
+
+		hashKeys.forEach(hashKey => {
+			const s = this.hashMap[hashKey];
+			if (s) {
+				if (this.pruneAgedBuckets && s.td.size() === 0) {
+					delete this.hashMap[hashKey];
+				} else {
+					extractSummariesForExport(s, this.percentiles).forEach(v => {
+						values.push(v);
+					});
+					values.push(getSumForExport(s, this));
+					values.push(getCountForExport(s, this));
+				}
+			}
 		});
 
 		return {
diff --git a/lib/timeWindowQuantiles.js b/lib/timeWindowQuantiles.js
index 9e7228dd..176cb5b8 100644
--- a/lib/timeWindowQuantiles.js
+++ b/lib/timeWindowQuantiles.js
@@ -17,6 +17,11 @@ class TimeWindowQuantiles {
 			(maxAgeSeconds * 1000) / ageBuckets || Infinity;
 	}
 
+	size() {
+		const bucket = rotate.call(this);
+		return bucket.size();
+	}
+
 	percentile(quantile) {
 		const bucket = rotate.call(this);
 		return bucket.percentile(quantile);
diff --git a/test/summaryTest.js b/test/summaryTest.js
index 10ad4b0a..29fc2abb 100644
--- a/test/summaryTest.js
+++ b/test/summaryTest.js
@@ -492,7 +492,7 @@ describe('summary', () => {
 			jest.setSystemTime(0);
 		});
 
-		it('should slide when maxAgeSeconds and ageBuckets are set', async () => {
+		it('should present percentiles as zero when maxAgeSeconds and ageBuckets are set but not pruneAgedBuckets', async () => {
 			const localInstance = new Summary({
 				name: 'summary_test',
 				help: 'test',
@@ -516,6 +516,34 @@ describe('summary', () => {
 			const { values } = await localInstance.get();
 			expect(values[0].labels.quantile).toEqual(0.01);
 			expect(values[0].value).toEqual(0);
+			expect(values[7].value).toEqual(100);
+			expect(values[8].value).toEqual(1);
+		});
+
+		it('should prune expired buckets when pruneAgedBuckets are set with maxAgeSeconds and ageBuckets', async () => {
+			const localInstance = new Summary({
+				name: 'summary_test',
+				help: 'test',
+				maxAgeSeconds: 5,
+				ageBuckets: 5,
+				pruneAgedBuckets: true,
+			});
+
+			localInstance.observe(100);
+
+			for (let i = 0; i < 5; i++) {
+				const { values } = await localInstance.get();
+				expect(values[0].labels.quantile).toEqual(0.01);
+				expect(values[0].value).toEqual(100);
+				expect(values[7].metricName).toEqual('summary_test_sum');
+				expect(values[7].value).toEqual(100);
+				expect(values[8].metricName).toEqual('summary_test_count');
+				expect(values[8].value).toEqual(1);
+				jest.advanceTimersByTime(1001);
+			}
+
+			const { values } = await localInstance.get();
+			expect(values.length).toEqual(0);
 		});
 
 		it('should not slide when maxAgeSeconds and ageBuckets are not configured', async () => {
diff --git a/test/timeWindowQuantilesTest.js b/test/timeWindowQuantilesTest.js
index b4c9d6c8..cda7c181 100644
--- a/test/timeWindowQuantilesTest.js
+++ b/test/timeWindowQuantilesTest.js
@@ -41,7 +41,7 @@ describe('timeWindowQuantiles', () => {
 		});
 	});
 
-	describe('rotatation', () => {
+	describe('rotation', () => {
 		it('rotatation interval should be configured', () => {
 			let localInstance = new TimeWindowQuantiles(undefined, undefined);
 			expect(localInstance.durationBetweenRotatesMillis).toEqual(Infinity);