Skip to content

Support multi-process shared file sets #16

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

Merged
merged 2 commits into from
Aug 22, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ public static class RollingFileLoggerConfigurationExtensions
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
/// <param name="buffered">Indicates if flushing to the output file can be buffered or not. The default
/// is false.</param>
/// <param name="shared">Allow the log files to be shared by multiple processes. The default is false.</param>
/// <returns>Configuration object allowing method chaining.</returns>
/// <remarks>The file will be written using the UTF-8 character set.</remarks>
/// <remarks>The file will be written using the UTF-8 encoding without a byte-order mark.</remarks>
public static LoggerConfiguration RollingFile(
this LoggerSinkConfiguration sinkConfiguration,
string pathFormat,
Expand All @@ -64,11 +65,12 @@ public static LoggerConfiguration RollingFile(
long? fileSizeLimitBytes = DefaultFileSizeLimitBytes,
int? retainedFileCountLimit = DefaultRetainedFileCountLimit,
LoggingLevelSwitch levelSwitch = null,
bool buffered = false)
bool buffered = false,
bool shared = false)
{
var formatter = new MessageTemplateTextFormatter(outputTemplate, formatProvider);
return RollingFile(sinkConfiguration, formatter, pathFormat, restrictedToMinimumLevel, fileSizeLimitBytes,
retainedFileCountLimit, levelSwitch, buffered);
retainedFileCountLimit, levelSwitch, buffered, shared);
}

/// <summary>
Expand All @@ -92,8 +94,9 @@ public static LoggerConfiguration RollingFile(
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
/// <param name="buffered">Indicates if flushing to the output file can be buffered or not. The default
/// is false.</param>
/// <param name="shared">Allow the log files to be shared by multiple processes. The default is false.</param>
/// <returns>Configuration object allowing method chaining.</returns>
/// <remarks>The file will be written using the UTF-8 character set.</remarks>
/// <remarks>The file will be written using the UTF-8 encoding without a byte-order mark.</remarks>
public static LoggerConfiguration RollingFile(
this LoggerSinkConfiguration sinkConfiguration,
ITextFormatter formatter,
Expand All @@ -102,11 +105,19 @@ public static LoggerConfiguration RollingFile(
long? fileSizeLimitBytes = DefaultFileSizeLimitBytes,
int? retainedFileCountLimit = DefaultRetainedFileCountLimit,
LoggingLevelSwitch levelSwitch = null,
bool buffered = false)
bool buffered = false,
bool shared = false)
{
if (sinkConfiguration == null) throw new ArgumentNullException(nameof(sinkConfiguration));
if (formatter == null) throw new ArgumentNullException(nameof(formatter));
var sink = new RollingFileSink(pathFormat, formatter, fileSizeLimitBytes, retainedFileCountLimit, buffered: buffered);

if (shared)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if(shared && buffered) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops .. copy-pasta, thanks :-)

{
if (buffered)
throw new ArgumentException("Buffered writes are not available when file sharing is enabled.", nameof(buffered));
}

var sink = new RollingFileSink(pathFormat, formatter, fileSizeLimitBytes, retainedFileCountLimit, buffered: buffered, shared: shared);
return sinkConfiguration.Sink(sink, restrictedToMinimumLevel, levelSwitch);
}
}
Expand Down
28 changes: 21 additions & 7 deletions src/Serilog.Sinks.RollingFile/Sinks/RollingFile/RollingFileSink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ public sealed class RollingFileSink : ILogEventSink, IDisposable
readonly int? _retainedFileCountLimit;
readonly Encoding _encoding;
readonly bool _buffered;
readonly bool _shared;
readonly object _syncRoot = new object();

bool _isDisposed;
DateTime? _nextCheckpoint;
FileSink _currentFile;
ILogEventSink _currentFile;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the two file sinks will expose a common interface/ABC; for now ILogEventSink is the best I could do here.


