Skip to content
This repository has been archived by the owner on Mar 1, 2022. It is now read-only.

Commit

Permalink
Feature/issue25 (#26)
Browse files Browse the repository at this point in the history
* Review and correct BOM marker handling in TryReadLine

No longer drops lines if BOM marker detected and avoids offsetting the line start.
Also adds more Selflog writes to try to catch bookmark read failures.

* Extract Persistence methods into own classes

Three classes have been created:
- FileBufferDataProvider, with the responsability of tracking and retrieving buffered content on disk
- FileBasedBookmarkProvider, with the responsability of reading from and writing to the bookmark file
- InvalidPayloadLogger, reposnible for writing events to disk that failed to be processed (dead-letter queue type concept)

For testing, the FS was abstracted through FileSystemAdapter

An attempt mas made to seperate concerns as much as possible, and avoid refs / outs to limit tracking complexity. HttpLogShipper
was pretty big and hard to follow along, with lots of state change. Eficiency was not considered yet in this refactoring effort.
Tests were added to simulate multiple scenarios.

Related to #25

* cleanup and review enhancements

includes:
- HRESULT exception handling comments and constants
- SelfLog in case of IOException (locked buffer file)
- refactor GetBatchOfEvents
-

fix for appVeyor package generation

Test project targets net452 to work correctly with xUnit. See xunit/xunit#1130 (comment) for more info:

add disposable interface to BookmarkProvider

delete all the old buffer files (N-2) in one timer cycle

read / create initial bookmark based on _retainedFileLimitCount

* correct MoveForward behavior to respect retentionLimit

* update readme
  • Loading branch information
MiguelAlho authored Oct 6, 2017
1 parent 30538f8 commit eaa7dd7
Show file tree
Hide file tree
Showing 29 changed files with 1,912 additions and 360 deletions.
4 changes: 3 additions & 1 deletion Build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ if(Test-Path .\artifacts) {

$branch = @{ $true = $env:APPVEYOR_REPO_BRANCH; $false = $(git symbolic-ref --short -q HEAD) }[$env:APPVEYOR_REPO_BRANCH -ne $NULL];
$revision = @{ $true = "{0:00000}" -f [convert]::ToInt32("0" + $env:APPVEYOR_BUILD_NUMBER, 10); $false = "local" }[$env:APPVEYOR_BUILD_NUMBER -ne $NULL];
$suffix = @{ $true = ""; $false = "$($branch.Substring(0, [math]::Min(10,$branch.Length)))-$revision"}[$branch -eq "master" -and $revision -ne "local"]
#if branch includes features/, things blow up, so let's remove it
$branchForPackageName = $branch -replace "feature[s]?/",'';
$suffix = @{ $true = ""; $false = "$($branchForPackageName.Substring(0, [math]::Min(10,$branchForPackageName.Length)))-$revision"}[$branch -eq "master" -and $revision -ne "local"]

echo "build: Version suffix is $suffix"

Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@ var log = new LoggerConfiguration()
.CreateLogger();
```

This will write unsent messages to a `buffer-{Date}.json` file in the specified folder (`C:\test\` in the example).
This will write unsent messages to a `buffer-{Date}.json` file in the specified folder (`C:\test\` in the example).

The method also takes a `reretainedFileCountLimit` parameter that will allow you to control how much info to store / ship when back online. By default, the value is `null` with the intent is to send all persisted data, no matter how old. If you specify a value, only the data in the last N buffer files will be shipped back, preventing stale data to be indexed (if that info is no longer usefull).
2 changes: 2 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ version: '{build}'
skip_tags: true
image: Visual Studio 2017
configuration: Release
init:
- git config --global core.autocrlf false
install:
- ps: mkdir -Force ".\build\" | Out-Null
build_script:
Expand Down
7 changes: 5 additions & 2 deletions sample/sampleDurableLogger/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ namespace SampleDurableLogger
{
public class Program
{
public static void Main(string[] args)
public static void Main()
{
SetupLogglyConfiguration();
using (var logger = CreateLogger(@"c:\test\"))
using (var logger = CreateLogger(@"C:\test\Logs\"))
{
logger.Information("Test message - app started");
logger.Warning("Test message with {@Data}", new {P1 = "sample", P2 = DateTime.Now});
Expand Down Expand Up @@ -44,6 +44,9 @@ public static void Main(string[] args)

static Logger CreateLogger(string logFilePath)
{
//write selflog to stderr
Serilog.Debugging.SelfLog.Enable(Console.Error);

return new LoggerConfiguration()
.MinimumLevel.Debug()
//Add enrichers
Expand Down
3 changes: 2 additions & 1 deletion serilog-sinks-loggly.sln
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 15
VisualStudioVersion = 15.0.26430.16
VisualStudioVersion = 15.0.26430.12
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{037440DE-440B-4129-9F7A-09B42D00397E}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "assets", "assets", "{E9D1B5E1-DEB9-4A04-8BAB-24EC7240ADAF}"
ProjectSection(SolutionItems) = preProject
.editorconfig = .editorconfig
appveyor.yml = appveyor.yml
Build.ps1 = Build.ps1
README.md = README.md
assets\Serilog.snk = assets\Serilog.snk
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ public static class LoggerConfigurationLogglyExtensions
/// <param name="retainedInvalidPayloadsLimitBytes">A soft limit for the number of bytes to use for storing failed requests.
/// The limit is soft in that it can be exceeded by any single error payload, but in that case only that single error
/// payload will be retained.</param>
/// <param name="retainedFileCountLimit">number of files to retain for the buffer</param>
/// <param name="retainedFileCountLimit">number of files to retain for the buffer. If defined, this also controls which records
/// in the buffer get sent to the remote Loggly instance</param>
/// <returns>Logger configuration, allowing configuration to continue.</returns>
/// <exception cref="ArgumentNullException">A required parameter is null.</exception>
public static LoggerConfiguration Loggly(
Expand Down Expand Up @@ -91,6 +92,5 @@ public static LoggerConfiguration Loggly(

return loggerConfiguration.Sink(sink, restrictedToMinimumLevel);
}

}
}
13 changes: 12 additions & 1 deletion src/Serilog.Sinks.Loggly/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,15 @@
"201AC646C451830FC7E61A2DFD633D34" +
"C39F87B81894191652DF5AC63CC40C77" +
"F3542F702BDA692E6E8A9158353DF189" +
"007A49DA0F3CFD55EB250066B19485EC")]
"007A49DA0F3CFD55EB250066B19485EC")]

[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=00240000048000009400000006020000002" +
"40000525341310004000001000100c547ca" +
"c37abd99c8db225ef2f6c8a3602f3b3606c" +
"c9891605d02baa56104f4cfc0734aa39b93" +
"bf7852f7d9266654753cc297e7d2edfe0ba" +
"c1cdcf9f717241550e0a7b191195b7667bb" +
"4f64bcb8e2121380fd1d9d46ad2d92d2d15" +
"605093924cceaf74c4861eff62abf69b929" +
"1ed0a340e113be11e6a7d3113e92484cf70" +
"45cc7")]
3 changes: 2 additions & 1 deletion src/Serilog.Sinks.Loggly/Serilog.Sinks.Loggly.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Description>Serilog sink for Loggly.com service</Description>
Expand All @@ -15,6 +15,7 @@
<PackageProjectUrl>http://serilog.net</PackageProjectUrl>
<PackageLicenseUrl>http://www.apache.org/licenses/LICENSE-2.0</PackageLicenseUrl>
<GenerateAssemblyVersionAttribute>false</GenerateAssemblyVersionAttribute>
<RootNamespace>Serilog</RootNamespace>
</PropertyGroup>

<ItemGroup>
Expand Down
16 changes: 16 additions & 0 deletions src/Serilog.Sinks.Loggly/Sinks/Loggly/Durable/FileSetPosition.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace Serilog.Sinks.Loggly.Durable
{
public class FileSetPosition
{
public FileSetPosition(long position, string fileFullPath)
{
NextLineStart = position;
File = fileFullPath;
}

public long NextLineStart { get; }
public string File { get; }

public static readonly FileSetPosition None = default(FileSetPosition);
}
}
3 changes: 2 additions & 1 deletion src/Serilog.Sinks.Loggly/Sinks/Loggly/DurableLogglySink.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ public DurableLogglySink(
eventBodyLimitBytes,
levelControlSwitch,
retainedInvalidPayloadsLimitBytes,
encoding);
encoding,
retainedFileCountLimit);

//writes events to the file to support connection recovery
_sink = new RollingFileSink(
Expand Down
89 changes: 89 additions & 0 deletions src/Serilog.Sinks.Loggly/Sinks/Loggly/FileBasedBookmarkProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using System;
using System.IO;
using System.Text;
using Serilog.Debugging;
using Serilog.Sinks.Loggly.Durable;

namespace Serilog.Sinks.Loggly
{
class FileBasedBookmarkProvider : IBookmarkProvider
{
readonly IFileSystemAdapter _fileSystemAdapter;
readonly Encoding _encoding;

readonly string _bookmarkFilename;
Stream _currentBookmarkFileStream;

public FileBasedBookmarkProvider(string bufferBaseFilename, IFileSystemAdapter fileSystemAdapter, Encoding encoding)
{
_bookmarkFilename = Path.GetFullPath(bufferBaseFilename + ".bookmark");
_fileSystemAdapter = fileSystemAdapter;
_encoding = encoding;
}

public void Dispose()
{
_currentBookmarkFileStream?.Dispose();
}

public FileSetPosition GetCurrentBookmarkPosition()
{
EnsureCurrentBookmarkStreamIsOpen();

if (_currentBookmarkFileStream.Length != 0)
{
using (var bookmarkStreamReader = new StreamReader(_currentBookmarkFileStream, _encoding, false, 128, true))
{
//set the position to 0, to begin reading the initial line
bookmarkStreamReader.BaseStream.Position = 0;
var bookmarkInfoLine = bookmarkStreamReader.ReadLine();

if (bookmarkInfoLine != null)
{
//reset position after read
var parts = bookmarkInfoLine.Split(new[] {":::"}, StringSplitOptions.RemoveEmptyEntries);
if (parts.Length == 2 && long.TryParse(parts[0], out long position))
{
return new FileSetPosition(position, parts[1]);
}

SelfLog.WriteLine("Unable to read a line correctly from bookmark file");
}
else
{
SelfLog.WriteLine(
"For some unknown reason, we were unable to read the non-empty bookmark info...");
}
}
}

//bookmark file is empty or has been misread, so return a null bookmark
return null;
}

public void UpdateBookmark(FileSetPosition newBookmark)
{
EnsureCurrentBookmarkStreamIsOpen();

using (var bookmarkStreamWriter = new StreamWriter(_currentBookmarkFileStream, _encoding, 128, true))
{
bookmarkStreamWriter.BaseStream.Position = 0;
bookmarkStreamWriter.WriteLine("{0}:::{1}", newBookmark.NextLineStart, newBookmark.File);
bookmarkStreamWriter.Flush();
}
}

void EnsureCurrentBookmarkStreamIsOpen()
{
//this will ensure a stream is available, even if it means creating a new file associated to it
if (_currentBookmarkFileStream == null)
_currentBookmarkFileStream = _fileSystemAdapter.Open(
_bookmarkFilename,
FileMode.OpenOrCreate,
FileAccess.ReadWrite,
FileShare.Read);
}


}
}
Loading

0 comments on commit eaa7dd7

Please sign in to comment.