Improve logging template/scopes, or provide a logging delegate?

[abagonhishead]

Currently, StrongGrid.Client's message template passed to ILogger, while including lots of useful information, is a bit messy.
The request verb, request URL, request headers, response body, response headers and diagnostic/timing information all appear to be concatenated/interpolated into one long string, rather than using properties or scopes to attach useful information to the event. For example:

// Bad
logger.Log(LogEventLevel.Information, $"REQUEST: {request.Method} {request.RequestUri} {request.Headers} {request.Body} RESPONSE: {response.Headers} {response.Body}");

// Good
using (logger.BeginScope("{RequestHeaders}", request.Headers))
{
  logger.Log(LogEventLevel.Information, "SendGrid request: {RequestMethod} {RequestUri}", request.Method, request.RequestUri);

  // ... get response & read body ...

  using(logger.BeginScope("{ResponseHeaders}", response.Headers))
  using(logger.BeginScope("{ResponseBody}", response.Body))
  {
    logger.Log(LogEventLevel.Information, "SendGrid response: {ResponseStatusCode}/{ResponseStatus} for {RequestMethod} {RequestUri}", (int)response.StatusCode, response.StatusCode.ToString(), request.Method, request.RequestUri)
  }
}

This greatly affects readability for anyone using property-based logging solutions (e.g. Seq)

Could we use scopes and message templates for this instead, as demonstrated above?

Also acceptable would be the option to pass a logging delegate into the client, ideally for both the request & the response, which would then disable the default logging calls.

Better still would be supporting something like Flurl which allows people to do this for themselves easily using their existing HTTP client implementation(s), but I realise that's probably a bit of a faff and introduces a lot of dependencies! ;)

Happy to have a look at this myself and raise a PR if you think it's acceptable.

Cheers.

[Jericho]

Interesting idea. I definitely can see the benefit to those using Seq. I'll need a little while to think about how to implement this. I'll set a side some time this weekend to work on it.

If I publish beta nuget packages, would you have time to test and give me some feedback in the next few days/week?

[Jericho]

Regarding your suggestion to use Flurl: this sounds like we would have to replace all our code when making http requests. I'm sure we could figure how to do that but it would definitely be a lot of work, which I don't have much desire to do.

< impersonates Marc Cuban on Shark Tank >For all these reasons... I"M OUT!
😉

[Jericho]

I worked on this over the weekend and I have a proof of concept.

Firstly, I tried using (logger.BeginScope("{RequestHeaders}", request.Headers)) but what I observed is that the values in the dictionary passed to the BeginScope method are being lost in the three environments I use for testing purposes: console, Microsoft Application insights in Azure and Logz.io. I assume it's different in your environment and that Seq can pickup these value (I have never used Seq so I don't know for sure) but I'm not willing to compromise the information available when a developer is not using something similar to Seq.

My second attempt, however, was much more successful: I enhanced the message template and made sure that every single data point is a named parameter. This results in all the necessary info available in non-structured logging environments (like the console for example) as you can see below:

and all the named parameters being captured as "custom properties" in systems like Azure Application Insights:

and similarly in logz.io:

I have published StrongGrid 0.107.0-structured-loggi0014 to my personal nuget feed, let me know if you have a chance to beta test it.

[Jericho]

@abagonhishead Just checking if you had a chance to try the beta package? Also, FYI I published an updated beta version to fix a few minor details.

[Jericho]

🎉 This issue has been resolved in version 0.107.0 🎉

The release is available on:

• GitHub Release
• NuGet Package

Your GitReleaseManager bot 📦🚀