-
Notifications
You must be signed in to change notification settings - Fork 365
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
feat: add node_exporter metrics #1627
Conversation
Ran locally and it worked fine. Did not have access to a server to test the playbook yett. |
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 it locally. I only see the node_exporter
dashboard for the batcher, is that alright? Shouldn't I also see the aggregator dashboard?
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 fine but this PR needs approves from DevOps
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.
Run it locally and everything seems to work fine, but mac os doesn't show many graphs. I left it running for a few minutes and if I chose the 24h time frame on Grafana some new graphs appears.
Anyways, I think this PR should really be tested on a server so all graphs are shown and we test the ansible script.
If you tested it locally, then the metrics displayed were from your computer, not specifically from the batcher. Did it say 'Batcher' anywhere? |
- name: Node Exporter Setup | ||
hosts: "{{ host }}" | ||
|
||
tasks: |
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.
This Ansible task assumes the repo is already cloned, which might not be the case.
Also, there are some sender
servers, like aligned-holesky-stage-1-sender
, which @JuArce told me to test this PR on. Why are we running this ansible inside the Aggergator and Batcher? Is this whole Node Exporter thing an addon to this components, or should it be a separate component with it's own server?
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.
Yes, I made the playbook to be used after the repo was cloned intentionally, so we could include it within the playbooks of the Aggregator and Batcher. But I made a mistake when adding the playbook to the others, as the repository hadn't been cloned at that step yet 🤣. I'll fix that in a moment.
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.
Addressed in #7923c1a!
prometheus/prometheus.yaml
Outdated
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.
Unless this file is for deploying prometheus
locally, we should also add this scrape target to the actual telemetry servers, right?
In my PR adding Ansible to the whole Telemetry, I have a config file I copy to prometheus.
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.
Yes, we need to add the two new jobs to that file as well
scripts/install_node_exporter.sh
Outdated
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.
If you found this script online, we should credit the original author.
Please add a comment below the shebang (#!/bin/bash
) with a link to the original version.
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.
I actually use install_aligned.sh as a reference.
# Conflicts: # Makefile
Add node_exporter metrics
Description
Adds node_exporter to be deployed in the Aligned servers.
How to Test
localhost:9100
and you should see the exposed metrics.localhost:3000
and you should see the new dashboardNode Exporter Full
with your metrics displayed.Note
If you are on macos, some of the metrics will appear with
no data
.Type of change
Checklist
testnet
, everything else tostaging