diff --git a/CHANGELOG.md b/CHANGELOG.md index 8623279f76..33c3c17e8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/packages/sdk-metrics/src/aggregator/Histogram.ts b/packages/sdk-metrics/src/aggregator/Histogram.ts index a916e18248..f20784b054 100644 --- a/packages/sdk-metrics/src/aggregator/Histogram.ts +++ b/packages/sdk-metrics/src/aggregator/Histogram.ts @@ -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'; /** @@ -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 { @@ -104,7 +98,7 @@ export class HistogramAggregator implements Aggregator { 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( diff --git a/packages/sdk-metrics/src/utils.ts b/packages/sdk-metrics/src/utils.ts index c2bc440849..835de92fe1 100644 --- a/packages/sdk-metrics/src/utils.ts +++ b/packages/sdk-metrics/src/utils.ts @@ -163,3 +163,30 @@ export function setEquals(lhs: Set, rhs: Set): 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; +} diff --git a/packages/sdk-metrics/test/utils.test.ts b/packages/sdk-metrics/test/utils.test.ts index 04633a14ba..16ec0a2b3a 100644 --- a/packages/sdk-metrics/test/utils.test.ts +++ b/packages/sdk-metrics/test/utils.test.ts @@ -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'; @@ -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]); + }); + } + }); });