Giter Club home page Giter Club logo

Comments (23)

Maikuolan avatar Maikuolan commented on June 18, 2024 2

So, just to let others know: I've started working on this feature now. I was hoping to get it finished by today, but didn't end up having as much time to work on it as I'd anticipated. Almost there though; If all goes well, I should hopefully have it working by tomorrow at some point. Ill reply again once I've got it committed.

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024 2

Done and implemented. :-)

The way I've coded it, it should currently be possible to specify an arbitrary log directory, or just store logs directly in the root of vault directory, and either way, log rotation should work just fine (in theory, anyway; it's a new feature though, so as always, time will tell if any unexpected problems appear, and of course, if anyone encounters any problems, let me know about it).

The way log rotation determines whether a file is a log file or something else, is by checking whether the name of the log file matches the pattern/convention specified by the configuration directive corresponding to the type of log file being rotated, and should also be capable of stepping through and handling dedicated log file directories as per necessary (so, as long as nobody starts giving log files PHP extensions, copying the names of unrelated files for their log files, or engaging in other similarly bad practices, it shouldn't cause any problems, I think).

The way log rotation determines which log files to act upon when rotating log files is by checking the last modified date of the files in question, acting upon the "oldest" (i.e., least recently modified) files until the "specified limit" (i.e., maximum permitted number of log files as per the log rotation limit directive) isn't exceeded anymore.

Current available actions are "delete" (deletes the files being acted upon) and "archive" (firstly creates GZ-compressed copies of the files being acted upon, then deletes the original files). More actions can be added, pending any suggestions anyone might have..? Anyhow, let me know if you've got any.

I'll keep this issue open for another day or so, pending any unexpected discoveries, related suggestions, etc, and then close it afterwards. Marking issue as "implemented".

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024 1

It's definitely a nice idea (IMO).

I've been trying to think about the best way to go about actually implementing this during my spare time over the past few days. Currently, the two best approaches I've thought of are:

  1. Having CIDRAM create a record in the cache whenever a new logfile is created. Whenever the number of cached records relating to logfiles exceeds the specified limit, have it delete the files corresponding to the oldest entries, until it no longer exceeds the specified limit.

  2. Having CIDRAM periodically scan the contents of the vault to detect potential logfiles, and when the total number found exceeds the specified limit, have it delete the oldest files (i.e., least recently modified files) found until it no longer exceeds the specified limit.

Both methods would probably work okay. The main downside of option 1 is that it might skip a beat if people manually delete the logfiles themselves, or reupload old logfiles or things like that, due to creating inconsistencies with the cache records. I doubt that'd happen too often though. Of course, having cache records at all is somewhat of a redundancy, if we don't need to have them in the first place (as per option 2). The main downside of option 2 is that some simple conditions would need to be met in order to determine whether a file is a logfile or something else (and because we obviously don't want to be deleting the wrong files by mistake; e.g., checking whether files are using common extensions, like txt, or log, or having "logfile" in the name and things like that), all of which is fine.. except that, in order for the process to work correctly (i.e., for the old logfiles to be deleted), users would need their logfiles to conform to whatever conventions are expected by the conditions to be met (e.g., if I named my logfiles in such a way that the expected conditions aren't met, it mightn't be recognised as a logfile).

None of these are really significant problems, of course; Just sharing my thought process on it a bit. '^.^

Ideas from others are invited too, of course. In any case.. If there aren't any objections from anyone, I could make a start on this at some point over the next few days and have it ready for a consequent update. :-)

from cidram.

harryqt avatar harryqt commented on June 18, 2024 1

I would say no 2. Just a opinion.

from cidram.

soumsps avatar soumsps commented on June 18, 2024 1

Having CIDRAM periodically scan the contents of the vault to detect potential logfiles, and when the total number found exceeds the specified limit, have it delete the oldest files (i.e., least recently modified files) found until it no longer exceeds the specified limit.

I feel this would be a better approach. @Maikuolan I think downsides of option 2 can be solved easily by assigning a log folder by default and you should make it non-optional. And log rotation checker should see only this folder for any old log files.

-- Downside: Old version of CIDRAM might have created log files in /vault folder or any other user-specified folder. How would CIDRAM clean those log files on update installation? This is also a downside of Option 1.

e.g., checking whether files are using common extensions, like txt, or log, or having "logfile" in the name and things like that)

This solution will complicate the usage I think.

from cidram.

DanielRuf avatar DanielRuf commented on June 18, 2024 1

https://github.com/Seldaek/monolog/blob/master/src/Monolog/Handler/RotatingFileHandler.php

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024 1

Monolog's approach has some pretty good ideas. It looks like they've got similar considerations there too, like how in order for log rotation to work correctly, the filenames for logs must conform to specific formats.

and you should make it non-optional.

Enforcing that users use a specific directory for logs would work around the problem of filenames for logs needing to conform to specific formats, and is a good idea, but a reduction in configurability, whether by removing an existing configuration directive entirely, or by simply changing the nature of an existing configuration directive as such that some previously optional aspect of it becomes mandatory, would technically be a backwards-incompatible change (albeit perhaps not a huge or paradigm-shifting kind of change, but still backwards-incompatible nonetheless). As I'm hoping that CIDRAM remains in conformity to semver as closely as possible, any planned backwards-incompatible changes would need to be earmarked for a future CIDRAM v2.0.0 release (as opposed to a potential upcoming v1.x.x releases). In short: I wouldn't want to make a dedicated logs directory mandatory for anything earlier than the next major version release (v2.0.0).

That said though.. The idea is still viable. We could always basically just implement the entirety of the idea stopping short only at making the directory mandatory (i.e., when a limit for the number of logs is specified, and when that limit is exceeded, check whether a predetermined directory exists and contains logs, and if so, delete the oldest logs from that directory until the limit isn't exceeded anymore), but then include a note in the documentation for it, that in order for log rotation to work correctly, that this predetermined directory must be used (with the specified limit basically doing nothing is this predetermined directory isn't used). Also, keeping all logs in the same directory could help users with "house cleaning" their installation, keeping it all looking more organised and such. Though of course, it could also be argued that this would be merely shifting the problem of needing to conform to specific filenames, to needing to conform to specific directory usage (which in either case, is not some kind of horrible or disastrous thing at all, but would nonetheless require us to "pick our poison" per se, in the sense of whether we'd prefer to be conforming to specific filenames or specific directories).

(I'm also strongly leaning away from "option 1" at this point; I anticipate potentially lots of people asking "why doesn't X thing work" in the future and such if we were to rely on the cache, so definitely, "option 2" is better than "option 1", I think).

So.. Yeah. I don't think the process of implementing log rotation will be difficult at all (i.e., the coding itself, subsequent testing and so on).. but the process of planning it might be a little trickier (just deciding on this thing over that thing, this method over that method and so on). The "best" way, I think, would probably be whatever way is closest to some kind of consensus of preferences. I don't really have much personal preference on this myself, and could happily work with whatever.. but maybe I should ask around, to gage how other users would feel about it..? '^.^

from cidram.

 avatar commented on June 18, 2024 1

As a friend of structure, I would say these formats for plain text, apache format accordingly:
log/cidram-{yyyy}-{mm}.txt
log/cidram-{yyyy}-{mm}.log

For serialized output the same but with proper ending for the serialization format. As I don't use it I don't remember which format CIDRAM outputs ;)

So in plain terms, a "log" subdirectory to the vault and files that have 4-digit year and 2-digit month notation which ends with the common extension.

Of course this is assuming a monthly rotation.

from cidram.

soumsps avatar soumsps commented on June 18, 2024 1

In CIDRAM we have multiple types of log files i.e general log, recaptcha log et cectra.

If I set log rotation value to 7, then that means I would like to have 7 log of each type. No matter if that log was created hourly, daily or monthly basis @Maikuolan are you planning the same way or there is some difference??

it won't be too much for users, or confuse them too much, having all the different directives.

I agree 👍

more directives being available, but there does eventually reach a point where it becomes too much.

For now, this far future issue should be ignored, and with CIDRAM V2.0.0 this could be solved :)

from cidram.

harryqt avatar harryqt commented on June 18, 2024 1

@Maikuolan Thank you so much. Great work. 👍 Updated and testing.. 👌

from cidram.

soumsps avatar soumsps commented on June 18, 2024 1

Thank you @Maikuolan . Updated, but found an issue don't know if it is related with the update or not..

Issue: Normally I used to keep General log and Recaptcha log.. but today when I was seeing logs from CIDRAM frontend, there was no recaptcha log (i am sure there were atleast 10 days log present before).

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024 1

Issue: Normally I used to keep General log and Recaptcha log.. but today when I was seeing logs from CIDRAM frontend, there was no recaptcha log (i am sure there were atleast 10 days log present before).

Good catch, and thanks. Some things I missed that needed to be updated in the reCAPTCHA module. I'll get that sorted now. 👍

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024 1

Done. :-)

