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

[READY] [Issue-567] - Adding OpenWRT Grafana Dashboard #669

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

jshcmpbll
Copy link
Contributor

@jshcmpbll jshcmpbll commented Feb 13, 2024

Description of PR

relates to: #567

Previous Behavior

No dashboards configured by default for grafana

New Behavior

openwrt grafana dashboard

Tests

nix build
@sarcasticadmin is going to test with a local OpenWRT deployment since I dont have any hardware

@sarcasticadmin sarcasticadmin self-requested a review February 19, 2024 18:16
Copy link
Member

@sarcasticadmin sarcasticadmin left a comment

Choose a reason for hiding this comment

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

@jshcmpbll This looks like exactly what we need!

Going to work on getting this connected to openwrt shortly

@@ -65,11 +66,18 @@ in
datasources.settings.datasources = [
{
name = "prometheus";
uid = "P1809F7CD0C75ACF3";
Copy link
Member

Choose a reason for hiding this comment

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

just to clarify, does this value have any specific meaning or does it just need to be $SOME_VALUE that matches all the datasource references form the dashboard json?

Copy link
Member

Choose a reason for hiding this comment

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

Im unclear on this myself

@jshcmpbll might be able to add some insight

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kylerisse Yup! The value itself has no real meaning but would need to match the data source values found in the dashboard like so:

"uid": "P1809F7CD0C75ACF3"

Each time I started the VM it seemed to use that UID for the datasource by default but I figured we might as well set it so there isn't a possibility of some change in the future resulting in the UID changing.

Copy link
Member

Choose a reason for hiding this comment

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

sweet, thanks @jshcmpbll

@jshcmpbll
Copy link
Contributor Author

@jshcmpbll This looks like exactly what we need!

Going to work on getting this connected to openwrt shortly

I started working on the exporter replay test we talked about but still trying to figure out networking for nix tests.

Copy link
Collaborator

@owendelong owendelong left a comment

Choose a reason for hiding this comment

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

LGTM

@owendelong
Copy link
Collaborator

Not merging because marked "REVIEW". If you want it merged, please mark it READY.

Previously this was defaulting to port 80 which is not where the
exporters are in the APs.
Copy link
Member

@sarcasticadmin sarcasticadmin left a comment

Choose a reason for hiding this comment

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

I was able to setup an AP and run this monitoring vm. The dashboard populates as expected and everything seems to be coming in nicely! (ignore the wifi quality metric, I really didnt gather too much data for this initial test)

ss-202402231708670002

I added a small change in the inventory.py to ensure that we are using port 9100 for the metrics exporters on the APs.

Nice work @jshcmpbll ! This is a big step for getting the dashboard built out!

@sarcasticadmin sarcasticadmin changed the title [REVIEW] [Issue-567] - Adding OpenWRT Grafana Dashboard [READY] [Issue-567] - Adding OpenWRT Grafana Dashboard Feb 23, 2024
@sarcasticadmin sarcasticadmin merged commit 8cb8aab into socallinuxexpo:master Feb 23, 2024
1 check passed
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.

4 participants