From 2aee400bbb1fa5b2a4ee95259c1aad7ebdea8247 Mon Sep 17 00:00:00 2001 From: "Mattias Kindborg @FantasticFiasco" Date: Mon, 21 Feb 2022 00:12:50 +0100 Subject: [PATCH] fix: promote non-durable sink parameter 'queueLimitBytes' (#247) Fixes #245 --- .all-contributorsrc | 9 +++ CHANGELOG.md | 80 +------------------ README-v8.md | 5 +- README.md | 1 + .../LoggerSinkConfigurationExtensions.cs | 12 +-- .../Sinks/Http/Private/NonDurable/HttpSink.cs | 2 +- .../HttpSinkGivenCodeConfigurationShould.cs | 7 +- .../Http/Private/NonDurable/HttpSinkShould.cs | 6 +- .../appsettings_http.json | 2 +- 9 files changed, 31 insertions(+), 93 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 909dfa5a..11141296 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -141,6 +141,15 @@ "contributions": [ "doc" ] + }, + { + "login": "seruminar", + "name": "Yuriy Sountsov", + "avatar_url": "https://avatars.githubusercontent.com/u/35008875?v=4", + "profile": "https://github.com/seruminar", + "contributions": [ + "ideas" + ] } ], "contributorsPerLine": 7 diff --git a/CHANGELOG.md b/CHANGELOG.md index ea473233..c3442b1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,83 +12,9 @@ This project adheres to [Semantic Versioning](http://semver.org/) and is followi **Migration guide** -By far the easiest way to migrate your code is to stop using positional arguments and instead use named arguments. +The parameter `batchSizeLimitBytes` has been introduced to the methods `Http`, `DurableHttpUsingFileSizeRolledBuffers` and `DurableHttpUsingTimeRolledBuffers`. Please verify that the arguments pass by you to these methods still align with your intentions. -If you use the non-durable sink please make the following changes to your code. - -```csharp -// Before migration -log = new LoggerConfiguration() - .WriteTo.Http("https://www.mylogs.com", 500, 500) - .CreateLogger(); - -// After migration -log = new LoggerConfiguration() - .WriteTo.Http( - requestUri: "https://www.mylogs.com", - batchPostingLimit: 500, - // the new argument batchSizeLimitBytes is positioned here - queueLimit: 500) - .CreateLogger(); -``` - -If you use the durable file size rolled sink please make the following changes to your code. - -```csharp -// Before migration -log = new LoggerConfiguration() - .WriteTo.DurableHttpUsingFileSizeRolledBuffers( - "https://www.mylogs.com", - "MyBuffer", - ByteSize.GB, - false, - 10, - 500, - TimeSpan.FromSeconds(10)) - .CreateLogger(); - -// After migration -log = new LoggerConfiguration() - .WriteTo.DurableHttpUsingFileSizeRolledBuffers( - requestUri: "https://www.mylogs.com", - bufferBaseFileName: "MyBuffer", - bufferFileSizeLimitBytes: ByteSize.GB, - bufferFileShared: false, - retainedBufferFileCountLimit: 10, - batchPostingLimit: 500, - // the new argument batchSizeLimitBytes is positioned here - period: TimeSpan.FromSeconds(10)) - .CreateLogger(); -``` - -If you use the durable time rolled sink please make the following changes to your code. - -```csharp -// Before migration -log = new LoggerConfiguration() - .WriteTo.DurableHttpUsingTimeRolledBuffers( - "https://www.mylogs.com", - "MyBuffer-{Date}.json", - ByteSize.GB, - false, - 10, - 500, - TimeSpan.FromSeconds(10)) - .CreateLogger(); - -// After migration -log = new LoggerConfiguration() - .WriteTo.DurableHttpUsingTimeRolledBuffers( - requestUri: "https://www.mylogs.com", - bufferPathFormat: "MyBuffer-{Date}.json", - bufferFileSizeLimitBytes: ByteSize.GB, - bufferFileShared: false, - retainedBufferFileCountLimit: 10, - batchPostingLimit: 500, - // the new argument batchSizeLimitBytes is positioned here - period: TimeSpan.FromSeconds(10)) - .CreateLogger(); -``` +To automatically mitigate this kind of *new parameter issue* in the future would be to move from using positional arguments to use named arguments. - [#166](https://github.com/FantasticFiasco/serilog-sinks-http/issues/166) Support for content encoding [Gzip](https://en.wikipedia.org/wiki/Gzip) using HTTP client `JsonGzipHttpClient` (contribution by [@vaibhavepatel](https://github.com/vaibhavepatel), [@KalininAndreyVictorovich](https://github.com/KalininAndreyVictorovich) and [@AntonSmolkov](https://github.com/AntonSmolkov)) - [#166](https://github.com/FantasticFiasco/serilog-sinks-http/issues/166) Support for specifying `HttpClient` when creating `JsonHttpClient` and `JsonGzipHttpClient` @@ -195,7 +121,7 @@ Given you are configuring the sink in application configuration you should apply ``` - [#206](https://github.com/FantasticFiasco/serilog-sinks-http/issues/206) [BREAKING CHANGE] Argument `bufferFileSizeLimitBytes` to extension methods `DurableHttpUsingFileSizeRolledBuffers` and `DurableHttpUsingTimeRolledBuffers` no longer accepts `0` as value -- [#203](https://github.com/FantasticFiasco/serilog-sinks-http/issues/203) [BREAKING CHANGE] Non-durable sink has changed from having its maximum queue size defined as number of events into number of bytes, making it far easier to reason about memory consumption. +- [#203](https://github.com/FantasticFiasco/serilog-sinks-http/issues/203), [#245](https://github.com/FantasticFiasco/serilog-sinks-http/issues/245) [BREAKING CHANGE] Non-durable sink has changed from having its maximum queue size defined as number of events into number of bytes, making it far easier to reason about memory consumption. It's importance to the behavior of the sink was also the reasoning for promoting it from being optional to being mandatory. (proposed by [@seruminar](https://github.com/seruminar)) Given you are configuring the sink in code you should do the following changes. diff --git a/README-v8.md b/README-v8.md index ab7aae25..9efe405c 100644 --- a/README-v8.md +++ b/README-v8.md @@ -43,7 +43,7 @@ In the following example, the sink will POST log events to `http://www.mylogs.co ```csharp ILogger log = new LoggerConfiguration() .MinimumLevel.Verbose() - .WriteTo.Http(requestUri: "https://www.mylogs.com") + .WriteTo.Http(requestUri: "https://www.mylogs.com", queueLimitBytes: null) .CreateLogger(); log.Information("Logging {@Heartbeat} from {Computer}", heartbeat, computer); @@ -59,7 +59,8 @@ Used in conjunction with [Serilog.Settings.Configuration](https://github.com/ser { "Name": "Http", "Args": { - "requestUri": "https://www.mylogs.com" + "requestUri": "https://www.mylogs.com", + "queueLimitBytes": null } } ] diff --git a/README.md b/README.md index a9ef7500..9cde29be 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,7 @@ The following users have made significant contributions to this project. Thank y
Sergios
🐛
Anton Smolkov
💻 🐛
Michael J Conrad
📖 +
Yuriy Sountsov
🤔 diff --git a/src/Serilog.Sinks.Http/LoggerSinkConfigurationExtensions.cs b/src/Serilog.Sinks.Http/LoggerSinkConfigurationExtensions.cs index e84ab887..996ff35d 100644 --- a/src/Serilog.Sinks.Http/LoggerSinkConfigurationExtensions.cs +++ b/src/Serilog.Sinks.Http/LoggerSinkConfigurationExtensions.cs @@ -43,6 +43,10 @@ public static class LoggerSinkConfigurationExtensions /// /// The logger configuration. /// The URI the request is sent to. + /// + /// The maximum size, in bytes, of events stored in memory, waiting to be sent over the + /// network. Specify null for no limit. + /// /// /// The maximum size, in bytes, for a serialized representation of a log event. Log events /// exceeding this size will be dropped. Specify null for no limit. Default value is null. @@ -67,10 +71,6 @@ public static class LoggerSinkConfigurationExtensions /// /// Default value is null. /// - /// - /// The maximum size, in bytes, of events stored in memory, waiting to be sent over the - /// network. Specify null for no limit. Default value is null. - /// /// /// The time to wait between checking for event batches. Default value is 2 seconds. /// @@ -100,10 +100,10 @@ public static class LoggerSinkConfigurationExtensions public static LoggerConfiguration Http( this LoggerSinkConfiguration sinkConfiguration, string requestUri, + long? queueLimitBytes, long? logEventLimitBytes = null, int? logEventsInBatchLimit = 1000, long? batchSizeLimitBytes = null, - long? queueLimitBytes = null, TimeSpan? period = null, ITextFormatter? textFormatter = null, IBatchFormatter? batchFormatter = null, @@ -127,10 +127,10 @@ public static LoggerConfiguration Http( var sink = new HttpSink( requestUri: requestUri, + queueLimitBytes: queueLimitBytes, logEventLimitBytes: logEventLimitBytes, logEventsInBatchLimit: logEventsInBatchLimit, batchSizeLimitBytes: batchSizeLimitBytes, - queueLimitBytes: queueLimitBytes, period: period.Value, textFormatter: textFormatter, batchFormatter: batchFormatter, diff --git a/src/Serilog.Sinks.Http/Sinks/Http/Private/NonDurable/HttpSink.cs b/src/Serilog.Sinks.Http/Sinks/Http/Private/NonDurable/HttpSink.cs index 3744ded0..0cff1981 100644 --- a/src/Serilog.Sinks.Http/Sinks/Http/Private/NonDurable/HttpSink.cs +++ b/src/Serilog.Sinks.Http/Sinks/Http/Private/NonDurable/HttpSink.cs @@ -43,10 +43,10 @@ public class HttpSink : ILogEventSink, IDisposable public HttpSink( string requestUri, + long? queueLimitBytes, long? logEventLimitBytes, int? logEventsInBatchLimit, long? batchSizeLimitBytes, - long? queueLimitBytes, TimeSpan period, ITextFormatter textFormatter, IBatchFormatter batchFormatter, diff --git a/test/Serilog.Sinks.HttpTests/HttpSinkGivenCodeConfigurationShould.cs b/test/Serilog.Sinks.HttpTests/HttpSinkGivenCodeConfigurationShould.cs index d2beae2e..d871bc8a 100644 --- a/test/Serilog.Sinks.HttpTests/HttpSinkGivenCodeConfigurationShould.cs +++ b/test/Serilog.Sinks.HttpTests/HttpSinkGivenCodeConfigurationShould.cs @@ -39,9 +39,9 @@ public async Task WriteLogEvent(LogEventLevel level) .WriteTo .Http( requestUri: webServerFixture.RequestUri(testId), + queueLimitBytes: ByteSize.MB, logEventsInBatchLimit: 100, batchSizeLimitBytes: ByteSize.MB, - queueLimitBytes: ByteSize.MB, period: TimeSpan.FromMilliseconds(1), textFormatter: new NormalRenderedTextFormatter(), batchFormatter: new ArrayBatchFormatter(), @@ -71,9 +71,9 @@ public async Task WriteBatches(int numberOfEvents) .WriteTo .Http( requestUri: webServerFixture.RequestUri(testId), + queueLimitBytes: null, logEventsInBatchLimit: 100, batchSizeLimitBytes: ByteSize.MB, - queueLimitBytes: null, period: TimeSpan.FromMilliseconds(1), textFormatter: new NormalRenderedTextFormatter(), batchFormatter: new ArrayBatchFormatter(), @@ -104,9 +104,9 @@ public async Task OvercomeNetworkFailure() .WriteTo .Http( requestUri: webServerFixture.RequestUri(testId), + queueLimitBytes: ByteSize.MB, logEventsInBatchLimit: 100, batchSizeLimitBytes: ByteSize.MB, - queueLimitBytes: ByteSize.MB, period: TimeSpan.FromMilliseconds(1), textFormatter: new NormalRenderedTextFormatter(), batchFormatter: new ArrayBatchFormatter(), @@ -136,6 +136,7 @@ public void ConfigureHttpClient() .WriteTo .Http( requestUri: "https://www.mylogs.com", + queueLimitBytes: null, httpClient: httpClient, configuration: configuration) .CreateLogger(); diff --git a/test/Serilog.Sinks.HttpTests/Sinks/Http/Private/NonDurable/HttpSinkShould.cs b/test/Serilog.Sinks.HttpTests/Sinks/Http/Private/NonDurable/HttpSinkShould.cs index d4447727..f21e20c1 100644 --- a/test/Serilog.Sinks.HttpTests/Sinks/Http/Private/NonDurable/HttpSinkShould.cs +++ b/test/Serilog.Sinks.HttpTests/Sinks/Http/Private/NonDurable/HttpSinkShould.cs @@ -29,10 +29,10 @@ public async Task StayIdleGivenNoLogEvents() using (new HttpSink( requestUri: webServerFixture.RequestUri(testId), + queueLimitBytes: null, logEventLimitBytes: null, logEventsInBatchLimit: null, batchSizeLimitBytes: null, - queueLimitBytes: null, period: period, textFormatter: new NormalTextFormatter(), batchFormatter: new ArrayBatchFormatter(), @@ -56,10 +56,10 @@ public async Task RespectLogEventLimitBytes() using var sink = new HttpSink( requestUri: webServerFixture.RequestUri(testId), + queueLimitBytes: null, logEventLimitBytes: 1, // Is lower than emitted log event logEventsInBatchLimit: null, batchSizeLimitBytes: null, - queueLimitBytes: null, period: period, textFormatter: new NormalTextFormatter(), batchFormatter: new ArrayBatchFormatter(), @@ -91,10 +91,10 @@ public async Task RespectQueueLimitBytes() using var sink = new HttpSink( requestUri: webServerFixture.RequestUri(testId), + queueLimitBytes: 134, // Queue only holds the first event, which allocates 134 bytes logEventLimitBytes: null, logEventsInBatchLimit: null, batchSizeLimitBytes: null, - queueLimitBytes: 134, // Queue only holds the first event, which allocates 134 bytes period: period, textFormatter: new NormalTextFormatter(), batchFormatter: new ArrayBatchFormatter(), diff --git a/test/Serilog.Sinks.HttpTests/appsettings_http.json b/test/Serilog.Sinks.HttpTests/appsettings_http.json index e3a0aa32..8a1e7d5a 100644 --- a/test/Serilog.Sinks.HttpTests/appsettings_http.json +++ b/test/Serilog.Sinks.HttpTests/appsettings_http.json @@ -6,9 +6,9 @@ "Name": "Http", "Args": { "requestUri": "http://placeholder.com", + "queueLimitBytes": null, "logEventsInBatchLimit": 100, "batchSizeLimitBytes": 1048576, - "queueLimitBytes": null, "period": "00:00:00.001", "textFormatter": "Serilog.Sinks.Http.TextFormatters.NormalRenderedTextFormatter, Serilog.Sinks.Http", "batchFormatter": "Serilog.Sinks.Http.BatchFormatters.ArrayBatchFormatter, Serilog.Sinks.Http",