Skip to content

Commit

Permalink
feat(sdk-metrics): apply binary search in histogram (#3539)
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas authored Jan 31, 2023
1 parent 279458e commit 2a76d88
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

* feat(sdk-metrics): apply binary search in histogram recording [#3539](https://github.com/open-telemetry/opentelemetry-js/pull/3539) @legendecas

### :bug: (Bug Fix)

### :books: (Refine Doc)
Expand Down
14 changes: 4 additions & 10 deletions packages/sdk-metrics/src/aggregator/Histogram.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
import { DataPointType, HistogramMetricData } from '../export/MetricData';
import { HrTime } from '@opentelemetry/api';
import { InstrumentDescriptor, InstrumentType } from '../InstrumentDescriptor';
import { Maybe } from '../utils';
import { binarySearchLB, Maybe } from '../utils';
import { AggregationTemporality } from '../export/AggregationTemporality';

/**
Expand Down Expand Up @@ -77,14 +77,8 @@ export class HistogramAccumulation implements Accumulation {
this._current.hasMinMax = true;
}

for (let i = 0; i < this._boundaries.length; i++) {
if (value < this._boundaries[i]) {
this._current.buckets.counts[i] += 1;
return;
}
}
// value is above all observed boundaries
this._current.buckets.counts[this._boundaries.length] += 1;
const idx = binarySearchLB(this._boundaries, value);
this._current.buckets.counts[idx + 1] += 1;
}

setStartTime(startTime: HrTime): void {
Expand All @@ -104,7 +98,7 @@ export class HistogramAggregator implements Aggregator<HistogramAccumulation> {
public kind: AggregatorKind.HISTOGRAM = AggregatorKind.HISTOGRAM;

/**
* @param _boundaries upper bounds of recorded values.
* @param _boundaries sorted upper bounds of recorded values.
* @param _recordMinMax If set to true, min and max will be recorded. Otherwise, min and max will not be recorded.
*/
constructor(
Expand Down
27 changes: 27 additions & 0 deletions packages/sdk-metrics/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,30 @@ export function setEquals(lhs: Set<unknown>, rhs: Set<unknown>): boolean {
}
return true;
}

/**
* Binary search the sorted array to the find lower bound for the value.
* @param arr
* @param value
* @returns
*/
export function binarySearchLB(arr: number[], value: number): number {
let lo = 0;
let hi = arr.length - 1;

while (hi - lo > 1) {
const mid = Math.trunc((hi + lo) / 2);
if (arr[mid] <= value) {
lo = mid;
} else {
hi = mid - 1;
}
}

if (arr[hi] <= value) {
return hi;
} else if (arr[lo] <= value) {
return lo;
}
return -1;
}
31 changes: 30 additions & 1 deletion packages/sdk-metrics/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@

import * as sinon from 'sinon';
import * as assert from 'assert';
import { callWithTimeout, hashAttributes, TimeoutError } from '../src/utils';
import {
binarySearchLB,
callWithTimeout,
hashAttributes,
TimeoutError,
} from '../src/utils';
import { assertRejects } from './test-utils';
import { MetricAttributes } from '@opentelemetry/api';

Expand Down Expand Up @@ -60,4 +65,28 @@ describe('utils', () => {
}
});
});

describe('binarySearchLB', () => {
const tests = [
/** [ arr, value, expected lb idx ] */
[[0, 10, 100, 1000], -1, -1],
[[0, 10, 100, 1000], 0, 0],
[[0, 10, 100, 1000], 1, 0],
[[0, 10, 100, 1000], 10, 1],
[[0, 10, 100, 1000], 1000, 3],
[[0, 10, 100, 1000], 1001, 3],

[[0, 10, 100, 1000, 10_000], -1, -1],
[[0, 10, 100, 1000, 10_000], 0, 0],
[[0, 10, 100, 1000, 10_000], 10, 1],
[[0, 10, 100, 1000, 10_000], 1001, 3],
[[0, 10, 100, 1000, 10_000], 10_001, 4],
] as [number[], number, number][];

for (const [idx, test] of tests.entries()) {
it(`test idx(${idx}): find lb of ${test[1]} in [${test[0]}]`, () => {
assert.strictEqual(binarySearchLB(test[0], test[1]), test[2]);
});
}
});
});

0 comments on commit 2a76d88

Please sign in to comment.