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

[Bug] Logreader warns about log_type to be syslog #979

Open
Antreesy opened this issue Aug 22, 2023 · 8 comments · May be fixed by #1041
Open

[Bug] Logreader warns about log_type to be syslog #979

Antreesy opened this issue Aug 22, 2023 · 8 comments · May be fixed by #1041

Comments

@Antreesy
Copy link
Collaborator

Steps to reproduce

  1. configure 'log_type' to be 'syslog',
  2. wait for warning

Expected behaviour

This is one of the defined ways to log system messages, so everything should be fine

Actual behaviour

Visible message:
Information Logreader File-based logging must be enabled to access logs from the Web UI. Your log_typeis currently set to: [syslog]. If you feel this is an error, please verifylog_type in your config.php and check the Nextcloud Administration Manual. This is not an actual log entry.

@joshtrichards
Copy link
Member

Hi @Antreesy - What's your concern? It's not an error or even a warning. It's just informational. The alternative is to have nothing displayed.

Context: The logreader app can't access NC logs if they're sent to the system syslog. When using syslog, your logging destination (and underlying storage) is driven by your syslog daemon configuration. Sometimes the logs won't even be retained on-server if centralized logging is in-use. Same goes for systemd logging.

@Antreesy
Copy link
Collaborator Author

Context: The logreader app can't access NC logs if they're sent to the system syslog.

Hi @joshtrichards , thanks for the quick reply. Got this from the customer ticket, who is claiming, that it was possible to use syslog and still use a logreader app to observe them before Nc27. Did i misunderstood one of you, probably?

@joshtrichards
Copy link
Member

No way that was possible that I can think of. Until recently (i.e. NC27) the logreader app hung when opened if syslog was configured. :) See #866.

@joshtrichards
Copy link
Member

Though now you've got me wondering if we previously permitted a config like this:

"log_type" => "syslog",
"syslog_tag" => "Nextcloud",
"logfile" => "nextcloud.log",
"loglevel" => 2,

...and logged simultaneously to both channels (file+syslog) just because the logfile was configured. 🤔 If we did it was probably accidental.

It is also possible that previously if there happened to be a nextcloud.log on-disk (maybe an old one) that it would load that one even if syslog was configured. But if that was the case it just means "using logreader" under that scenario would have been to look at outdated logs, not current logs.

@Antreesy
Copy link
Collaborator Author

Antreesy commented Aug 22, 2023

See #866.

Thanks for providing a link, I have already forgot about it, and now it returns to me 🙈 🪃

Then I'll ask for configuration, to see if that was a reason.

@Antreesy
Copy link
Collaborator Author

Antreesy commented Aug 23, 2023

So, customer used log_type: syslog all the time, but syslog was configured to write the messages to the file path where log_type: file would store it, so, default nextcloud.log file. Therefore the messages were shown up in Web-UI.

Here an excerpt from rsyslog.conf
if  ($programname == 'Nextcloud')
then {
##A template that resembles traditional syslogd file output:
##$template TraditionalFormat,"%timegenerated% %HOSTNAME%
##%syslogtag%%msg:::drop-last-lf%\n"
#
 $template nude,"%msg:::drop-last-lf%\n"
 -/opt/nextcloud/wolke/nextcloud.log;nude
 @fbhsyslog
 stop
}

I guess, it should work then, as before, only if there weren't any changes, that block this comfiguration?

@joshtrichards
Copy link
Member

joshtrichards commented Aug 23, 2023

That really should have never worked, but here we are. Hah. Okay, at least now it makes sense.

Setting their log_type to file would fix this, but they're replicating their logs off-machine via rsylog too (I think that's what the @fbhsyslog is doing). So presumably that's not an option.

Hmm. I guess we could change the logic to something like:

if ($log_type === "file" || ($logfile !== "" && file_exists($logfile))

...or something along those lines.

This would still accomplish the main goal of the original PR, but the side effect would be we'd still load misleading old log files for people that happen to have logfile configured even though they're using syslog/systemd. And that could get confusing.

We then also can't add a warning "may be old logs you're viewing" because that would impact people who are doing this intentionally... like this user.

Another possibility might be if rsyslog supports input from pipes or, perhaps, replication from a file-based log. 🤔 Then they'd do this on their own outside the scope of NC.

Bigger picture: Maybe adding a new log_type (or making it an array permitting multiple simultaneous log_types) would be a cleaner approach. But we have to be careful - and that is outside the scope of the logreader because that gets into territory that entails changes in core. The logging has to be very reliable and lightweight to not create problems throughout NC.

Just brainstorming. No commitments being made here. Others will have to weigh in.

@susnux
Copy link
Contributor

susnux commented Nov 9, 2023

Maybe we can even support all logging types by no longer depend on nextcloud's log but listen to OCP\Log\BeforeMessageLoggedEvent and create our own one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants