luci-mod-status: display log_file in status if defined

[Chris1189]

Commit includes wrapper syslog in /usr/libexec. If a log file is configured, the output of this file is displayed. Otherwise the output of logread is displayed.

[systemcrash]

Is it certain that the wrapper script passes the same exit codes as logread?

In general this is a good idea, although I think other devs would want to see the wrapper go in to a different repo. I don't think there's any real merit in doing so, since the GUI is the only place it gets leveraged here.

[Chris1189]

Is it certain that the wrapper script passes the same exit codes as logread?

In general this is a good idea, although I think other devs would want to see the wrapper go in to a different repo. I don't think there's any real merit in doing so, since the GUI is the only place it gets leveraged here.

This is a good point and I checked that there are the same return codes.
If there is the need to move the wrapper somewhere else, I will not obstruct that.

[Chris1189]

I made the necessary changes, @systemcrash
The exit codes are identical to logread.

I am looking forward to your review.
Thank you!

[systemcrash]

It looks like you excised the ternary operator above - which more than anything looks like a sanity check in case stat[n] is null. Maybe a null check and handling isn't bad here after all. We would already be in some weird place because to arrive at this state implies that the wrapper is absent for some reason. I guess it's no worse anyway, because the old 'resolves' had null as a default. What does exec_direct do in that case?

[Chris1189]

I could also bring back the ternary operator
var logger = stat[0] ? stat[0].path : null;,
but as you already said it would be a weird place and the Promise already returns null in that case.

From the docs:
exec_direct executes the command while bypassing ubus with cgi-io helper applet at `cgi-bin/cgi-exec'. It is useful to fetch large command outputs, which may exceed the ubus message size limits. It also enforces the same access permission rules as the ubus base exec call.

[systemcrash]

Merged. Thanks @Chris1189

[stokito]

Sorry, that missed review of the PR. The logread command is the only one correct way to request logs. The command gets logs from the in-memory ring buffer of logd.
The syslog-ng is another more advanced log collection daemon that writes logs to a plain file.
Some router manufactures e.g. Turris use it instead of the default OpenWrt logd.
So the logread command doesn't have and ability to fetch logs from the logd.

Instead the syslog-ng created a shell script wrapper that reads logs from the log file. The script has a different path that's why in the LogView we need to detect which logread is installed.

I made a PR to the syslog-ng that will change the symlink the logread script once the syslog-ng is installed. Then the command will be the same whatever the logread implementation is in use and the LogView can be simplified.

Your implementation goes even further and it tries to read a log file itself instead of asking the logread command to do this.

	log_file="$(uci_get system @system[0] log_file "")"
	if [ -f "$log_file" ]; then
		cat "$log_file"
		return
	fi

You don't pass the -e flag to filter i.e. you should use grep -E "${pattern}" "${logfile}" as the logread script does. Basically you reinvented the existing logread script.
This is a breaking change because a filter doesn't work anymore.

As far I understood you wish to receive all the saved logs, not just those that the logd has in a ring buffer.
The logread command has an option -F <file> Log file.
So we may specify the log file instead to achieve same behavior. Maybe this should solved with some additional flag or time range i.g. if there are no logs in a ring buffer for the time then load logs from a file.

Here are sources of the logread https://github.com/openwrt/ubox/blob/master/log/logread.c#L148

Also check the OpenWrt Wiki: Logging messages page for some details.

So for now I would recommend to revert the commit (since it has breaking changes), merge the PR with alternatives, then simplify the logic here (maybe later).

But the archival log reading from a file should be decided separately. Do we really need the feature?
We can make a workaround by specifying a log file but anyway it would be good to discuss with the logread developers.

[systemcrash]

@stokito we determined that -F is a write only parameter, not read. Ie destination file.

is there a good way to reconcile these disparities?

[stokito]

oh, yes it's used to redirect logs to the file.
The /etc/init.d/log script will start the /sbin/logread -f -F /var/log/mylog -p /var/run/log command to dump messages from a ring buffer into the file.
The question remains how to read from the file and do we need this at all. This may be useful to debug problems after reboot. Honestly, this kind of troubleshooting is anyway better to perform manually from command line and then disable the logging to a file to avoid erasing of a NAND flash.
Unless I missed something I think it would be better to revert the commit.

[systemcrash]

Perhaps it’s time for an option to read from a specific file. So the only problem is log filtering…? The -e option only ever filters for line start. So unless the log pages get some filter option, what’s the win? (Js can filter by itself without relying on underlying binaries).

[jow-]

I agree with @stokito here, don't implement parallel infrastructure in the gui, don't attempt to read log files directly when cli level commands exist for that purpose.

[systemcrash]

Is that what you assess has happened here? The script still leverages logread. But until logread itself gains a parameter to read from other log files, the wrapper script uses cat.

[jow-]

Is that what you assess has happened here?

What happens here is that LuCI starts implementing log related infrastructure code, this belongs into base first.

[Chris1189]

As @systemcrash mentioned logread only puts log messages to a specific file with -F.
In /etc/config/system the option log_file redirects log messages to the set file. We wanted to display contents of that file because this way we can see log messages from before boot.

From the messages you wrote to the discussion I gathered that you wish to add the filter option -e from logread to the syslog-wrapper?

My intent was not to implement parallel infrastructure but to add a feature. I want to read the file if it is set.