datalust/seq-extensions-logging

Allow logging string as JSON object using the @ directive

omidkrad opened this issue ยท 11 comments

This is a feature request to add better support for JSON logging. My suggestion is to treat all string objects that are passed with the @ directive as JSON, and if it couldn't parse as JSON then record them as string. Logging without the @ directive should make it to log as string.

This makes the implementation more consistent with how seq-logging works in NodeJS that allows easy logging of JSON objects. It also allows the user to take care of the serialization to JSON which can work with both Newtonsoft and System.Text.Json.

Thanks for the suggestion; I think #45 has a better path to implementation (it fits with the current API and internals); perhaps we reopen that one?

Taking the string-as-JSON route would pose more questions about dealing with invalid JSON, parsing performance, etc.

Sure, we can reopen #45. So there are 3 suggestions right now:

  1. Serialize the object honoring JSON formatting attributes. This requires adding configuration to AddSeq(...) method to select JSON library (Newtonsoft or Dotnet) including JsonSerializerOptions.
  2. Allow serializing JsonDocument. However, it looks like JsonDocument can be created only when deserializing a JSON string. So with that, the object needs to be serialized to JSON and then deserialized to JsonDocument before being passed to the logger. Because of that, I don't think this is very intuitive.
  3. Logging JSON string specified with @ as JSON object, and fall-back to string if cannot parse as JSON because of invalid JSON etc. I think this is the most intuitive one because first, it allows selecting between JSON and string by specifying the @ directive or not, and second it allows the user to use any JSON serialization method they want.

Is there anyway I can simply pass a specific "don't parse" object, something like _log.BeginScope(new Dictionary<string, object>{ ("obj", new RawString(jsonEncodedString)) }. I hate how rigid this class is when it comes to formatting scope values.

For those of you stumbling upon this, I literally had to convert it to a json string (Using the JSON utility that was provided by another library), then use json.net to convert it back to a dictionary, only to have seq RECONVERT it back to json...what a computationally expensive, and dumb thing, but it works:

https://stackoverflow.com/questions/11561597/deserialize-json-recursively-to-idictionarystring-object

using var scope = _log.BeginScope(new KeyValuePair<string, object?>[]
{
    new("@Message", jsonMsgStr != null ? JsonConvert.DeserializeObject<IDictionary<string, object>>(jsonMsgStr, new DictionaryConverter()) : null)
});

Thanks @danbopes. We'll definitely loop back around to this one when we're able to ๐Ÿ‘

@nblumhardt Appreciate it.

For a quick and dirty fix, I really think any string value when the property has an @ before it, should not be treated as a literal string.

By specifying @, you're signaling that you want to attempt to destructure the value, and if it's a string, then I think you can safely assume that the string is in the correct format (Formatted as json).

Thanks for your reply. Unfortunately, it's a very common pattern in the wild for people to notice the @ in log statements and just do it everywhere (despite this not making much sense). I think the ship has already sailed on that one, and introducing new behavior behind @ now could cause a lot of unexpected breakage.

Something along the lines of:

using var scope = _log.BeginScope(new KeyValuePair<string, object?>[]
{
    new("@Message", new JsonString(jsonMsgStr))
});

could work (ignoring the problems of where to put the JsonString type) ๐Ÿค”

any progress on this?

Not recently, @tobiaszuercher, but this package is on our list for a refresh soon, so we'll keep this in mind.

Some work-in-progress! #51

This is now published in 6.1.0 ๐ŸŽ‰