/// <summary>Construct a <see cref="RollingFileSink"/>.</summary>
/// <param name="pathFormat">String describing the location of the log files,
Expand All @@ -54,28 +55,36 @@ public sealed class RollingFileSink : ILogEventSink, IDisposable
/// For unrestricted growth, pass null. The default is 1 GB.</param>
/// <param name="retainedFileCountLimit">The maximum number of log files that will be retained,
/// including the current log file. For unlimited retention, pass null. The default is 31.</param>
/// <param name="encoding">Character encoding used to write the text file. The default is UTF-8.</param>
/// <param name="encoding">Character encoding used to write the text file. The default is UTF-8 without BOM.</param>
/// <param name="buffered">Indicates if flushing to the output file can be buffered or not. The default
/// is false.</param>
/// <param name="shared">Allow the log files to be shared by multiple processes. The default is false.</param>
/// <returns>Configuration object allowing method chaining.</returns>
/// <remarks>The file will be written using the UTF-8 character set.</remarks>
public RollingFileSink(string pathFormat,
ITextFormatter textFormatter,
long? fileSizeLimitBytes,
int? retainedFileCountLimit,
Encoding encoding = null,
bool buffered = false)
bool buffered = false,
bool shared = false)
{
if (pathFormat == null) throw new ArgumentNullException(nameof(pathFormat));
if (fileSizeLimitBytes.HasValue && fileSizeLimitBytes < 0) throw new ArgumentException("Negative value provided; file size limit must be non-negative");
if (retainedFileCountLimit.HasValue && retainedFileCountLimit < 1) throw new ArgumentException("Zero or negative value provided; retained file count limit must be at least 1");

#if !SHARING
if (shared)
throw new NotSupportedException("File sharing is not supported on this platform.");
#endif

_roller = new TemplatedPathRoller(pathFormat);
_textFormatter = textFormatter;
_fileSizeLimitBytes = fileSizeLimitBytes;
_retainedFileCountLimit = retainedFileCountLimit;
_encoding = encoding ?? new UTF8Encoding(encoderShouldEmitUTF8Identifier: false);
_encoding = encoding;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DRY: Serilog.Sinks.File configures the default encoding, including it here means the two may drift if one is updated without updating the other.

_buffered = buffered;
_shared = shared;
}

/// <summary>
Expand All @@ -98,8 +107,7 @@ public void Emit(LogEvent logEvent)
// If the file was unable to be opened on the last attempt, it will remain
// null until the next checkpoint passes, at which time another attempt will be made to
// open it.
if (_currentFile != null)
_currentFile.Emit(logEvent);
_currentFile?.Emit(logEvent);
}
}

Expand Down Expand Up @@ -148,7 +156,13 @@ void OpenFile(DateTime now)

try
{
#if SHARING
_currentFile = _shared ?
(ILogEventSink)new SharedFileSink(path, _textFormatter, _fileSizeLimitBytes, _encoding) :
new FileSink(path, _textFormatter, _fileSizeLimitBytes, _encoding, _buffered);
#else
_currentFile = new FileSink(path, _textFormatter, _fileSizeLimitBytes, _encoding, _buffered);
#endif
}
catch (IOException ex)
{
Expand Down Expand Up @@ -223,7 +237,7 @@ void CloseFile()
{
if (_currentFile != null)
{
_currentFile.Dispose();
(_currentFile as IDisposable)?.Dispose();
_currentFile = null;
}

Expand Down
9 changes: 5 additions & 4 deletions src/Serilog.Sinks.RollingFile/project.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "2.3.0-*",
"version": "3.0.0-*",
"description": "The rolling file sink for Serilog - Simple .NET logging with fully-structured events",
"authors": [ "Serilog Contributors" ],
"packOptions": {
Expand All @@ -9,16 +9,17 @@
"iconUrl": "http://serilog.net/images/serilog-sink-nuget.png"
},
"dependencies": {
"Serilog": "2.0.0",
"Serilog.Sinks.File": "2.0.0"
"Serilog.Sinks.File": "3.0.0-dev-00735"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make a note to switch this back to 3.0 when this goes dev -> master

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dotnet pack will (should) fail on this, so if we forget it, it'll be caught by CI on the dev-master PR 👍

},
"buildOptions": {
"keyFile": "../../assets/Serilog.snk",
"xmlDoc": true,
"warningsAsErrors": true
},
"frameworks": {
"net4.5": {},
"net4.5": {
"buildOptions": {"define": ["SHARING"]}
},
"netstandard1.3": {
"dependencies": {
"System.IO": "4.1.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System;
using Xunit;

namespace Serilog.Tests
{
public class RollingFileLoggerConfigurationExtensionsTests
{
[Fact]
public void BuffferingIsNotAvailableWhenSharingEnabled()
{
Assert.Throws<ArgumentException>(() =>
new LoggerConfiguration()
.WriteTo.RollingFile("logs", buffered: true, shared: true));
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are admittedly a bit light, it's fair to assume the SharedFileSink does the heavy-lifting and is tested in its own repository, but some more coverage would be nice to add here at some point too.

4 changes: 1 addition & 3 deletions test/Serilog.Sinks.RollingFile.Tests/project.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
{
"version": "2.0.0",
"testRunner": "xunit",
"dependencies": {
"Serilog.Sinks.RollingFile": { "target": "project" },
"xunit": "2.1.0",
"dotnet-test-xunit": "1.0.0-rc2-build10025",
"Serilog.Sinks.File": "2.0.0"
"dotnet-test-xunit": "1.0.0-rc2-build10025"
},
"buildOptions": {
"keyFile": "../../assets/Serilog.snk",
Expand Down