opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

TypeError: (0, http_1.createServer)(this._requestHandler).unref when running in a Bun environment

Open acamposruiz opened this issue 4 months ago • 3 comments

What happened?

I am in the process of migrating an application that uses this project from Node.js to Bun. During the migration, I encountered an error related to the use of unref(), which seems to be exclusive to Node.js environments.

Steps to Reproduce

Run 'experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts' in a Bun environment

Expected Result

Should work as expected.

Actual Result

The process stops due to the following error:

2024-02-16 11:47:36 --------------------- Bun Inspector ---------------------
2024-02-16 11:47:37 88 |         this._appendTimestamp =
2024-02-16 11:47:37 89 |             typeof config.appendTimestamp === 'boolean'
2024-02-16 11:47:37 90 |                 ? config.appendTimestamp
2024-02-16 11:47:37 91 |                 : PrometheusExporter.DEFAULT_OPTIONS.appendTimestamp;
2024-02-16 11:47:37 92 |         // unref to prevent prometheus exporter from holding the process open on exit
2024-02-16 11:47:37 93 |         this._server = (0, http_1.createServer)(this._requestHandler).unref();
2024-02-16 11:47:37                   ^
2024-02-16 11:47:37 TypeError: (0, http_1.createServer)(this._requestHandler).unref is not a function. (In '(0, http_1.createServer)(this._requestHandler).unref()', '(0, http_1.createServer)(this._requestHandler).unref' is undefined)
2024-02-16 11:47:37       at new PrometheusExporter (/usr/src/app/node_modules/@opentelemetry/exporter-prometheus/build/src/PrometheusExporter.js:93:14)
2024-02-16 11:47:37       at new Meter (/usr/src/app/node_modules/@mediaset/metrics/dist/src/metrics/metrics.js:19:26)
2024-02-16 11:47:37       at /usr/src/app/node_modules/@mediaset/metrics/dist/src/metrics/metrics.js:104:19

Additional Details

I have implemented a patch as a workaround:

diff --git a/node_modules/@opentelemetry/exporter-prometheus/build/src/PrometheusExporter.js b/node_modules/@opentelemetry/exporter-prometheus/build/src/PrometheusExporter.js
index b9776ec..1e4d7cb 100644
--- a/node_modules/@opentelemetry/exporter-prometheus/build/src/PrometheusExporter.js
+++ b/node_modules/@opentelemetry/exporter-prometheus/build/src/PrometheusExporter.js
@@ -90,7 +90,12 @@ class PrometheusExporter extends sdk_metrics_1.MetricReader {
                 ? config.appendTimestamp
                 : PrometheusExporter.DEFAULT_OPTIONS.appendTimestamp;
         // unref to prevent prometheus exporter from holding the process open on exit
-        this._server = (0, http_1.createServer)(this._requestHandler).unref();
+        this._server = (0, http_1.createServer)(this._requestHandler);
+
+        // Bun construction of the server to allow for unref
+        if (typeof this._server.unref === 'function') {
+        this._server.unref();
+        }
         this._serializer = new PrometheusSerializer_1.PrometheusSerializer(this._prefix, this._appendTimestamp);
         this._baseUrl = `http://${this._host}:${this._port}/`;
         this._endpoint = (config.endpoint || PrometheusExporter.DEFAULT_OPTIONS.endpoint).replace(/^([^/])/, '/$1');

I propose including this patch in the project. I am willing to create a PR if the team agrees.

OpenTelemetry Setup Code

No response

package.json

No response

Relevant log output

No response

acamposruiz avatar Feb 16 '24 10:02 acamposruiz

Re-categorizing as a feature as we don't expect any of our packages to work with bun. We're happy to accept a PR if that's the only change needed to get it working. Our policy for unsupported runtimes is that if there's something easy we can do to make the life easier we'll accept the changes. :slightly_smiling_face:

Keep in mind that us accepting this change does not mean that bun becomes a supported runtime.

pichlermarc avatar Feb 21 '24 16:02 pichlermarc

Thank you very much @pichlermarc , the PR is done here https://github.com/open-telemetry/opentelemetry-js/pull/4613 Anything else I can do just tell me :)

acamposruiz avatar Apr 08 '24 10:04 acamposruiz

Pretty sure this problem was fixed in Bun itself

weyert avatar Apr 10 '24 00:04 weyert