MSsql sink writes times as local time #392 #1
Conversation
| @@ -45,14 +46,14 @@ public static class LoggerConfigurationMSSqlServerExtensions | |||
| LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum, | |||
| int batchPostingLimit = MSSqlServerSink.DefaultBatchPostingLimit, | |||
| TimeSpan? period = null, | |||
| IFormatProvider formatProvider = null) | |||
| IFormatProvider formatProvider = null, bool utcTimestamp = true) | |||
There was a problem hiding this comment.
By forcing it to true, you do change the default behaviour. Would an opt in not be better?
There was a problem hiding this comment.
+1 for an opt-in, plus it's convention throughout Serilog that optional Booleans default to false, so helps there too.
Also missing a carriage return to keep the formatting clean in this parameter list.
There was a problem hiding this comment.
Also, perhaps we could name it more specifically - something like storeTimestampInUtc? The current name at first glance sounds like it is the timestamp. :-)
|
Looks good to me, just some minor issues mentioned in the comments. We will also need to version this as 2.0, because the API of the Cheers! |
|
Thanks, this looks great, let's kick it off! |
MSsql sink writes times as local time #392
* Updated reference to Serilog version 2.5.0 as suggested in #110 * Added a new Sink `MSSqlServerAuditSink` that persists LogEvents to the database immediately and propagates any errors that occur. * Moved, and refactored common functionality from `MSSqlServerSink` into `MSSqlServerSinkTraits` which is now used by both sinks.
…ormat Make sure the JSON has a valid structure.
Sync to base
Hi,
Here is an implementation for the requested enhancement.
Let me know what you think.
Thanks,
Michael