PeriodicExportingMetricReader: timeout can be greater than interval

[sburuiana-snyk]

trafficstars

What happened?

A PeriodicExportingMetricReader has a _exportInterval and a _exportTimeout, that can optionally be specified via options in the constructor. If not specified, they will default to 60s and 30s respectively, as per otel documentation.

This line in the reader code suggests that the intention is that timeout should never be greater than interval (which does make sense from a logic standpoint), and the reader will throw an error if values are specified as such in the constructor.

However, if just one of the two values is specified (and the other is implicitly set to the default value), it can lead to a situation where the timeout does become greater than the interval. For example, consider this snippet:

const periodicMetricReader = new PeriodicExportingMetricReader({
  exporter: this.metricExporter,
  exportIntervalMillis: 10000,
});

This will lead to the reader having _exportInterval == 10000 and _exportTimeout == 30000, which seems unintended based on the previously mentioned constructor behavior.

A quick suggestion for a fix would be to move the check a couple lines below:

-    if (
-     options.exportTimeoutMillis !== undefined &&
-      options.exportIntervalMillis !== undefined &&
-      options.exportIntervalMillis < options.exportTimeoutMillis
-    ) {
-      throw Error(
-        'exportIntervalMillis must be greater than or equal to exportTimeoutMillis'
-      );
-    }

    this._exportInterval = options.exportIntervalMillis ?? 60000;
    this._exportTimeout = options.exportTimeoutMillis ?? 30000;

+    if (this._exportInterval < this._exportTimeout) {
+      throw Error(
+        'export interval must be greater than or equal to export timeout'
+      );
+    }

or maybe just reject the constructor if one of the values is specified and not the other. I'm not sure what the intended resulting behavior would need to be, so I'm opening this issue instead of opening a PR.

OpenTelemetry Setup Code

Irrelevant for this issue

package.json

Irrelevant for this issue

Relevant log output

Irrelevant for this issue

Operating System and Version

Irrelevant for this issue

Runtime and Version

Irrelevant for this issue