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
84 stars 12 forks source link

Exception Data property is not exist on Seq #12

Closed cilerler closed 7 years ago

cilerler commented 7 years ago

Following code

var exception = new Exception("USPS API error 'Invalid Address.  '");
exception.Data.Add("payload", address); //object
exception.Data.Add("response", xmlResponse); //string
_logger.LogError(-1, nex, "An error occured");

generates an output as

image

where it should also include Data in the fields.

nblumhardt commented 7 years ago

Thanks for the note. This one's a little tricky since the Exception might be an AggregateException, and each exception within the aggregate have data, and so-on. I might take a look at how SerilogExceptions handles this case and see if there's a sensible default we could use here 👍

cilerler commented 7 years ago

It looks like it doesn't provide Internal Exceptions neither which is fine, but not having Data forces my hand to keep adding additional code to push debugging variables and even worse have to do round-trip to find that data after filtering the error.

cilerler commented 7 years ago

This is definitely one of the very important missing point. 🤓

instead of code below

catch (Exception ex)
{
    ex.Data.Add("key", _keyAuthentication);
    _logger.LogError(-1, ex, ex.Message);
}

I have to write like this 😱

catch (Exception ex)
{
    _logger.LogError(-1, ex, $"{ex.Message} {{key}}", _keyAuthentication);
}

You may say it is not that bad, well for a single variable yes but when you have 10 variables it would be very ugly. 🙄

cilerler commented 7 years ago

By the way, my 2 cents says you should add Exception Data fields on top of red exception box above the stack trace. 😎

cilerler commented 7 years ago

is there any chance you may put this enhancement to in-front of the line? 😊