Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single endpoint for all prometheus metrics #278

Merged
merged 15 commits into from
Mar 19, 2025

Conversation

kennsippell
Copy link
Member

@kennsippell kennsippell commented Mar 17, 2025

The fastify-metrics library doesn't play particularly nicely for extensibility. I attempted a few vectors to merge these two endpoints:

  1. Interupt the response from fastify-metrics and append additional metrics from BullMQ
  2. Obtain the promClient object and add the bullmq metrics as custom counters within fastify-metrics
  3. Registry the fastify-metrics as its own endpoint and have /metrics invoke it

In the end, I liked the code for 1 the best. Let me know if you'd like to see some of the other approaches!

#273

Copy link
Collaborator

@mrjones-plip mrjones-plip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the code looks good as far as I can tell, I cannot get the /metrics endpiont to work - I get a 302 to login. I actually went back to a branch before you made your bullq change and the /metrics endpoint didn't work then either. I had to go all the way back to 4dafb28 - and then it finally worked! Something changed between now and then - I can't see what off hand 🤔

However, researching what we did back then, I found the watchdog-config directory! This has all the files we just recreated in watchdog. We should pick one winner (watchdog or UMT) and remove the watchdog config files from the loser.

I was involved with this earlier PR - sorry I didn't remember about these files already existing!

@kennsippell
Copy link
Member Author

kennsippell commented Mar 17, 2025

Oh that's weird it isn't working... The first guess I have is that have an outdated build artifact in your dist folder. Can we follow these steps to test?

  1. Delete dist folder
  2. Run ./docker-local-setup.sh build
  3. Navigate to http://localhost:3500/metrics

I'm seeing a prometheus endpoint with both fastify and bullmq metrics when I run this test. Hope that works.

We should pick one winner (watchdog or UMT) and remove the watchdog config files from the loser.

Oh wow! I didn't see that until now. That's too bad they also aren't on watchdog. I'd personally prefer to move them to the watchdog repro probably so we can track changes and deploy them for https://watchdog.app.medicmobile.org. Does that make sense to you?

@mrjones-plip
Copy link
Collaborator

Ah - sweet - thanks Kenn! Nuking the dist folder did the trick. Faster for me to run npm run dev, but I'm sure docker-local-setup.sh will work too. /metrics works now - yay!

and yeah - go ahead and nuke the watchdog files here in favor of watchdog repo. Maybe replace the readme with a big "see watchdog repo" and a link?

@mrjones-plip
Copy link
Collaborator

I finished testing in the other watchdog ticket and this new set up works great - nicely done! OK to merge.

@kennsippell kennsippell merged commit 7a698b8 into main Mar 19, 2025
1 check passed
@kennsippell kennsippell deleted the 273-bullmq-single-endpoint branch March 19, 2025 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants