Skip to content

Commit

Permalink
fix(metric-reader): do not use default timeouts.
Browse files Browse the repository at this point in the history
  • Loading branch information
pichlermarc committed Jan 5, 2022
1 parent 08501aa commit 151c6bb
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,19 @@ export abstract class MetricReader {
throw new Error('Collection is not allowed after shutdown');
}

return await callWithTimeout(this._metricProducer.collect(), options.timeoutMillis ?? 10000);
// No timeout if timeoutMillis is undefined or null.
if (options.timeoutMillis == null) {
return await this._metricProducer.collect();
}

return await callWithTimeout(this._metricProducer.collect(), options.timeoutMillis);
}

/**
* Shuts down the metric reader, the promise will reject after the specified timeout or resolve after completion.
* Shuts down the metric reader, the promise will reject after the optional timeout or resolve after completion.
*
* <p> NOTE: this operation will continue even after the promise rejects due to a timeout.
* @param options options with timeout (default: 10000ms).
* @param options options with timeout.
*/
async shutdown(options: ReaderShutdownOptions): Promise<void> {
// Do not call shutdown again if it has already been called.
Expand All @@ -116,21 +121,33 @@ export abstract class MetricReader {
return;
}

await callWithTimeout(this.onShutdown(), options.timeoutMillis ?? 10000);
// No timeout if timeoutMillis is undefined or null.
if (options.timeoutMillis == null) {
await this.onShutdown();
} else {
await callWithTimeout(this.onShutdown(), options.timeoutMillis);
}

this._shutdown = true;
}

/**
* Flushes metrics read by this reader, the promise will reject after the specified timeout or resolve after completion.
* Flushes metrics read by this reader, the promise will reject after the optional timeout or resolve after completion.
*
* <p> NOTE: this operation will continue even after the promise rejects due to a timeout.
* @param options options with timeout (default: 10000ms).
* @param options options with timeout.
*/
async forceFlush(options: ReaderForceFlushOptions): Promise<void> {
if (this._shutdown) {
throw new Error('Cannot forceFlush on already shutdown MetricReader.');
}

await callWithTimeout(this.onForceFlush(), options.timeoutMillis ?? 10000);
// No timeout if timeoutMillis is undefined or null.
if (options.timeoutMillis == null) {
await this.onForceFlush();
return;
}

await callWithTimeout(this.onForceFlush(), options.timeoutMillis);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ class TestDeltaMetricExporter extends TestMetricExporter {
}

class TestMetricProducer implements MetricProducer {
public collectionTime = 0;

async collect(): Promise<MetricData[]> {
await new Promise(resolve => setTimeout(resolve, this.collectionTime));
return [];
}
}
Expand Down Expand Up @@ -329,5 +332,24 @@ describe('PeriodicExportingMetricReader', () => {
await reader.shutdown({});
await assert.rejects(() => reader.collect({}));
});

it('should time out when timeoutMillis is set', async () => {
const exporter = new TestMetricExporter();
const reader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: MAX_32_BIT_INT,
exportTimeoutMillis: 80,
});
const producer = new TestMetricProducer();
producer.collectionTime = 40;
reader.setMetricProducer(producer);

await assert.rejects(
() => reader.collect({ timeoutMillis: 20 }),
thrown => thrown instanceof TimeoutError
);

await reader.shutdown({});
});
});
});

0 comments on commit 151c6bb

Please sign in to comment.