Skip to content
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.

Sign up for GitHub

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

Attempt at adding support for .NET Framework 4.8 #116

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+19 −11
Draft

Attempt at adding support for .NET Framework 4.8 #116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 15 additions & 9 deletions src/NLog.Loki/HttpLokiTransport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System.Collections.Generic;
using System.IO;
using System.IO.Compression;
using System.Net.Http;
using System.Net.Http.Json;
using System.Text.Json;
using System.Threading.Tasks;
using NLog.Common;
Expand All @@ -25,22 +25,22 @@ public HttpLokiTransport(
{
_lokiHttpClient = lokiHttpClient;
_gzipLevel = gzipLevel;

_jsonOptions = new JsonSerializerOptions();
_jsonOptions.Converters.Add(new LokiEventsSerializer(orderWrites));
_jsonOptions.Converters.Add(new LokiEventSerializer());
}

public async Task WriteLogEventsAsync(IEnumerable<LokiEvent> lokiEvents)
{
using var jsonStreamContent = CreateContent(lokiEvents);
using var jsonStreamContent = await CreateContent(lokiEvents);
using var response = await _lokiHttpClient.PostAsync("loki/api/v1/push", jsonStreamContent).ConfigureAwait(false);
await ValidateHttpResponse(response).ConfigureAwait(false);
}

public async Task WriteLogEventsAsync(LokiEvent lokiEvent)
{
using var jsonStreamContent = CreateContent(lokiEvent);
using var jsonStreamContent = await CreateContent(lokiEvent);
using var response = await _lokiHttpClient.PostAsync("loki/api/v1/push", jsonStreamContent).ConfigureAwait(false);
await ValidateHttpResponse(response).ConfigureAwait(false);
}
Expand All @@ -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)
Copy link
Owner Author

Choose a reason for hiding this comment

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

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();
Copy link
Owner Author

Choose a reason for hiding this comment

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

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?

Copy link
Owner Author

Choose a reason for hiding this comment

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

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.

await JsonSerializer.SerializeAsync(memStream, lokiEvent, options: _jsonOptions);
memStream.Position = 0;
using var con = new StreamContent(memStream);
con.Headers.ContentType.MediaType = "application/json";

// Some old loki versions support only 'application/json',
// not 'application/json; charset=utf-8' content-Type header
jsonContent.Headers.ContentType.CharSet = string.Empty;
con.Headers.ContentType.CharSet = string.Empty;

// If no compression required
if(_gzipLevel == CompressionLevel.NoCompression)
return jsonContent;
return con;

return new CompressedContent(jsonContent, _gzipLevel);
return new CompressedContent(con, _gzipLevel);
}

private static async ValueTask ValidateHttpResponse(HttpResponseMessage response)
Expand Down
1 change: 0 additions & 1 deletion src/NLog.Loki/LokiTarget.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.IO.Compression;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text;
Expand Down
5 changes: 4 additions & 1 deletion src/NLog.Loki/NLog.Loki.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netstandard2.0;net6.0;net7.0;net8.0</TargetFrameworks>
<TargetFrameworks>net48;netstandard2.0;net6.0;net7.0;net8.0</TargetFrameworks>
<LangVersion>10.0</LangVersion>
<PackageId>NLog.Targets.Loki</PackageId>
<Authors>Anton Gogolev, Corentin Altepe</Authors>
Expand All @@ -27,7 +27,10 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Net.Http" Version="2.2.29" />
<PackageReference Include="NLog" Version="5.2.8" />
<PackageReference Include="System.Net.Http" Version="4.3.4" />
Copy link
Owner Author

Choose a reason for hiding this comment

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

Condition these new packages to .NET Framework 4.8 only

<PackageReference Include="System.Text.Json" Version="8.0.2" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
Expand Down
Loading