Skip to content

Add Roll on specific DayOfWeek #325

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

Closed
walterstypula opened this issue Oct 8, 2024 · 8 comments
Closed

Add Roll on specific DayOfWeek #325

walterstypula opened this issue Oct 8, 2024 · 8 comments

Comments

@walterstypula
Copy link

walterstypula commented Oct 8, 2024

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.

walterstypula pushed a commit to walterstypula/serilog-sinks-file that referenced this issue Oct 8, 2024
@bartelink
Copy link
Member

bartelink commented Oct 10, 2024

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: #323 (comment)

@walterstypula
Copy link
Author

@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
Copy link
Member

bartelink commented Oct 11, 2024

@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:

  • the rippling file renames (nothing to do with your actual need, but it similarly comes up a lot and it definitely causes frustration that it's not supported) are something that a decision has been made on - that won't happen here, but there are alternatives - some people simply want what they had in Log4Net etc and that's perfectly fine, just not for this repo
  • naming and more complex rolling strategies are not something that there's been a proven need or a concrete proposal on. The normal contribution guidelines apply across the entire suite of officially maintained Serilog repos (and most other significant OSS projects):
    • the maintainers have to live with any features/rules/complexity added forever
    • so features need to be well-founded
    • so the first step after you have a desire for a feature has to be to put up a proposal
    • if you had proposed the feature and I'd read it, I'd have answered exactly as I did after the fact (Note I'm a helper but don't have skin in the game to the same extent as Nick; my aim is to avoid wasted time that Nick could better invest in things other than reiterating stances, Note he'll definitely read this and regularly corrects people misspeaking on his behalf)
  • IMO looking at recent issues that have been closed and/or opened is well worth the time regardless of what repo you want to contribute to

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
Copy link
Author

walterstypula commented Oct 11, 2024

@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?

@walterstypula walterstypula reopened this Oct 11, 2024
@bartelink
Copy link
Member

bartelink commented Oct 11, 2024

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
Copy link
Author

@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
Copy link
Member

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
Copy link
Member

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!

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

No branches or pull requests

3 participants