from cidram.

harryqt avatar harryqt commented on June 18, 2024 1

faster than the Flash! 😗

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024 1

Figured out why they weren't showing in the front-end logs page; Latest commit should fix that. Not yet sure why they aren't rotating properly though.

from cidram.

soumsps avatar soumsps commented on June 18, 2024 1

Both problem solved.. 👍

log rotation not working on recaptcha log

I think 24hr cycle was not complete or new log was not created so that log rotation could be fired up.. after I updated CIDRAM 2nd last update Some things I missed that needed to be updated in the reCAPTCHA module.

Today everything is working fine :)

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024

Out of curiousity.. How would you all approach logs personally, in what you'd consider to be an "ideal" setup (preferred filenames, extensions, directory placement, whether directly in the vault, or in some other dedicated directory and such)?

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024

Looks good. :-)

Asked around a bit, and seems most people (that use subdirectories) use some variation of log, or logs, AFAICT, so either of those would be fairly safe bets to use as a "predefined" directory.

Another possible idea though: I could always add an additional configuration directive, to allow people to "predefine" their own logs directory (possibly defaulting to log or logs). This would allow more control in the sense of where to store logs and such, and in how log rotation could work. But, the negatives of that, is that (1) it'd mean there'd be at least two newly added configuration directives for log rotation (the directory to use, and the threshold/limit), as opposed to just the one (the threshold/limit). With five directives for logs (logfile, logfileApache, logfileSerialized, truncate, FrontEndLog) already existing, an extra two would mean.. seven. However we go for log rotation, in any case, there'll probably be enough directives to justify earmarking splitting them off to their own configuration category for a future CIDRAM v2.0.0 release (whenever that happens; no specific plans or deadlines yet), but I hope that in any case, it won't be too much for users, or confuse them too much, having all the different directives. It's normal, I think, that aiming to give more control to users would result in more directives being available, but there does eventually reach a point where it becomes too much. I don't think we're too close to being there yet..? But it's something to keep at the back of one's mind, I guess. And (2), the risk, generally, of confusing users (like if they think specifying the directory in this newly added directive could do other things, like determine where logs are placed in its own right, which is actually determined by the already existing directives; mostly resolvable with clear documentation and instructions, but some people don't read properly, or might just get confused because it isn't what they expect; things like that). ¯\_(ツ)_/¯

