-
Notifications
You must be signed in to change notification settings - Fork 17
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
Add default metrics #24
Conversation
They can be disabled using a commandline arg, like in the node_exporter
By now my Node Exporter PR(prometheus/node_exporter#2808) containing the same changes as rpi-exporter.go:128-145 is merged(and released), so I think that solution is "good enough". |
Can you rebase on |
Whoops, looks like GitHubs web tool merges, rather than rebasing. |
No stress. If the diff view is cleaned up, that's fine as well. Looks like this PR also updated a dependency? Prometheus SDK apparently. At some point, I have to give this repo a makeover that utilises Go modules... 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I don't think this PR updates a dependency, it does however add one. |
@ToMe25 Do you mind giving the main branch a run? If you're happy, I'll stage a release (that unfortunately is a rather manual process that involves the Prometheus tooling ( Really time to port this to Go modules and GitHub actions... |
Ups yeah. Totally fine, tho. |
I assume you mean the |
@lukasmalkmus i did some testing, and I didn't find any problems. It took me this long in part because I'm an idiot and tried to run a x86 build on my RPi. Here a few things I feel like i should mention, which I didn't mention so far because I thought you would test it yourself anyway:
Here is an example metrics page from a RPi3:
|
Hi. Since some time passed since this PR, I was just wondering: Is there an ETA for the 0.9.0 release? |
@ToMe25 Apologies but maintaining this hasn't been the greatest experience, recently. If I ever have some time and motivation I refactor this and establish proper CI with GitHub Actions so we can have automatic releases and I don't have to fight with That being said, I managed to cut a pre-release. Do you mind giving it a test run before I cut a real release? https://github.com/lukasmalkmus/rpi_exporter/releases/tag/v0.9.0-rc.0. /cc @carlosedp Looks like we'll release |
@lukasmalkmus If you want I can make a PR to set up GitHub CI for that for you. If you have any special promu config/args you use for the official builds please let me know so I can integrate them in the CI. Also I would make it create a build for every push to the If you would like anything different from my suggestion, please let me know :) |
I'm actually thinking of pulling out all the Promu and/or Prometheus specific pieces. I have another exporter here, that got an overhaul last year: https://github.com/lukasmalkmus/tankerkoenig_exporter. The Makefile and CI there are way more sane if you wanna give it a look. I guess I'm opting for something like this. |
@lukasmalkmus That's probably the better solution, but I don't have the know how to help with that. I will test the Rc build tomorrow and let you know if I find any issues. |
@lukasmalkmus I tested everything I could think of with one of the builds, and tested some basic things with the other three. I did all of these tests on my RPi 3b with Raspbian lite 64 bit. After this testing there are three things I think are worth mentioning, though I don't think any of them are that much of an issue.
I'll make a separate issue about the third of those, but I don't see it as anything urgent. |
Hi Lukas, I can take a look at it once it's released... It's been a long time and I don't remember very well how I used to release it :) |
@ToMe25 Awesome. Thanks for testing. Will look through your issues/PRs by the end of the week. Also gonna look into modernising the CI/CD for this repo. @carlosedp That would be great! On the other hand... If I give CI/CD an overhaul anyhow, we could probably release to GHCR and DockerHub simultaneously. If you share the Dockerfile with me we could probably set something up that releases under you DockerHub username. Probably we need to put an API token down for the GitHub Action Secrets. Lemme get back to you once I make some progress on this, if that's fine with you :) |
This PR adds the default collectors to the metrics endpoint.
These collectors are added to the default registry by default, and collect some data about the current process.
However since this exporter uses a custom registry, it didn't use them until now.
Using these collectors can be disabled using
--web.disable-exporter-metrics
, like in the node_exporter.I would have liked to solve rpi_exporter.go:128-145 somewhat more elegantly, but I couldn't find a way to do that.
This PR is based on #23.
Since they both required the same base changes, and I didn't want to implement those twice, all commits from #23 are also included in this PR.
I hope this means I wont have to rebase this if #23 gets merged, but maybe I will have to, idk.