[change] Turn netjson_monitoring into proper shell tool

[openwisp]

[change] Turn netjson_monitoring into proper shell tool

[nemesifier]

@nemesisdesign The ubus uses for this the option -S

root@st-dev-07 /etc # ubus
Usage: ubus [<options>] <command> [arguments...]
Options:
 -s <socket>:           Set the unix domain socket to connect to
 -t <timeout>:          Set the timeout (in seconds) for a command to complete
 -S:                    Use simplified output (for scripts)
 -v:                    More verbose output
 -m <type>:             (for monitor): include a specific message type
                        (can be used more than once)
 -M <r|t>               (for monitor): only capture received or transmitted traffic

Commands:
 - list [<path>]                        List objects
 - call <path> <method> [<message>]     Call an object method
 - subscribe <path> [<path>...] Subscribe to object(s) notifications
 - listen [<path>...]                   Listen for events
 - send <type> [<message>]              Send an event
 - wait_for <object> [<object>...]      Wait for multiple objects to appear on ubus
 - monitor                              Monitor ubus traffic

ok, so the opposite, show pretty output by default and simplified with an option, that works for me.
@devkapilbansal are you keen to help out with these improvements?

[nemesifier]

Ok, should be easy, is this part of the system path or would we have to call the script as /usr/libexec/netjson_monitoring?
Being able to call netjson_monitoring as a command is handy if anyone wants to use NetJSON in their applications.

[feckert]

Ok, should be easy, is this part of the system path or would we have to call the script as /usr/libexec/netjson_monitoring?

The directory is not in the system PATH.
But this is a lua script with has no help.
From my point of view only script should end up in the PATH which are also intended for the user.
I have also seen this with openwisp-config which is also not so good.

Being able to call netjson_monitoring as a command is handy if anyone wants to use NetJSON in their applications.

You could add an option to openwisp-monitoring to call only the netjson llua script and then return the output and exit
https://github.com/openwisp/openwrt-openwisp-monitoring/blob/gsoc21/openwrt-openwisp-monitoring/files/monitoring.agent

[devkapilbansal]

If I am not wrong, we created two packages instead of one because some people only want netjson-monitoring for their use case. If we need to add an option in openwisp-monitoring itself, then what's the purpose of having two different packages?

[feckert]

@devkapilbansal That is not what I meant.
The lua script netjson-monitoring is only called by openwisp-monitoring, because this is a helper script.
The command openwisp-monitoring is the entry command for this stuff.
So my suggestion is to move netjson-monitoring to the following location /usr/libexec/openwisp-monitoring/ so this does net called directly because /usr/libexec/openwips-monitoring path is not in the shell $PATH variable.
The reason is that no one knows what netjson-monitoring does if your are not familiar with openwisp.
And it does not have a help text and is only called by openwisp-monitoring.

If you want to get the output from netjson-monitoring you could extend openwisp-monitoring shell script with an option for example openwisp-monitoring --dump. As I said, the main reason is that we do not polluting the shell $PATH with helper scripts.

If you would right the programm in C you would have only openwisp-monitoring and not a helper script like netjson-monitoring.

[nemesifier]

@devkapilbansal That is not what I meant.
The lua script netjson-monitoring is only called by openwisp-monitoring, because this is a helper script.
The command openwisp-monitoring is the entry command for this stuff.
So my suggestion is to move netjson-monitoring to the following location /usr/libexec/openwisp-monitoring/ so this does net called directly because /usr/libexec/openwips-monitoring path is not in the shell $PATH variable.
The reason is that no one knows what netjson-monitoring does if your are not familiar with openwisp.
And it does not have a help text and is only called by openwisp-monitoring.

If you want to get the output from netjson-monitoring you could extend openwisp-monitoring shell script with an option for example openwisp-monitoring --dump. As I said, the main reason is that we do not polluting the shell $PATH with helper scripts.

If you would right the programm in C you would have only openwisp-monitoring and not a helper script like netjson-monitoring.

