-
Notifications
You must be signed in to change notification settings - Fork 125
Support ILoggingFailureListener
#342
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// Copyright © Serilog Contributors | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
using Serilog.Core; | ||
using Serilog.Debugging; | ||
using Serilog.Events; | ||
|
||
namespace Serilog.Sinks.File; | ||
|
||
sealed class FailedSink : ILogEventSink, ISetLoggingFailureListener | ||
{ | ||
ILoggingFailureListener _failureListener = SelfLog.FailureListener; | ||
|
||
public void Emit(LogEvent logEvent) | ||
{ | ||
_failureListener.OnLoggingFailed(this, LoggingFailureKind.Final, "the sink could not be initialized", [logEvent], exception: null); | ||
} | ||
|
||
public void SetFailureListener(ILoggingFailureListener failureListener) | ||
{ | ||
_failureListener = failureListener ?? throw new ArgumentNullException(nameof(failureListener)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
|
||
namespace Serilog.Sinks.File; | ||
|
||
sealed class RollingFileSink : ILogEventSink, IFlushableFileSink, IDisposable | ||
sealed class RollingFileSink : ILogEventSink, IFlushableFileSink, IDisposable, ISetLoggingFailureListener | ||
{ | ||
readonly PathRoller _roller; | ||
readonly ITextFormatter _textFormatter; | ||
|
@@ -33,6 +33,8 @@ sealed class RollingFileSink : ILogEventSink, IFlushableFileSink, IDisposable | |
readonly bool _rollOnFileSizeLimit; | ||
readonly FileLifecycleHooks? _hooks; | ||
|
||
ILoggingFailureListener _failureListener = SelfLog.FailureListener; | ||
|
||
readonly object _syncRoot = new(); | ||
bool _isDisposed; | ||
DateTime? _nextCheckpoint; | ||
|
@@ -72,6 +74,7 @@ public void Emit(LogEvent logEvent) | |
{ | ||
if (logEvent == null) throw new ArgumentNullException(nameof(logEvent)); | ||
|
||
bool failed; | ||
lock (_syncRoot) | ||
{ | ||
if (_isDisposed) throw new ObjectDisposedException("The log file has been disposed."); | ||
|
@@ -84,12 +87,18 @@ public void Emit(LogEvent logEvent) | |
AlignCurrentFileTo(now, nextSequence: true); | ||
} | ||
|
||
/* TODO: We REALLY should add this to avoid stuff become missing undetected. | ||
if (_currentFile == null) | ||
{ | ||
SelfLog.WriteLine("Log event {0} was lost since it was not possible to open the file or create a new one.", logEvent.RenderMessage()); | ||
} | ||
*/ | ||
failed = _currentFile == null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Falco20019 this should cover the TODO There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will have a look beginning next week, thanks 🎉 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nblumhardt There is sadly no artifact to use on AppVeyor as the build "failed successfully" ;) I will create a local build for testing for now, but you might want to fix the CI pipeline if you intend to use Actions + AppVeyor in parallel. If you are dropping AppVeyor with this PR as-well, I would add |
||
} | ||
|
||
if (failed) | ||
{ | ||
// Support fallback chains without the overhead of throwing an exception. | ||
_failureListener.OnLoggingFailed( | ||
this, | ||
LoggingFailureKind.Permanent, | ||
"the target file could not be opened or created", | ||
[logEvent], | ||
exception: null); | ||
} | ||
} | ||
|
||
|
@@ -170,14 +179,22 @@ void OpenFile(DateTime now, int? minSequence = null) | |
new FileSink(path, _textFormatter, _fileSizeLimitBytes, _encoding, _buffered, _hooks); | ||
|
||
_currentFileSequence = sequence; | ||
|
||
if (_currentFile is ISetLoggingFailureListener setLoggingFailureListener) | ||
{ | ||
setLoggingFailureListener.SetFailureListener(_failureListener); | ||
} | ||
} | ||
catch (IOException ex) | ||
{ | ||
if (IOErrors.IsLockedFile(ex)) | ||
{ | ||
SelfLog.WriteLine( | ||
"File target {0} was locked, attempting to open next in sequence (attempt {1})", path, | ||
attempt + 1); | ||
_failureListener.OnLoggingFailed( | ||
this, | ||
LoggingFailureKind.Temporary, | ||
$"file target {path} was locked, attempting to open next in sequence (attempt {attempt + 1})", | ||
events: null, | ||
exception: null); | ||
sequence = (sequence ?? 0) + 1; | ||
continue; | ||
} | ||
|
@@ -216,7 +233,7 @@ void ApplyRetentionPolicy(string currentFilePath, DateTime now) | |
// ReSharper disable once ConvertClosureToMethodGroup | ||
var potentialMatches = Directory.GetFiles(_roller.LogFileDirectory, _roller.DirectorySearchPattern) | ||
.Select(f => Path.GetFileName(f)) | ||
.Union(new[] { currentFileName }); | ||
.Union([currentFileName]); | ||
|
||
var newestFirst = _roller | ||
.SelectMatches(potentialMatches) | ||
|
@@ -239,7 +256,12 @@ void ApplyRetentionPolicy(string currentFilePath, DateTime now) | |
} | ||
catch (Exception ex) | ||
{ | ||
SelfLog.WriteLine("Error {0} while processing obsolete log file {1}", ex, fullPath); | ||
_failureListener.OnLoggingFailed( | ||
this, | ||
LoggingFailureKind.Temporary, | ||
$"error while processing obsolete log file {fullPath}", | ||
events: null, | ||
ex); | ||
} | ||
} | ||
} | ||
|
@@ -286,4 +308,9 @@ public void FlushToDisk() | |
_currentFile?.FlushToDisk(); | ||
} | ||
} | ||
|
||
public void SetFailureListener(ILoggingFailureListener failureListener) | ||
{ | ||
_failureListener = failureListener ?? throw new ArgumentNullException(nameof(failureListener)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like the concept of having every log still appearing somewhere if everything went wrong. Thanks!