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

feat: Allow to show logs of all logging types by proxying messages #1041

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Nov 11, 2023

This creates an internal log file for all non file loggers, this enables showing all kind of logs.
The internal log file is then rotated by a background job, I think rotation makes more sense to keep the behavior similar to the file logger log file.
Maybe we can add a feature in the future to read also rotated logs (Nextcloud & internal log).

TODO

  • Log rotation / truncating is required otherwise the log would grow infinitely.
  • Adjust unit tests

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Attention: 39 lines in your changes are missing coverage. Please review.

Comparison is base (29d0240) 100.00% compared to head (ea032e8) 27.78%.

Additional details and impacted files
@@              Coverage Diff               @@
##              master    #1041       +/-   ##
==============================================
- Coverage     100.00%   27.78%   -72.22%     
- Complexity         0      188      +188     
==============================================
  Files             10       18        +8     
  Lines            168      565      +397     
  Branches          31        0       -31     
==============================================
- Hits             168      157       -11     
- Misses             0      408      +408     
Flag Coverage Δ
unit 27.78% <42.64%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/Controller/LogController.php 0.00% <ø> (ø)
lib/Log/LogProxy.php 100.00% <100.00%> (ø)
lib/Service/SettingsService.php 76.00% <ø> (ø)
lib/Listener/LogListener.php 43.75% <60.00%> (ø)
lib/BackgroundJobs/Rotate.php 0.00% <0.00%> (ø)
lib/Log/LogIteratorFactory.php 0.00% <0.00%> (ø)

... and 22 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@susnux susnux force-pushed the feat/allow-listing-all-logs branch 7 times, most recently from 463a66a to b52244f Compare November 11, 2023 22:56
@susnux susnux marked this pull request as ready for review November 11, 2023 22:57
@susnux susnux force-pushed the feat/allow-listing-all-logs branch from b52244f to 5c13cc9 Compare November 11, 2023 22:58
…non-file based loggers

Also no longer provide the "enabled" setting as now all log types are supported

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Decided to rotate instead of truncate for future use with possible feature that also supports reading the rotated log file.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/allow-listing-all-logs branch from 5c13cc9 to ea032e8 Compare November 11, 2023 23:21
@blizzz blizzz added this to the Nextcloud 29 milestone Nov 23, 2023
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the reviewing process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR reviewing process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@susnux susnux modified the milestones: Nextcloud 29, Nextcloud 30 Mar 29, 2024
@kesselb
Copy link

kesselb commented May 25, 2024

I like the idea 👍

@come-nc
Copy link
Contributor

come-nc commented Nov 14, 2024

@susnux Would it not be possible to access syslog logs without duplicating them?
Are there other backends than file and syslog?

@kesselb
Copy link

kesselb commented Nov 14, 2024

Are there other backends than file and syslog?

@susnux
Copy link
Contributor Author

susnux commented Nov 14, 2024

Would it not be possible to access syslog logs without duplicating them?

syslog in every case I know not allowed to be read by PHP, and I think this makes sense to not read other apps output.

Basically what this is doing is doing it similar as tee, so you have your native log for e.g. other logging solutions or journald integration, but also be able to have this in Nextcloud for quick analysis.

@come-nc
Copy link
Contributor

come-nc commented Nov 14, 2024

Would it not be possible to access syslog logs without duplicating them?

syslog in every case I know not allowed to be read by PHP, and I think this makes sense to not read other apps output.

Basically what this is doing is doing it similar as tee, so you have your native log for e.g. other logging solutions or journald integration, but also be able to have this in Nextcloud for quick analysis.

But you are basically duplicating log, this is the same as supporting to have several log backend active at once, to have both files and syslog, no?

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

Successfully merging this pull request may close these issues.

[Bug] Logreader warns about log_type to be syslog
4 participants