serilog / serilog-sinks-file

Write Serilog events to files in text and JSON formats, optionally rolling on time or size
Apache License 2.0
333 stars 118 forks source link

Add Roll on specific DayOfWeek #325

Closed walterstypula closed 1 week ago

walterstypula commented 1 week ago

Is your feature request related to a problem? Please describe. I'd like to have the ability to roll the log file on a specific day of the week. Currently, log entry aggregation isn't too loose or too aggressive. The jump from a day to month is too large. 1) When day configuration is used we may encounter data loss if we do not keep enough rolling files. 2) When month configuration is used, large log file(s) maybe the result.

Describe the solution you'd like For any day, Sunday through Saturday, we want to select a specific day to aggregate all log entries into a specific date file for the chosen roll on day.

For example, if today is Monday, Jan 1, 2024 or Tuesday, Jan 2, 2024 and I want all logs aggregated on a Sunday. All logs from that Monday, Tuesday, etc. should be added to the file with Sunday's Date: 2023-12-31. When the today's date hits Sunday, 2024-01-07 a new log file should be created and all subsequent days should log entries into this new log file until the next roll on date.

Describe alternatives you've considered Logging by month results in too many log entries in a single file, Logging by day results in too many log files created and data loss.

Additional context Previous implementations tried to Aggregate by week, however, this doesn't work. These implementations did not fall to a specific date. A new log file would be created whenever the application was restarted. For example, if we had a daily deployment strategy it would create one log file for every day application was deployed.

bartelink commented 1 week ago

Hi - thanks for taking the time to describe your aims in detail, and the PR. Unfortunately this sort of complexity is being ruled out of this Sink repo for the moment, which is of course harsh, but even without being Nick, I value the directness and simplicity of the naming strategy as it stands - this Sink's download counts are off all charts, and being able to rule out random issues with naming algorithms is of immense value.

I would assume that repos forked from this would consider your proposition, given you're prepared to write comprehensive tests etc; though I'd recommend asking first before going to the trouble of doing the PR as ultimately the maintainer bears the responsibility and cost of carrying forward all features till the end of time...

Example similar well intended improvement being batted back on similar basis: https://github.com/serilog/serilog-sinks-file/pull/323#issuecomment-2384954555

walterstypula commented 1 week ago

@nblumhardt it appears this repo is not taking any new features/enhancements at this time. Could we have the README.md of this repo updated to indicate so?

bartelink commented 1 week ago

@walterstypula while I can appreciate this has been a frustrating experience for you, I think that's a step too far and mischaracterizes the process quite a bit:

Ultimately quality OSS relies on people making tough decisions like these in the broader interest.

The bottom line here is that this particular kind of feature is being pushed back on in this repo, and there are forked repos that have more lenient stances - it's just a matter of there being a different bar to entry in this one.

And if a judgement on something like this turns out wrong and is proven to actually be perfectly reasonable and viable elsewhere, then the answer is to say "in forked sink X, I added feature Y; I propose to add it here as follows , what do you think"?

walterstypula commented 1 week ago

@bartelink I completely understand the reasoning taken here and I agree with it, no frustration here.

I recommend the contribution outline above should be documented for this repo. This will provide better clarity for future contributors, especially those that do not contribute often. Many repos do have a contributors.md, I didn't see one, I came to my own conclusions, my conclusion was just wrong for this repo, no hard feelings.

I'm reopening this issue as it is still a valid one, I closed it in error. Myself and the community maybe able to add more clarity and edge cases as needed.

@bartelink could you elaborate on the file renames, I do not understand the reference here?

bartelink commented 1 week ago

For me (and I've spent a while maintaining this repo's issues, have a look), this is unfortunately a clear wont fix case Having it around as a way to signpost that is not a great use of the issues system Issues is better for things that might actually happen

But I won't spring clean this issue for a while, and others might disagree.

Regarding flagging contribution rules, I'm all for things being clear - if you want to propose a contributing.md for either this or serilog core, I could see a point to it. The main thing I would ask of it is that it conveys "there are lots of users and not a lot of maintainer time, so we err on the side of not taking low usage features so be sure to ask before investing time making a OR" VS "we are closed minded people that don't consider features if we don't think we'll personally use them so don't even waste your time bro".

Re file renames, I linked issue #40, which was the main thing prompting the forked sink I feel would me more likely / in a better position to consider this.

walterstypula commented 1 week ago

@bartelink I'm getting the feeling you're mischaracterizing my intentions, and thats okay, a lot of intent is lost over text. My understanding wasn't 'we are closed minded...' it was the former 'lots of users and not a lot of maintainer time'. I agree with the "don't rock the boat" approach taken here given the limitations and wide use.

bartelink commented 1 week ago

Not at all - I appreciate how you're being perfectly level headed on this. I was being pejorative with the intent of any proposed ontributing.md being minimal and permissive in tone. But I was referring back to your "it appears this repo is not taking any new features/enhancements at this time" and reasserting that such a stance is not the case, and any contributing.md should instead reflect what I've found to be the general stance/experience around here of "any properly mapped out proposal will get a yes/no and collaboration a lot quicker than you'd think"... Apologies for my directness and kudos to you for not attributing intent to text on the Internet ;)

nblumhardt commented 1 week ago

Thanks for the contribution @walterstypula. As @bartelink suggests, the repo is still very much open to features, but as it's a mature package with wide deployment, features are added very conservatively.

For rolling policies in particular, it's an area where each possible addition is small in isolation, but there are a surprising number of them. Instead of making the sink larger and more complex in this are, we've preferred to leave things simple enough that alternatives not already covered can be added fairly easily in forks and private copies of the sink. Having a smaller sink project to maintain also helps us focus on keeping the implemented feature set as close to rock-solid as possible.

Thanks again, though!