datalust / seq-api

HTTP API client for Seq
https://datalust.co/seq
Apache License 2.0
77 stars 21 forks source link

Streaming events: can't deserialize `Trace` level (writing events with MS.Ext.Logging) #127

Open migajek opened 7 months ago

migajek commented 7 months ago

tl;dr: When writing logs with MS Ext Logging and streaming them back to application as demonstrated in this library documentation, deserializing LogEvent using LogEventReader.ReadFromJObject fails with exception "Requested value 'Trace' was not found".

long verson: I'm using Seq as a logging storage for Microsoft.Extensions.Logging based app (using Seq.Extensions.Logging) Therefore the message levels stored in Seq are levels defined by MS Ext Logging.

When streaming events back to the application, the documentation states the events should be deserialized into Serilog's LogEvent instances (using LogEventReader.ReadFromJObject) - unfortunately, the level is Serilog's LogEventLevel which is not compatible with MS Ext Logging levels (Trace vs Verbose and Critical vs Fatal)

Technical aspects of the issue aside, I find it a little inconsistent:

migajek commented 7 months ago

Quick and a little dirty workaround for now:

_stream
                .Select(FixEntryLoggingLevel)
                .Select(LogEventReader.ReadFromJObject)
                .Subscribe(...)

...

  private static JObject FixEntryLoggingLevel(JObject entry)
    {
        if (!entry.TryGetValue("@l", out var levelToken))
        {
            return entry;
        }

        var value = levelToken?.Value<string>();
        entry["@l"] = value switch
        {
            "Trace" => "Verbose",
            "None" => "Verbose",
            "Critical" => "Fatal",
            _ => value
        };
        return entry;
    }
nblumhardt commented 7 months ago

Hi @migajek; thanks for raising this.

LogEventReader is part of Serilog.Formatting.Compact.Reader, which this package doesn't depend on or use; unfortunately it's a Serilog component and will likely only work when the events originated from Serilog.

The JSON transformation trick you're using here is probably the best way to go, for now; having it mentioned here will hopefully help anyone who hits this to work around it in the future 👍

migajek commented 7 months ago

Hi @nblumhardt, thanks - I understand LogEventReader belongs to Serilog - I'm not however suggesting the change to how reader works, rather what the API exposes :)

I don't really understand why Seq provides different JSON representation for streaming API, comparing to fetching API. I suppose you have good reasons for that (be it performance or some internal technical considerations).

However , if I may suggest - I'd consider providing the same EventEntity on both surface APIs.

I am assuming my usage scenario might be quite common - I'm combining past and incoming events together: fetch last N events for given filter, and subscribe to incoming events using the same filter. It just feels strange to use two different models for the same piece of data :)

nblumhardt commented 7 months ago

That makes sense - thanks @migajek. Does need some more thought at our end :+1:

migajek commented 7 months ago

thanks :) sure, I'm fine with the workaround for now. Will keep an eye on this cheers