Optimizations

[andrewmd5]

This PR addresses a few memory leaks and aims to reduce runtime allocations. Here are some of the changes:

• Await individual task directly rather than allocating multiple task into a single group.
• Forward exceptions to OnException directly to avoid delegate allocations.
• Because we know the max size of a chunk, I've Introduced a reusable StringBuilder to the DatadogHttpClient and LogFormatter so batches don't reallocate.
• Add a lightweight semaphore to ensure only one batch of events is in-flight at any given time. (needed to allow for reusable builders)
• Adds a flowing cancellation token that will ensure resources get released when the sink is disposed, or if a client breaks.
• Adds a delay to the sink dispose method to ensure pending batches get written.
• Constructs chunks used by the HttpClient inline.
• Other minor fixes.

Tested and haven't seen any issues. We run our .NET code in environments with very little memory (1 GB) and these changes help ensure logging isn't creating memory pressure.

[andrewmd5]

One note - it would be possible to entirely remove the round-trip JSON de(serialization) that occurs in the formatter by leveraging the LogEvent properties. For instance, calling logEvent.AddPropertyIfAbsent(new LogEventProperty("source", new ScalarValue(_source))); would set the source under the properties object, which gets serialized like this:

{
      "Timestamp":"2021-11-01T15:52:29.6217246-07:00",
      "level":"Error",
      "MessageTemplate":"Yo!",
      "message":"Yo!",
      "Exception":"System.Exception: test 1",
      "Properties":{
         "source":"csharp"
      },
      "host":"DESKTOP-I1EFOH0"
   }

~But this would be a breaking change that would require the Datadog backend to be updated presumably. Alternatively, I can try implementing a custom ITextFormatter that achieves the same result in a separate PR.~

~I’ve got a custom formatter working that removes the need for JSON serializers; formatting is now a single operation. Will stage in a separate PR.~ I included it in this PR.

[andrewmd5]

I've put the resource recycling behind a flag on the configuration class. I've also gone ahead and removed the dependency on System.Text.Json and Newtonsoft.Json. I implemented a custom Serilog formatter which forks the built in JSON formatter and writes data in the format the original implementation was trying to achieve without the roundtrip serialization.

Quick test show formatting now only takes a few MS on every target and allocations are down even without recycling. Pushed data to our Datadog account and everything looks good. This should be ready to merge.

[andrewmd5]

@gh123man I was also curious- once this is merged is it possible to get a new release on Nuget? Even if it’s pre-release that would work for us.

[gh123man]

@AndrewMD5 Yep sure thing - ill publish a release once this goes in.

[andrewmd5]

Can you explain why you added CancellationToken?

Cancellation is cooperative in .NET. Everything in the call-chain has to be explicitly passed the CancellationToken in order for it to work well. This means you need to explicitly pass the token into other APIs that take a token if you want cancellation to be most effective. So to dispose the sink, we have to dispose all the resources the sink created, and cancel any task those resources might be currently executing. This also helps avoid runtime exceptions.

[andrewmd5]

Can you also rebase your PR from origin/master to hide change related to #68?

IIRC it already is based on origin/master; do you mean since #68 is merged in already that you'd prefer if I rebase off 4c0dab81dd2b6e1a7f6ddbfc385f86ee90501644 to hide the changes #68 made?

[ogaca-dd]

@AndrewMD5: Thank you for your quick feedback. As the PR has a lot of changes and as I have a busy week, I cannot review the PR this week, sorry for that. I will review this PR next week.

[andrewmd5]

@AndrewMD5: Thank you for your quick feedback. As the PR has a lot of changes and as I have a busy week, I cannot review the PR this week, sorry for that. I will review this PR next week.

Do you think you'll be able to get this reviewed this week?

[andrewmd5]

Happy new year folks! I’ll address the comments this weekend and get any necessary changes in.

[jszwedko]

Closing due to lack of response, but feel free to reopen if you want to pick this back up! Thanks for the initial efforts here!