Of course, if users were allowed to specify their own predefined logs directory via such a configuration directive, some common sense would need to be applied in the code (sanity check..? security check..? etc) to ensure ideally that what they specify is a subdirectory within their vault (so not the vault itself, or something higher up, or worse; things like ../../ or .../; all of which could result in the wrong things being deleted by log rotation, if CHMOD permissions or the likes thereof allow for it). That's easy to implement though.

Thoughts (regarding the two directives, specifying one's own directory for log rotation, etc)?

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024

are you planning the same way or there is some difference??

Planning the same way; no difference. :-)

For now, this far future issue should be ignored, and with CIDRAM V2.0.0 this could be solved :)

True, true. Still.. Worth mentioning it anyhow, I guess. ^.^

from cidram.

soumsps avatar soumsps commented on June 18, 2024

@Maikuolan still recaptcha log not generated :( I guess it get generated in real-time?

I am seeing same error on another site. where i did not updated the log rotation version.

Update :

  1. log rotation not working on recaptcha
  2. recaptcha log is generated but is not showing in frontend

I have set log rotation to 7 and delete previous
What I see in frontend :
screenshot_2

What I see with file manager :
screenshot_1

recaptcha log older than 7 days were not deleted.

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024

Hm.. Strange. :-/

At work at the moment, but I'll take a closer look at it again when I get home a little later today.

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024

Ah, ok. Awesome! 😊 👍

from cidram.

Maikuolan avatar Maikuolan commented on June 18, 2024

Seems to all be good at this point AFAICT. Closing as implemented/resolved. :-)

from cidram.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.