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

prometheus-node-exporter-lua: hostad ubus stats #13606

Merged
merged 1 commit into from
Oct 12, 2020

Conversation

PolynomialDivision
Copy link
Member

There is already the hostapd_stations exporter, which uses hostapd-utils (more precisely hostapd-cli) to get client statistics. However, the ubus interface is permanently integrated under hostapd in OpenWrt. So this exporter needs one dependency less.

For now it exports mainly the rrm statistics. Many people are interested in what your device supports. The exporter provides information about the radio-resource-managment extensions.

Maintainer: @champtar
Compile tested: (put here arch, model, OpenWrt version)
Run tested: r14549+2-036221ce5a89

@PolynomialDivision PolynomialDivision force-pushed the add-hostapd-ubus-exporter branch from a4f0e88 to 06201c5 Compare October 8, 2020 13:51
@PolynomialDivision PolynomialDivision marked this pull request as ready for review October 8, 2020 16:27
@PolynomialDivision
Copy link
Member Author

Ah wait. There is to much code.

@PolynomialDivision PolynomialDivision force-pushed the add-hostapd-ubus-exporter branch from 06201c5 to fd96829 Compare October 9, 2020 10:02
@PolynomialDivision
Copy link
Member Author

Merge? ;)

@champtar
Copy link
Member

champtar commented Oct 9, 2020

Do you want to look at lua bitop first ?
#9707 (comment)

@karlp
Copy link
Contributor

karlp commented Oct 9, 2020

Do you want to look at lua bitop first ?
#9707 (comment)

It should be s/bit32/bit/g and.... done. You can even keep calling the local bit32 and just change the require to

local bit32 = require "bit"

and you should be good to go.

@PolynomialDivision PolynomialDivision force-pushed the add-hostapd-ubus-exporter branch from fd96829 to 0299d10 Compare October 10, 2020 10:11
@PolynomialDivision
Copy link
Member Author

Wait. Run-testing gives me
Sat Oct 10 12:45:14 2020 daemon.info procd: Instance prometheus-node-exporter-lua::instance1 pid 6134 not stopped on SIGTERM, sending SIGKILL instead

@PolynomialDivision
Copy link
Member Author

If I remove my plugin, it will have that SIGKILL issue, too. So this is a bug that is not caused by my plugin. So merge. ;)

@neheb
Copy link
Contributor

neheb commented Oct 11, 2020

     utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:9:7: unused loop variable dev
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:19:65: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:19:121: line is too long (129 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:20:64: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:20:121: line is too long (127 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:21:70: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:21:121: line is too long (139 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:22:69: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:22:121: line is too long (137 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:23:68: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:23:121: line is too long (135 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:24:64: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:24:121: line is too long (127 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:25:65: accessing undefined variable metric
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:25:121: line is too long (129 > 120)
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:26:1: line contains only whitespace
    utils/prometheus-node-exporter-lua/files/usr/lib/lua/prometheus-collectors/hostapd_ubus_stations.lua:54:1: line contains only whitespace

edit: aside from the metric stuff, the other warnings seem legit.

@karlp
Copy link
Contributor

karlp commented Oct 11, 2020

The warnings are not legitmate, as they're making assumptions that this is a internal project that should be bound by internal style guidelines. It's purely an implementation detail that this package has the code within the package. Style of this package should be completely in the hands of the package maintainers.

@champtar
Copy link
Member

@neheb is there a warning level instead of pass or fail for the linter ?

@neheb
Copy link
Contributor

neheb commented Oct 11, 2020

That's a question for @aparcar . I've suggested an allow to fail option for the linter.

@PolynomialDivision
Copy link
Member Author

edit: aside from the metric stuff, the other warnings seem legit.

Whups. Sry. I thought this would only be the metric warning. I will reduce line size. I have to learn to read more carefully. ;)

@PolynomialDivision PolynomialDivision force-pushed the add-hostapd-ubus-exporter branch 2 times, most recently from 87662be to c52f61c Compare October 12, 2020 17:47
There is already the hostapd_stations exporter, which uses
hostapd-utils (more precisely hostapd-cli) to get client statistics.
However, the ubus interface is permanently integrated under hostapd
in OpenWrt. So this exporter needs one dependency less.

For now it exports mainly the rrm statistics. Many people are
interested in what your device supports. The exporter provides
information about the radio-resource-managment extensions.

Signed-off-by: Nick Hainke <[email protected]>
@PolynomialDivision PolynomialDivision force-pushed the add-hostapd-ubus-exporter branch from c52f61c to d34d788 Compare October 12, 2020 17:51
@PolynomialDivision
Copy link
Member Author

@neheb Fixed the lint warnings. ;) Merge?

@aparcar
Copy link
Member

aparcar commented Oct 12, 2020

LGTM

@neheb neheb merged commit 182ee3f into openwrt:master Oct 12, 2020
@PolynomialDivision
Copy link
Member Author

Do you want to look at lua bitop first ?
#9707 (comment)

It should be s/bit32/bit/g and.... done. You can even keep calling the local bit32 and just change the require to

local bit32 = require "bit"

and you should be good to go.

I thought I did this. :/
Installed the exporter on a clean system again and the lib was missing... Then I saw I forgot to change exactly that line.
#13807

@PolynomialDivision PolynomialDivision deleted the add-hostapd-ubus-exporter branch January 20, 2022 19:27
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.

5 participants