From e5394bdbddd62d3036c530a6da4d3e6de6a08c74 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Wed, 25 Jan 2023 11:05:20 -0800 Subject: [PATCH] fix: apply changes picked up in rebase --- .../src/aggregator/ExponentialHistogram.ts | 27 ++++++++----------- .../aggregator/ExponentialHistogram.test.ts | 16 ++--------- .../exponential-histogram/helpers.ts | 10 ++++++- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts b/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts index 3edab1118c1..7ba26ea01f9 100644 --- a/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts +++ b/packages/sdk-metrics/src/aggregator/ExponentialHistogram.ts @@ -29,9 +29,8 @@ import { InstrumentDescriptor, InstrumentType } from '../InstrumentDescriptor'; import { Maybe } from '../utils'; import { AggregationTemporality } from '../export/AggregationTemporality'; import { Buckets } from './exponential-histogram/Buckets'; +import { getMapping } from './exponential-histogram/mapping/getMapping'; import { Mapping } from './exponential-histogram/mapping/types'; -import { ExponentMapping } from './exponential-histogram/mapping/ExponentMapping'; -import { LogarithmMapping } from './exponential-histogram/mapping/LogarithmMapping'; import * as util from './exponential-histogram//util'; /** @@ -66,13 +65,14 @@ class HighLow { constructor(public low: number, public high: number) {} } -export class ExponentialHistogramAccumulation implements Accumulation { - static DEFAULT_MAX_SIZE = 160; - static MIN_MAX_SIZE = 2; +const MAX_SCALE = 20; +const DEFAULT_MAX_SIZE = 160; +const MIN_MAX_SIZE = 2; +export class ExponentialHistogramAccumulation implements Accumulation { constructor( public startTime: HrTime = startTime, - private _maxSize = ExponentialHistogramAccumulation.DEFAULT_MAX_SIZE, + private _maxSize = DEFAULT_MAX_SIZE, private _recordMinMax = true, private _sum = 0, private _count = 0, @@ -81,13 +81,12 @@ export class ExponentialHistogramAccumulation implements Accumulation { private _max = Number.NEGATIVE_INFINITY, private _positive = new Buckets(), private _negative = new Buckets(), - private _mapping: Mapping = LogarithmMapping.get(LogarithmMapping.MAX_SCALE) + private _mapping: Mapping = getMapping(MAX_SCALE) ) { - if (this._maxSize < ExponentialHistogramAccumulation.MIN_MAX_SIZE) { + if (this._maxSize < MIN_MAX_SIZE) { diag.warn(`Exponential Histogram Max Size set to ${this._maxSize}, \ - changing to the minimum size of: \ - ${ExponentialHistogramAccumulation.MIN_MAX_SIZE}`); - this._maxSize = ExponentialHistogramAccumulation.MIN_MAX_SIZE; + changing to the minimum size of: ${MIN_MAX_SIZE}`); + this._maxSize = MIN_MAX_SIZE; } } @@ -419,11 +418,7 @@ export class ExponentialHistogramAccumulation implements Accumulation { this._positive.downscale(change); this._negative.downscale(change); - if (newScale <= 0) { - this._mapping = ExponentMapping.get(newScale); - } else { - this._mapping = LogarithmMapping.get(newScale); - } + this._mapping = getMapping(newScale); } /** diff --git a/packages/sdk-metrics/test/aggregator/ExponentialHistogram.test.ts b/packages/sdk-metrics/test/aggregator/ExponentialHistogram.test.ts index 63093e648f2..72f895096f9 100644 --- a/packages/sdk-metrics/test/aggregator/ExponentialHistogram.test.ts +++ b/packages/sdk-metrics/test/aggregator/ExponentialHistogram.test.ts @@ -26,9 +26,8 @@ import { ExponentialHistogramAggregator, } from '../../src/aggregator/ExponentialHistogram'; import { Buckets } from '../../src/aggregator/exponential-histogram/Buckets'; +import { getMapping } from '../../src/aggregator/exponential-histogram/mapping/getMapping'; import { Mapping } from '../../src/aggregator/exponential-histogram//mapping/types'; -import { ExponentMapping } from '../../src/aggregator/exponential-histogram//mapping/ExponentMapping'; -import { LogarithmMapping } from '../../src/aggregator/exponential-histogram/mapping/LogarithmMapping'; import * as assert from 'assert'; import { assertInEpsilon, @@ -534,10 +533,7 @@ describe('ExponentialHistogramAccumulation', () => { describe('min max size', () => { it('auto-corrects to min max', () => { const acc: any = new ExponentialHistogramAccumulation([0, 0], 0); - assert.strictEqual( - acc['_maxSize'], - ExponentialHistogramAccumulation.MIN_MAX_SIZE - ); + assert.strictEqual(acc['_maxSize'], 2); }); }); }); @@ -783,14 +779,6 @@ function centerValue(mapper: Mapping, x: number): number { return (lower + upper) / 2; } -function getMapping(scale: number): Mapping { - if (scale <= 0) { - return ExponentMapping.get(scale); - } else { - return LogarithmMapping.get(scale); - } -} - function assertHistogramsEqual( actual: ExponentialHistogramAccumulation, expected: ExponentialHistogramAccumulation diff --git a/packages/sdk-metrics/test/aggregator/exponential-histogram/helpers.ts b/packages/sdk-metrics/test/aggregator/exponential-histogram/helpers.ts index a2241fbf667..5f89b347710 100644 --- a/packages/sdk-metrics/test/aggregator/exponential-histogram/helpers.ts +++ b/packages/sdk-metrics/test/aggregator/exponential-histogram/helpers.ts @@ -21,7 +21,7 @@ export function assertInEpsilon( epsilon: number ) { assert.ok(!Number.isNaN(actual), 'unexpected NaN for actual argument'); - assert.ok(!Number.isNaN(expected), 'unexpected NaN for expected argument'); + assert.ok(!Number.isNaN(expected), 'unexpected NaN for exepected argument'); assert.ok(actual !== 0, 'unexpected 0 for actual argument'); const relErr = Math.abs(actual - expected) / Math.abs(actual); @@ -31,3 +31,11 @@ export function assertInEpsilon( `expected relative error: ${relErr} to be < ${epsilon}` ); } + +export function assertInDelta(actual: number, expected: number, delta: number) { + const actualDelta = Math.abs(expected - actual); + assert.ok( + actualDelta < delta, + `expected delta: ${delta} to be < ${actualDelta}` + ); +}