borisermakof / serilog-sinks-fluentd

A Sink that writes logs into Fluentd
Apache License 2.0
20 stars 30 forks source link

Add support for structured logging & some minor enhancements #15

Closed Pzixel closed 4 years ago

Pzixel commented 4 years ago

I'm facing severe issues with current implementation so I managed to spend some time to fix them:

  1. Everythig in quotes. Becuase of that you cannot query dates and number ranges
  2. Missing null values handling
  3. Missing structured values handling
  4. Missing template interpolation
  5. Missing culture info when formatting values
  6. Thread.Sleep replaced with proper Task.Delay
  7. Minor enhancements

This is my test code I used to check everything works:

static void Main(string[] args)
{
    var log = new LoggerConfiguration().WriteTo.Fluentd(new FluentdSinkOptions("127.0.0.1", 24224, "myindex")).CreateLogger();
    var argumentException = new ArgumentException();
    var propertyValue1 = DateTime.Now;
    var propertyValue0 = new
    {
        InnerValue = new {A = 1, B = 2},
        InnerArr = new[] {1,2,3},
        InnerArrWithStruct = new[] {new {C = 1, D = 2}, new {C = 1, D = 2}},
        Nullable = new int?(10),
        NullNullable = new int?(),
        Id = Guid.NewGuid(),
        Text = "HELLO THERE2",
        Span = TimeSpan.FromMilliseconds(123456789),
        DateLocal = propertyValue1,
        DateUtc = DateTime.UtcNow,
        DateOffsetLocal = DateTimeOffset.Now,
        DateOffsetUtc = DateTimeOffset.UtcNow,
        Number = 42
    };
    log.Error(argumentException, "Test template {@Struct}, Date = {Date}, Value = {Value}",
        propertyValue0,
        propertyValue1,
        45);
    log.Information(argumentException, "Test template {@Struct}, Date = {Date}, Value = {Value}",
        propertyValue0,
        propertyValue1,
        45);
    Thread.Sleep(5000); // because of batcher that doesn't send data immediately
}

Fixes #14

Pzixel commented 4 years ago

Ping?

We really need these changes. If they don't go into master then we had to issue the fork as our own package. But i'd like to share changes with upstream.

borisermakof commented 4 years ago

Hello. Thanks for your pr, in few ours will merge and upload new package