datalust / seq-extensions-logging

Add centralized log collection to ASP.NET Core apps with one line of code.
https://datalust.co/seq
Apache License 2.0
86 stars 13 forks source link

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

Open omidkrad opened 3 years ago

omidkrad commented 3 years ago

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!

nblumhardt commented 3 years ago

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!

omidkrad commented 3 years ago

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.

nblumhardt commented 3 years ago

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 😃 ).

omidkrad commented 3 years ago

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.

nblumhardt commented 3 years ago

Thanks for the follow-up 👍

omidkrad commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Re-opening per #46