Skip to content

Commit

Permalink
'pruneAgedBuckets' config option for summary (#540)
Browse files Browse the repository at this point in the history
  • Loading branch information
rilpires authored Mar 4, 2023
1 parent cf5173c commit eee3485
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 10 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
22 changes: 15 additions & 7 deletions lib/summary.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions lib/timeWindowQuantiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
30 changes: 29 additions & 1 deletion test/summaryTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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 () => {
Expand Down
2 changes: 1 addition & 1 deletion test/timeWindowQuantilesTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit eee3485

Please sign in to comment.