-
Notifications
You must be signed in to change notification settings - Fork 50
Add watchtower client number of backup metrics #120
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
Conversation
|
This will have to wait for lightninglabs/lndclient#235 to be merged before the proper |
ee6a834 to
59787a0
Compare
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.
Looks good, thanks for the addition 🗼! Could you make sure to change commit message headers to be consistent with the repo's (just have a look at the past ones). For example Update go.mod and go.sum would become mod: update go.mod and go.sum or Fix comment would be collectors: fix comment (i.e. prefixing with the system/topic it concerns).
We also don't have a long line linter active in this repo, please try to cap lines at 80 chars. Will do a test run after your fixes so that we can merge the lndclient PR and then update the library in here.
59787a0 to
e66180e
Compare
|
Things should be up to date now! |
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.
Looking great, we need to catch errors if the watchtower client wasn't enabled, see the comment, otherwise lndmon won't run. Please make sure to run the linter before you push, the code requires reformatting. I have tested backup metrics collection as well 🎉
125bea3 to
96f3471
Compare
|
Final look and final comments? |
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.
Tested again, LGTM 🎉. Thanks again!
A next step would be to add a dashboard such that other users will have access to those numbers by default (if you like to do another project).
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.
Thanks for your effort, I just flagged some minor improvements, otherwise looks good to go.
96f3471 to
3c49dbb
Compare
|
Great work, LGTM✅ |
Update dependencies affected by this upgrade through `go mod tidy`
Fix spelling mistake in comment
Add collector for the numBackups and numPendingBackups from the lnd watchtower client API.
3c49dbb to
a7f7f06
Compare
Summary: This PR adds support for tracking the number of backups that the watchtower client has recorded.
Purpose: We are interested in keeping track of if our watchtower is functioning and making sure that the number of backups corresponds to the number of transactions is one metric we would like to use for this purpose.
Code:
This PR adds a new collector,
wt_client_collector.go, which works very similarly to previous collectors.This PR relies on an updated version of
lndclient(update here) to communicate with the watchtower client interface oflnd.Semantics:
2 metrics are exposed
num_backupsandnum_pending_backups.The metrics will not show up at all if there is no tower active.
Each metric has a towers public key as a label.