-
Notifications
You must be signed in to change notification settings - Fork 811
net/vnstat: dashboard widget #5336
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -89,6 +89,42 @@ public function yearlyAction() | |||||
| return array("response" => $response); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * list interfaces tracked by vnstat | ||||||
| * @return array | ||||||
| */ | ||||||
| public function dbiflistAction() | ||||||
| { | ||||||
| $backend = new Backend(); | ||||||
| $response = trim($backend->configdRun("vnstat dbiflist")); | ||||||
| if (empty($response)) { | ||||||
| return array("interfaces" => array()); | ||||||
| } | ||||||
| $interfaces = array_map('trim', explode("\n", $response)); | ||||||
| $interfaces = array_values(array_filter($interfaces, function ($v) { | ||||||
| return !empty($v); | ||||||
| })); | ||||||
|
Comment on lines
+103
to
+106
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just how unreliable is the dbiflist output?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i went back and forth on this, whether to use dbiflist vs enumerated opnsense interfaces. if you previously had different interfaces and replaced your network card, the vnstat database would contain interfaces that don't intersect the current opnsense interfaces. does that matter? is that a concern?
i am open to any suggestions / recommendations here
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you dont know what exists or what users want to display, you can make it configurable. Here an example from the certificate widget in core:
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry for not responding, I simply don't know anything about vnstat, it would be best if someone with experience here reviews this, e.g., the maintainer or a power user of the plugin. |
||||||
| return array("interfaces" => $interfaces); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * retrieve vnstat data as structured JSON for a specific interface | ||||||
| * @return array | ||||||
| */ | ||||||
| public function jsonAction() | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this action name is too generic |
||||||
| { | ||||||
| $iface = $this->request->get('iface'); | ||||||
| if (empty($iface) || !preg_match('/^[a-zA-Z0-9_.]+$/', $iface)) { | ||||||
| return array("error" => "Invalid or missing interface name"); | ||||||
| } | ||||||
|
Comment on lines
+117
to
+119
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems spurious.. it's either an interface or not. even better to test the error when asking for garbage input (as an empty string can also be passed safely) |
||||||
| $backend = new Backend(); | ||||||
| $response = json_decode($backend->configdRun("vnstat json " . escapeshellarg($iface)), true); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
should do the same but canonically |
||||||
| if ($response === null) { | ||||||
| return array("error" => "Failed to retrieve vnstat data"); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. normally we use ['status' => 'failed'] everywhere. the error message is seldomly shown and not even translated |
||||||
| } | ||||||
| return $response; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * remove database folder | ||||||
| * @return array | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,18 @@ parameters: | |
| type:script_output | ||
| message:request Vnstat yearly status | ||
|
|
||
| [dbiflist] | ||
| command:/usr/local/bin/vnstat --dbiflist 1 | ||
| parameters: | ||
| type:script_output | ||
| message:request Vnstat interface list | ||
|
|
||
| [json] | ||
| command:/usr/local/bin/vnstat --json --iface %s | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the %s here looks misplaced? |
||
| parameters:%s | ||
| type:script_output | ||
| message:request Vnstat json status | ||
|
|
||
| [resetdb] | ||
| command:rm -rf /var/lib/vnstat | ||
| parameters: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| <metadata> | ||
| <vnstat> | ||
| <filename>Vnstat.js</filename> | ||
| <endpoints> | ||
| <endpoint>/api/vnstat/service/dbiflist</endpoint> | ||
| <endpoint>/api/vnstat/service/json</endpoint> | ||
| </endpoints> | ||
| <translations> | ||
| <title>Vnstat Traffic</title> | ||
| <h_date>Date</h_date> | ||
| <h_rx>RX</h_rx> | ||
| <h_tx>TX</h_tx> | ||
| <h_total>Total</h_total> | ||
| <h_avg_rate>Avg Rate</h_avg_rate> | ||
| <period_hourly>Hourly</period_hourly> | ||
| <period_daily>Daily</period_daily> | ||
| <period_monthly>Monthly</period_monthly> | ||
| <period_yearly>Yearly</period_yearly> | ||
| <msg_no_data>No vnstat data available</msg_no_data> | ||
| <excluded_interfaces>Excluded Interfaces</excluded_interfaces> | ||
| <refresh_interval>Refresh Interval (minutes)</refresh_interval> | ||
| <show_avg_rate>Show Average Rate</show_avg_rate> | ||
| </translations> | ||
| </vnstat> | ||
| </metadata> |


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.
would be nicer to set version 1.4 and remove the revision since this is a big enough change