diff --git a/CHANGELOG.md b/CHANGELOG.md index d54ae62368..6e2b1ef382 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/ ### :rocket: (Enhancement) * feat: support TraceState in SamplingResult [#3530](https://github.com/open-telemetry/opentelemetry-js/pull/3530) @raphael-theriault-swi +* feat(sdk-metrics): apply binary search in histogram recording [#3539](https://github.com/open-telemetry/opentelemetry-js/pull/3539) @legendecas ### :bug: (Bug Fix) diff --git a/style.md b/doc/contributing/style.md similarity index 100% rename from style.md rename to doc/contributing/style.md diff --git a/experimental/backwards-compatability/node14/tsconfig.json b/experimental/backwards-compatability/node14/tsconfig.json index 93d86afe21..5151549589 100644 --- a/experimental/backwards-compatability/node14/tsconfig.json +++ b/experimental/backwards-compatability/node14/tsconfig.json @@ -1,5 +1,5 @@ { - "extends": "../../../tsconfig.es5.json", + "extends": "../../../tsconfig.base.es5.json", "compilerOptions": { "rootDir": ".", "outDir": "build" diff --git a/experimental/backwards-compatability/node16/tsconfig.json b/experimental/backwards-compatability/node16/tsconfig.json index 93d86afe21..5151549589 100644 --- a/experimental/backwards-compatability/node16/tsconfig.json +++ b/experimental/backwards-compatability/node16/tsconfig.json @@ -1,5 +1,5 @@ { - "extends": "../../../tsconfig.es5.json", + "extends": "../../../tsconfig.base.es5.json", "compilerOptions": { "rootDir": ".", "outDir": "build" diff --git a/experimental/packages/otlp-grpc-exporter-base/tsconfig.json b/experimental/packages/otlp-grpc-exporter-base/tsconfig.json index 417e1ad468..bbfbbc57b0 100644 --- a/experimental/packages/otlp-grpc-exporter-base/tsconfig.json +++ b/experimental/packages/otlp-grpc-exporter-base/tsconfig.json @@ -6,6 +6,7 @@ }, "include": [ "src/**/*.ts", + "src/generated/*.js", "test/**/*.ts" ], "references": [ 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]); + }); + } + }); }); diff --git a/scripts/update-ts-configs.js b/scripts/update-ts-configs.js index bbe3bd2b35..0242a54668 100644 --- a/scripts/update-ts-configs.js +++ b/scripts/update-ts-configs.js @@ -247,12 +247,7 @@ function resolvePackageMeta(pkgDir) { } function readAndMaybeMergeTsConfig(tsconfigPath, updates) { - let tsconfig; - try { - tsconfig = readJSON(tsconfigPath); - } catch { - return updates; - } + const tsconfig = readJSON(tsconfigPath); updates = mergeTsConfig(tsconfig, updates); return updates; } @@ -284,8 +279,11 @@ function hasEsTargets(pjson) { function readJSON(filepath) { const fileContent = fs.readFileSync(filepath, 'utf8'); - const json = JSON.parse(fileContent); - return json; + try { + return JSON.parse(fileContent); + } catch (e) { + throw new Error(`Invalid JSON ${filepath}: ${e.message}`); + } } function writeJSON(filepath, content, dry) { diff --git a/tsconfig.es5.json b/tsconfig.base.es5.json similarity index 100% rename from tsconfig.es5.json rename to tsconfig.base.es5.json diff --git a/tsconfig.base.esm.json b/tsconfig.base.esm.json index e65651a426..d63786958e 100644 --- a/tsconfig.base.esm.json +++ b/tsconfig.base.esm.json @@ -1,5 +1,5 @@ { - "extends": "./tsconfig.es5.json", + "extends": "./tsconfig.base.es5.json", "compilerOptions": { "module": "ES6", "moduleResolution": "node"