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

Add logging of user downloads #432

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

kebelig
Copy link

@kebelig kebelig commented Nov 29, 2023

In our environment, it is necessary to log user downloads to determine which files are actually needed. I have extended the logging here to include the "context" parameter, utilizing the Monologger by default. Furthermore, logging of user downloads can now be activated via the "log_downloads" flag in the configuration.php file.

Preview of logs:

[2023-11-29 13:36:22] default.INFO: User download started {"type":"file","filepath":"/.gitignore","filename":".gitignore"} []

Expands logging to include the context parameter and implements logging for user downloads
@codecov-commenter
Copy link

Codecov Report

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

❗ No coverage uploaded for pull request base (master@7f33cc8). Click here to learn what that means.

Files Patch % Lines
backend/Controllers/DownloadController.php 66.66% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #432   +/-   ##
=========================================
  Coverage          ?   99.69%           
  Complexity        ?      381           
=========================================
  Files             ?       25           
  Lines             ?      975           
  Branches          ?        0           
=========================================
  Hits              ?      972           
  Misses            ?        3           
  Partials          ?        0           

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

@alcalbg
Copy link
Member

alcalbg commented Nov 30, 2023

Hi,

Thanks for this.

It looks good, but it could be a braking change for someone using a custom log implementation.

I'm not sure how to handle this. New branch? New major release (v7->v8)? Just note this BC in changelog like before?

@kebelig
Copy link
Author

kebelig commented Dec 2, 2023

Hi @alcalbg ,

the Monologger handlers are compatible. But yes, there is a problem with custom handlers, thats right.
Maybe a new major release with notes is the best way, so anybody could extend their custom integrations.
In our environments we use always the context parameter and send it to gralog logging server (via Monolog handler).

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

Successfully merging this pull request may close these issues.

None yet

4 participants