Skip to content

Commit 373edd9

Browse files
authored
Revert "fix(sdk-metrics): improve PeriodicExportingMetricReader() constructor input validation (#5621)" (#5671)
1 parent 0c21db4 commit 373edd9

File tree

3 files changed

+21
-37
lines changed

3 files changed

+21
-37
lines changed

CHANGELOG.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
2020
* fix(resource): do not trigger `Accessing resource attributes before async attributes settled` warning when detecting resources [#5546](https://github.com/open-telemetry/opentelemetry-js/pull/5546) @dyladan
2121
* verbose logging of detected resource removed
2222
* fix(resource): use dynamic import over require to improve ESM compliance [#5298](https://github.com/open-telemetry/opentelemetry-js/pull/5298) @xiaoxiangmoe
23-
* fix(sdk-metrics): improve PeriodicExportingMetricReader() constructor input validation [#5621](https://github.com/open-telemetry/opentelemetry-js/pull/5621) @cjihrig
2423

2524
### :books: Documentation
2625

packages/sdk-metrics/src/export/PeriodicExportingMetricReader.ts

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -60,37 +60,42 @@ export class PeriodicExportingMetricReader extends MetricReader {
6060
private readonly _exportTimeout: number;
6161

6262
constructor(options: PeriodicExportingMetricReaderOptions) {
63-
const {
64-
exporter,
65-
exportIntervalMillis = 60000,
66-
exportTimeoutMillis = 30000,
67-
metricProducers,
68-
} = options;
69-
7063
super({
71-
aggregationSelector: exporter.selectAggregation?.bind(exporter),
64+
aggregationSelector: options.exporter.selectAggregation?.bind(
65+
options.exporter
66+
),
7267
aggregationTemporalitySelector:
73-
exporter.selectAggregationTemporality?.bind(exporter),
74-
metricProducers,
68+
options.exporter.selectAggregationTemporality?.bind(options.exporter),
69+
metricProducers: options.metricProducers,
7570
});
7671

77-
if (exportIntervalMillis <= 0) {
72+
if (
73+
options.exportIntervalMillis !== undefined &&
74+
options.exportIntervalMillis <= 0
75+
) {
7876
throw Error('exportIntervalMillis must be greater than 0');
7977
}
8078

81-
if (exportTimeoutMillis <= 0) {
79+
if (
80+
options.exportTimeoutMillis !== undefined &&
81+
options.exportTimeoutMillis <= 0
82+
) {
8283
throw Error('exportTimeoutMillis must be greater than 0');
8384
}
8485

85-
if (exportIntervalMillis < exportTimeoutMillis) {
86+
if (
87+
options.exportTimeoutMillis !== undefined &&
88+
options.exportIntervalMillis !== undefined &&
89+
options.exportIntervalMillis < options.exportTimeoutMillis
90+
) {
8691
throw Error(
8792
'exportIntervalMillis must be greater than or equal to exportTimeoutMillis'
8893
);
8994
}
9095

91-
this._exportInterval = exportIntervalMillis;
92-
this._exportTimeout = exportTimeoutMillis;
93-
this._exporter = exporter;
96+
this._exportInterval = options.exportIntervalMillis ?? 60000;
97+
this._exportTimeout = options.exportTimeoutMillis ?? 30000;
98+
this._exporter = options.exporter;
9499
}
95100

96101
private async _runOnce(): Promise<void> {

packages/sdk-metrics/test/export/PeriodicExportingMetricReader.test.ts

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -226,26 +226,6 @@ describe('PeriodicExportingMetricReader', () => {
226226
}),
227227
/exportIntervalMillis must be greater than or equal to exportTimeoutMillis/
228228
);
229-
assert.throws(
230-
() =>
231-
new PeriodicExportingMetricReader({
232-
exporter: exporter,
233-
exportIntervalMillis: 100,
234-
// exportTimeoutMillis defaults to 30 seconds, which is greater than exportIntervalMillis.
235-
exportTimeoutMillis: undefined,
236-
}),
237-
/exportIntervalMillis must be greater than or equal to exportTimeoutMillis/
238-
);
239-
assert.throws(
240-
() =>
241-
new PeriodicExportingMetricReader({
242-
exporter: exporter,
243-
// exportIntervalMillis defaults to 60 seconds, which is less than exportTimeoutMillis.
244-
exportIntervalMillis: undefined,
245-
exportTimeoutMillis: 90_000,
246-
}),
247-
/exportIntervalMillis must be greater than or equal to exportTimeoutMillis/
248-
);
249229
});
250230

251231
it('should not start exporting', async () => {

0 commit comments

Comments
 (0)