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

Support for fewer rolling files, and shorter time window #19

Closed
rob-somerville opened this issue May 18, 2017 · 12 comments
Closed

Support for fewer rolling files, and shorter time window #19

rob-somerville opened this issue May 18, 2017 · 12 comments

Comments

@rob-somerville
Copy link
Contributor

We're logging audit data at a high rate. The RollingFileSink in the DurableHttpSink is creating a new log file once a day, and keeping 31 logs. We'd like to move to a model where log files are created every 30 minutes, and only the last three are retained. Do you think this could be made a configuration option? Should I submit a pull request?

@FantasticFiasco
Copy link
Owner

FantasticFiasco commented May 19, 2017

Hi Rob.

Yes that seems to be a valid problem. I know for a fact that you can specify the number of files the rolling file should retain, but am unsure whether you can control when the files should be rolled. Lets investigate this before proceeding further.

@FantasticFiasco
Copy link
Owner

The rolling file sink gives us the following configuration options:

public static LoggerConfiguration RollingFile(
    this LoggerSinkConfiguration sinkConfiguration,
    ITextFormatter formatter,
    string pathFormat,
    LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
    long? fileSizeLimitBytes = DefaultFileSizeLimitBytes,
    int? retainedFileCountLimit = DefaultRetainedFileCountLimit,
    LoggingLevelSwitch levelSwitch = null,
    bool buffered = false,
    bool shared = false,
    TimeSpan? flushToDiskInterval = null)

There doesn't seem to be an option for specifying when a file should be rolled. Would it be acceptable for you to specify the fileSizeLimitBytes instead?

@rob-somerville
Copy link
Contributor Author

rob-somerville commented May 19, 2017 via email

@FantasticFiasco
Copy link
Owner

Wow, I didn't even know that the rolling file sink supported {Date}, {Hour} and {HalfHour}. The parameter description of pathFormat only mentioned {Date}. Thanks for informing me!

What we can do is to replace bufferBaseFilename with a new parameter called bufferFilenameFormat, which would allow you to specify the full filename format, letting you decide the rolling period. Here is the description from the rolling file sink:

The sink supports three different filename format specifiers:

- {Date} Creates a file per day. Filenames use the yyyyMMdd format.
- {Hour} Creates a file per hour. Filenames use the yyyyMMddHH format.
- {HalfHour} Creates a file per half hour. Filenames use the yyyyMMddHHmm format.

If a log file path is provided without one of the specifiers above, {Date} will be inserted by default.

If this, in combination with retainedFileCountLimit is acceptable to you, I think we should go for that solution.

The other solution is to also support a file size based rolling behavior, as implemented by Serilog.Sinks.RollingFileAlternate. But I am hesitant to do so, mainly because Serilog.Sinks.RollingFileAlternate is far from being the battle tested dependency Serilog.Sinks.RollingFile is.

@rob-somerville
Copy link
Contributor Author

rob-somerville commented May 19, 2017 via email

@FantasticFiasco
Copy link
Owner

Is this something you would like to contribute? Otherwise I'll get right on it.

@rob-somerville
Copy link
Contributor Author

rob-somerville commented May 22, 2017 via email

@rob-somerville
Copy link
Contributor Author

I have a PR, but I'm getting a 403 when I try and create the branch. Is that permissions on the repo? I can send a zip if you'd rather.

@FantasticFiasco
Copy link
Owner

Could that be because you've cloned this repository and try to commit to it?

If so, try to create a fork, commit your changes (just copy the files with changes over to the forked repository) and create the PR from your fork.

Please let me know if I can help you along the way.

@FantasticFiasco
Copy link
Owner

You can find a 4.0.0 beta release on NuGet if you would like to try it out. I'll make an official release in two days if nothing unexpected occurs.

Thanks for improving the functionality and compatibility of this sink.

@rob-somerville
Copy link
Contributor Author

rob-somerville commented Jun 6, 2017 via email

FantasticFiasco added a commit that referenced this issue Jun 6, 2017
Added

- [BREAKING CHANGE] Support for Serilog.Settings.Configuration required
  changing the extension methods configuring a HTTP sink. 'Options' and
  'DurableOptions' no longer exist, and their properties are now optional
  parameters on the extension methods instead.
- Support for HTTP body configuration using 'IBatchFormatter' and
  'ITextFormatter'. This enables full control of how messages are serialized
  before being sent over the network. (contribution by @kvpt)
- Support for specifying the maximum number of retained buffer files and their
  rotation period on the durable HTTP sink. (contribution by @rob-somerville)

Fixes #8
Fixes #11
Fixes #19
@FantasticFiasco
Copy link
Owner

Any news on this?

FantasticFiasco added a commit that referenced this issue Jun 17, 2017
Added

- [BREAKING CHANGE] Support for Serilog.Settings.Configuration required
  changing the extension methods configuring a HTTP sink. 'Options' and
  'DurableOptions' no longer exist, and their properties are now optional
  parameters on the extension methods instead.
- Support for HTTP body configuration using 'IBatchFormatter' and
  'ITextFormatter'. This enables full control of how messages are serialized
  before being sent over the network. (contribution by @kvpt)
- Support for specifying the maximum number of retained buffer files and their
  rotation period on the durable HTTP sink. (contribution by @rob-somerville)

Fixes #8
Fixes #11
Fixes #19
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

No branches or pull requests

2 participants