Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

* feat(sdk-metrics): allow single bucket histograms [#4456](https://github.com/open-telemetry/opentelemetry-js/pull/4456) @pichlermarc
* feat(instrumentation): Make `init()` method public [#4418](https://github.com/open-telemetry/opentelemetry-js/pull/4418)

### :bug: (Bug Fix)

* fix(sdk-metrics): handle zero bucket counts in exponential histogram merge [#4459](https://github.com/open-telemetry/opentelemetry-js/pull/4459) @mwear
* fix(sdk-metrics): ignore `NaN` value recordings in Histograms [#4455](https://github.com/open-telemetry/opentelemetry-js/pull/4455) @pichlermarc
* fixes a bug where recording `NaN` on a histogram would result in the sum of bucket count values not matching the overall count
* fix(sdk-metrics): allow single bucket histograms [#4456](https://github.com/open-telemetry/opentelemetry-js/pull/4456) @pichlermarc
* fixes a bug where `Meter.createHistogram()` with the advice `explicitBucketBoundaries: []` would throw

### :books: (Refine Doc)

Expand Down
6 changes: 4 additions & 2 deletions packages/sdk-metrics/src/view/Aggregation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,10 @@ export class ExplicitBucketHistogramAggregation extends Aggregation {
private readonly _recordMinMax = true
) {
super();
if (boundaries === undefined || boundaries.length === 0) {
throw new Error('HistogramAggregator should be created with boundaries.');
if (boundaries == null) {
throw new Error(
'ExplicitBucketHistogramAggregation should be created with explicit boundaries, if a single bucket histogram is required, please pass an empty array'
);
}
// Copy the boundaries array for modification.
boundaries = boundaries.concat();
Expand Down
14 changes: 14 additions & 0 deletions packages/sdk-metrics/test/Instruments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,20 @@ describe('Instruments', () => {
});
});

it('should allow metric advice with empty explicit boundaries', function () {
const meter = new MeterProvider({
readers: [new TestMetricReader()],
}).getMeter('meter');

assert.doesNotThrow(() => {
meter.createHistogram('histogram', {
advice: {
explicitBucketBoundaries: [],
},
});
});
});

it('should collect min and max', async () => {
const { meter, deltaReader, cumulativeReader } = setup();
const histogram = meter.createHistogram('test', {
Expand Down
95 changes: 95 additions & 0 deletions packages/sdk-metrics/test/aggregator/Histogram.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,30 @@ describe('HistogramAggregator', () => {
sum: -65,
});
});

it('with single bucket', function () {
const aggregator = new HistogramAggregator([], true);
const prev = aggregator.createAccumulation([0, 0]);
prev.record(0);
prev.record(1);

const delta = aggregator.createAccumulation([1, 1]);
delta.record(2);
delta.record(11);

const expected = new HistogramAccumulation([0, 0], [], true, {
buckets: {
boundaries: [],
counts: [4],
},
count: 4,
sum: 14,
hasMinMax: true,
min: 0,
max: 11,
});
assert.deepStrictEqual(aggregator.merge(prev, delta), expected);
});
});

describe('diff', () => {
Expand Down Expand Up @@ -112,6 +136,35 @@ describe('HistogramAggregator', () => {

assert.deepStrictEqual(aggregator.diff(prev, curr), expected);
});

it('with single bucket', function () {
const aggregator = new HistogramAggregator([], true);
const prev = aggregator.createAccumulation([0, 0]);
prev.record(0);
prev.record(1);

const curr = aggregator.createAccumulation([1, 1]);
// replay actions on prev
curr.record(0);
curr.record(1);
// perform new actions
curr.record(2);
curr.record(11);

const expected = new HistogramAccumulation([1, 1], [], true, {
buckets: {
boundaries: [],
counts: [2],
},
count: 2,
sum: 13,
hasMinMax: false,
min: Infinity,
max: -Infinity,
});

assert.deepStrictEqual(aggregator.diff(prev, curr), expected);
});
});

describe('toMetricData', () => {
Expand Down Expand Up @@ -199,6 +252,48 @@ describe('HistogramAggregator', () => {
);
});

it('should transform to expected data with empty boundaries', () => {
const aggregator = new HistogramAggregator([], false);

const startTime: HrTime = [0, 0];
const endTime: HrTime = [1, 1];
const accumulation = aggregator.createAccumulation(startTime);
accumulation.record(0);
accumulation.record(1);

const expected: MetricData = {
descriptor: defaultInstrumentDescriptor,
aggregationTemporality: AggregationTemporality.CUMULATIVE,
dataPointType: DataPointType.HISTOGRAM,
dataPoints: [
{
attributes: {},
startTime,
endTime,
value: {
buckets: {
boundaries: [],
counts: [2],
},
count: 2,
sum: 1,
min: undefined,
max: undefined,
},
},
],
};
assert.deepStrictEqual(
aggregator.toMetricData(
defaultInstrumentDescriptor,
AggregationTemporality.CUMULATIVE,
[[{}, accumulation]],
endTime
),
expected
);
});

function testSum(instrumentType: InstrumentType, expectSum: boolean) {
const aggregator = new HistogramAggregator([1, 10, 100], true);

Expand Down
19 changes: 19 additions & 0 deletions packages/sdk-metrics/test/view/Aggregation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,25 @@ describe('ExplicitBucketHistogramAggregation', () => {
}
});

it('construct with empty boundaries', function () {
const boundaries: number[] = [];
const aggregation = new ExplicitBucketHistogramAggregation(boundaries);
assert.ok(aggregation instanceof ExplicitBucketHistogramAggregation);
assert.deepStrictEqual(aggregation['_boundaries'], []);
});

it('construct with undefined boundaries should throw', function () {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore simulate how a JS user could pass undefined
assert.throws(() => new ExplicitBucketHistogramAggregation(undefined));
});

it('construct with null boundaries should throw', function () {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore simulate how a JS user could pass null
assert.throws(() => new ExplicitBucketHistogramAggregation(null));
});

it('constructor should not modify inputs', () => {
const boundaries = [100, 10, 1];
const aggregation = new ExplicitBucketHistogramAggregation(boundaries);
Expand Down