Implement PerformanceObserver

paymog commented 1 year ago

What is the problem this feature would solve?

It would be great if Bun could implement the PerformanceObserver which seems to be required for using the prom-client package.

EDIT: turns out this is only required when collectDefaultMetrics is called on the client.

 ❮❮❮ bun src/index.ts
 8 | }, $;
 9 |
10 | class NotImplementedError extends Error {
11 |   code;
12 |
13 |   constructor(feature, issue) {
                             ^
NotImplementedError: PerformanceObserver is not yet implemented in Bun.
 code: "ERR_NOT_IMPLEMENTED"

      at new NotImplementedError (internal:shared:13:26)
      at internal:shared:2:68
      at new PerformanceObserver (node:perf_hooks:17:26)
      at /Users/paymahn/code/goldsky/rpc-node-proxy/node_modules/prom-client/lib/metrics/gc.js:44:13
      at collectDefaultMetrics (/Users/paymahn/code/goldsky/rpc-node-proxy/node_modules/prom-client/lib/defaultMetrics.js:47:2)
      at /Users/paymahn/code/goldsky/rpc-node-proxy/src/metrics/prometheus.ts:10:0
      at processTicksAndRejections (:1:2602)

What is the feature you are proposing to solve the problem?

Implementation of the PerformanceObserver which is currently not implemented

What alternatives have you considered?

None yet, I'm very new to the ecosystem.

kristofgazso commented 1 year ago

This is currently the only blocker for us to start using Bun 💯💯💯

vladislav-sidorovich commented 1 year ago

I would like to support @kristofgazso . For our company it's also the single blocker.

vicwolk commented 1 year ago

This is our only blocker as well.

agjs commented 1 year ago

Only blocker as well. Getting the error when trying to run our Fastify server, due to the fact that we are using fastify-metrics, that also uses prom-client.

MarkCupitt commented 1 year ago

Yep, us too, we would have to rewrite a bunch of container apps or remove prom-client

agjs commented 1 year ago

Hey @Jarred-Sumner. I'd like to maybe give Zig a shot and try implementing this feature in Bun. Keep in mind, I never wrote Zig, nor I ever contributed to this repository, but it would be great to have an excuse.

Beside setting up the Bun development environment, could you give me a guideline or two where to start looking.

Thanks

Tjerk-Haaye-Henricus commented 1 year ago

For us it would also be really important :D We would like to see Bun Performance in our reports 😎

dipbd1 commented 1 year ago

This is a blocker for my project too. I would love to contribute.

o-az commented 1 year ago

This is a currently the only blocker for https://github.com/0xOlias/ponder too.

Ponder uses prom-client which requires this API

marziply commented 1 year ago

Bun is a prime candidate for replacing Node for microservices I am working on. By commenting out collectDefaultMetrics, these services work perfectly. This issue is a blocker on any plans for migrating to Bun since Prometheus is not a tool I can remove.

stavalfi commented 1 year ago

Also a blocker for us. Can't remove Prometheus.

maximilianschmid commented 1 year ago

Last blocker for Planfred to run on bun

jnovax commented 11 months ago

Love to see it soon.

Jarred-Sumner commented 11 months ago

The good news is we can almost entirely re-use the implementation from Safari for this API.

I think this would mostly look like:

Copy these files:

Build the browser version of WebKit locally, and copy the coreesponding generated JS binding files
Fix any of the build errors that come up along the way
Add mark, measure, etc to the performance object
Add PerformanceObserver globally
Do a pass through the related APIs and see if there are any other globals that will need to be exposed

I don't love this approach because it would be nice to emit various events from zig. I also don't love how many memory allocations are involved in WebKit's implementation. performance.mark and performance.measure needs to be pretty cheap. But this approach is straightforward to ship (and would be faster than an equivalent JS approach)

agjs commented 10 months ago

Any movement on this?

MarkCupitt commented 10 months ago

yes, will this happen?

On Thu, 21 Dec 2023, 9:10 pm Aleksandar Grbic, @.***> wrote:

Any movement on this?

— Reply to this email directly, view it on GitHub https://github.com/oven-sh/bun/issues/4708#issuecomment-1866217439, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJ7RUEFNOEKLZORFH2ZN6DYKQYNLAVCNFSM6AAAAAA4RRQSYOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWGIYTONBTHE . You are receiving this because you commented.Message ID: @.***>

gchicoye commented 9 months ago

Hi, any news on that? Thanks a lot!!!

Jarred-Sumner commented 9 months ago

This will ship in Bun v1.0.22, thanks to @gvilums

mishraomp commented 6 months ago

can this issue be reopened @Jarred-Sumner , I tried to run bun , but its kicking off this error both in windows and docker environments


16 |
17 | class NotImplementedError extends Error {
18 |   code;
19 |   constructor(feature, issue) {
                               ^
NotImplementedError: perf_hooks.monitorEventLoopDelay is not yet implemented in Bun.
 code: "ERR_NOT_IMPLEMENTED"

      at new NotImplementedError (internal:shared:19:27)
      at internal:shared:2:69
      at monitorEventLoopDelay (node:perf_hooks:123:47)
      at /usr/src/app/node_modules/prom-client/lib/metrics/eventLoopLag.js:47:21
      at collectDefaultMetrics (/usr/src/app/node_modules/prom-client/lib/defaultMetrics.js:47:3)
      at /usr/src/app/prom.js:34:1
      at /usr/src/app/metrics.controller.js:17:7
      at /usr/src/app/app.module.js:17:7`

here is the link to the source code which I wanted to run Existing nest/express API with Node https://github.com/bcgov/nr-omrr-transparency/tree/main/backend

rhinodavid commented 6 months ago

can this issue be reopened @Jarred-Sumner , I tried to run bun , but its kicking off this error both in windows and docker environments Looks like it's here: https://github.com/oven-sh/bun/issues/9432

oven-sh / bun