Attempt at adding support for .NET Framework 4.8 #116
Conversation
This is only an idea of adding support for .NET Framework 4.8, following issue #114. This proof of concept is not ready to be merged. * The added dependencies must be conditioned to only .NET Framework 4.8. * The Json serliazing logic must be conditioned to .NET Framework 4.8 to avoid performance degradation for .NET 6+ users. * Documentation and build, test, publish pipelines must be updated accordingly
| <PackageReference Include="NLog" Version="5.2.8" /> | ||
| <PackageReference Include="System.Net.Http" Version="4.3.4" /> |
There was a problem hiding this comment.
Condition these new packages to .NET Framework 4.8 only
| @@ -49,19 +49,25 @@ public async Task WriteLogEventsAsync(LokiEvent lokiEvent) | |||
| /// Prepares the HttpContent for the loki event(s). | |||
| /// If gzip compression is enabled, prepares a gzip stream with the appropriate headers. | |||
| /// </summary> | |||
| private HttpContent CreateContent<T>(T lokiEvent) | |||
| private async Task<HttpContent> CreateContent<T>(T lokiEvent) | |||
There was a problem hiding this comment.
Create a different, async method for .NET Framework 4.8.
.NET 6+ will remain sync.
| var jsonContent = JsonContent.Create(lokiEvent, options: _jsonOptions); | ||
| //var jsonContent = JsonContent.Create(lokiEvent, options: _jsonOptions); | ||
|
|
||
| using var memStream = new MemoryStream(); |
There was a problem hiding this comment.
Use a pooled memory stream to limit allocations.
https://github.com/microsoft/Microsoft.IO.RecyclableMemoryStream
Though, I wonder if we really need to preallocate the json before sending the HTTP request. Is there a way to stream the json serialization and compression without having to pre-allocate the json?
There was a problem hiding this comment.
It doesn't look like it's possible to write to the stream without pre-allocating the entirety of the json stream.
https://marcroussy.com/2020/11/02/serialization-with-system-text-json/
The pooled memory stream should still come handy.
This is only an idea of adding support for .NET Framework 4.8, following issue #114. This proof of concept is not ready to be merged.