seq-extensions-logging icon indicating copy to clipboard operation
seq-extensions-logging copied to clipboard

Capture `System.Text.Json` documents directly as structured data

Open omidkrad opened this issue 4 years ago • 8 comments

I'm using System.Text.Json.Serialization attributes/convertors for configuring serialization of my classes, but I think Seq logger is using Newtonsoft.Json so when I use the @ character to log an object like Logger.LogInformation("{@Item}", Item); it uses the default serialization all will include all [JsonIgnore] properties which makes the object very bloated and large. My request is to add an option in .AddSeq to be able to chose the serialization method between Newtonsoft and Microsoft's. I think more people will need this as more people use dotnet's JSON library for their new projects. Thanks!

omidkrad avatar Jun 14 '21 21:06 omidkrad

Hi @omidkrad - thanks for getting in touch!

Seq.Extensions.Logging uses its own internal policies based on Serilog's, so it doesn't use any of the JSON.NET or System.Text.Json attributes, unfortunately.

Other than migrating over to the full Serilog package, you can control this by e.g. constructing an anonymous object, or by mapping to a logging-specific representation:

Logger.LogInformation("{@Item}", new { Item.Name, Item.Id });

Let me know if this helps!

nblumhardt avatar Jun 14 '21 23:06 nblumhardt

Isn't it possible if I first serialize to JSON string to get the JSON formatting I want and then pass that to the logger as JSON (not string)? I really wished if I could just use my current JSON formatters and converters and don't want to reimplement them in Serilog.

omidkrad avatar Jun 15 '21 00:06 omidkrad

Do you mean serialized then deserialize to System.Text.Json's JsonDocument? If so, I did put together something for Serilog to make this work:

https://gist.github.com/nblumhardt/8aea89e86663829170d08aa573bd0f49

Building in support for System.Text.Json here seems like it would be interesting to explore, but it's probably not feasible in the very short-term unless someone is able to figure it out/send a fleshed-out PR to get the ball rolling (the Seq team's hard at work shipping 2021.3, at least for the next month or so 😃 ).

nblumhardt avatar Jun 15 '21 00:06 nblumhardt

Yes, I was thinking serializing and then deserializing to an ExpandoObject, but JsonDocument seems better suited here. Thanks anyways for now I think I could get around it by mapping to a smaller log-specific object. Pro of that we force ourselves to keep the log object smaller, cons is that I cannot take advantage of my json formatters. Cheers.

omidkrad avatar Jun 15 '21 04:06 omidkrad

Thanks for the follow-up 👍

nblumhardt avatar Jun 15 '21 05:06 nblumhardt

I wanted to add that JsonDocument seems to be the proper type for passing a JSON object to the logger and it makes sense for Seq implementation for ILogger to log it correctly just like how seq-logging works in NodeJS.

Currently, instead of the JSON contents it shows this:

{
  RootElement: {    
    ValueKind: 'Object'
  }
}

Another way 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. And logging without the @ directive make it to log directly as string.

omidkrad avatar Jun 16 '21 22:06 omidkrad

I created a new issue for this (#46) because when I compare this with seq-logging for NodeJS, I'm finding myself doing a lot more work for logging any objects whereas in seq-logging everything is just JSON.

omidkrad avatar Jun 23 '21 18:06 omidkrad

Re-opening per #46

omidkrad avatar Jun 24 '21 06:06 omidkrad