@feckert one of the goals of the project is to make the netjson_monitoring available as a NetJSON DeviceMonitoring implementation. Nobody knows now but the goal is to make it available and some people may use it.

Therefore this program should be kept the way it is. OpenWISP Monitoring is strictly related to OpenWISP, the "NetJSON DeviceMonitoring" format can be used independently from OpenWISP.

I hope this clarifies.

[feckert]

@nemesisdesign Well, if that is the case, then we should add the following changes to netjson-monitoring to be a full shell command.

• When calling the command without arguments, the help text should be displayed (this is normal the case for shell commands)
• When called with the --help or -h option, the help text should also be displayed.
• When calling with the option --dump the output should be dumped in json

You could also consider adding an option to --dump that it should be well formatted (human readable) or simply a line (which is then quickly readable for a following script to read the json, since no new lines are included).

Translated with www.DeepL.com/Translator (free version)

[nemesifier]

@nemesisdesign Well, if that is the case, then we should add the following changes to netjson-monitoring to be a full shell command.

• When calling the command without arguments, the help text should be displayed (this is normal the case for shell commands)
• When called with the --help or -h option, the help text should also be displayed.
• When calling with the option --dump the output should be dumped in json

You could also consider adding an option to --dump that it should be well formatted (human readable) or simply a line (which is then quickly readable for a following script to read the json, since no new lines are included).

Maybe you meant pretty, readable or something similar?

Ok, let's do these changes, I'll update the issue description.

[feckert]

@nemesisdesign The ubus uses for this the option -S

root@st-dev-07 /etc # ubus
Usage: ubus [<options>] <command> [arguments...]
Options:
 -s <socket>:           Set the unix domain socket to connect to
 -t <timeout>:          Set the timeout (in seconds) for a command to complete
 -S:                    Use simplified output (for scripts)
 -v:                    More verbose output
 -m <type>:             (for monitor): include a specific message type
                        (can be used more than once)
 -M <r|t>               (for monitor): only capture received or transmitted traffic

Commands:
 - list [<path>]                        List objects
 - call <path> <method> [<message>]     Call an object method
 - subscribe <path> [<path>...] Subscribe to object(s) notifications
 - listen [<path>...]                   Listen for events
 - send <type> [<message>]              Send an event
 - wait_for <object> [<object>...]      Wait for multiple objects to appear on ubus
 - monitor                              Monitor ubus traffic

[devkapilbansal]

ok, so the opposite, show pretty output by default and simplified with an option, that works for me.
@devkapilbansal are you keen to help out with these improvements?

Sure, I will look into it.

Here, what is difference between pretty and simplified?
Also, what we are supposed to do? Like, to move netjson-monitoring to /usr/libexec/netjson_monitoring and call it via a script located at /usr/sbin or make changes in netjson-monitoring directly.

[nemesifier]

ok, so the opposite, show pretty output by default and simplified with an option, that works for me.
@devkapilbansal are you keen to help out with these improvements?

Sure, I will look into it.

Here, what is difference between pretty and simplified?
Also, what we are supposed to do? Like, to move netjson-monitoring to /usr/libexec/netjson_monitoring and call it via a script located at /usr/sbin or make changes in netjson-monitoring directly.

My understanding is that we can leave it where it is now but add the help text to it as @feckert suggested.
He suggests to make sure the output is indented by default (pretty) but can be spit out as "simplified" for script usage (as ubus does), so we'll need to edit the code of the openwisp monitoring agent to use the simplified/script version.

The only thing in which I'd differ from ubus, is that for the JSON output I recommend using 4 spaces, which is the most readable option. Ubus uses tabs and I find its JSON output not greatly readable.

[nemesifier]

@feckert @devkapilbansal If I recall correctly, lua-cjson does not allow to pretty print the output.
In order to not make the package bigger, I think we should not go ahead with this idea of the pretty print, the extra space is not worth an additional library.