Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect date format when using trace sink #14

Open
2 of 8 tasks
omfgicbf opened this issue Apr 1, 2020 · 2 comments
Open
2 of 8 tasks

Incorrect date format when using trace sink #14

omfgicbf opened this issue Apr 1, 2020 · 2 comments

Comments

@omfgicbf
Copy link

omfgicbf commented Apr 1, 2020

I know the preamble says to log sink specific stuff in the sink repo, however, I've checked the code for the trace sink and cannot find any sink specific date handling code.

Does this issue relate to a new feature or an existing bug?

  • Bug
  • New Feature

What version of Serilog is affected? Please list the related NuGet package.

Serilog 2.9.0
Serilog.Sinks.Trace 2.1.0

What is the target framework and operating system? See target frameworks & net standard matrix.

  • .NET Core 3.1
  • netCore 2.0
  • netCore 1.0
  • 4.7
  • 4.6.x
  • 4.5.x

Please describe the current behaviour?

The trace sink logs date/time in US format (which is not good, because that's not where I am):-

2020-04-02 09:37:54.463 +11:00 [Debug] timestamp: 04/02/2020 00:05:00
2020-04-02 10:27:49.677 +11:00 [Debug] "en-AU"

Also, notice the inconsistency... the entry date/time is in some custom format, yet DateTimes in output default to ? (I couldn't find the corresponding standard date/time format string).

Debug.WriteLine gets it right:-

timestamp: 2/04/2020 12:05:00 AM

As does the file sink (sort of. better, but not consistent):-

2020-04-02 09:37:54.463 +11:00 [DBG] timestamp: "2020-04-02T00:05:00.0000000+11:00"

Please describe the expected behaviour?

Use CurrentCulture instead of assuming I'm in the US.
2020-04-02 09:37:54.463 +11:00 [Debug] timestamp: 02/04/2020 00:05:00

Or at least keep the defaults consistent...
2020-04-02 09:37:54.463 +11:00 [Debug] timestamp: 2020-04-02 00:05:00.000 +11:00

If the current behaviour is a bug, please provide the steps to reproduce the issue and if possible a minimal demo of the problem

.WriteTo.Trace()

log.Debug("timestamp: {0}", ts);
log.Debug("{0}", System.Globalization.CultureInfo.CurrentCulture);

I notice this is marked as fixed in serilog/serilog#146 but doesn't seem to apply to the trace sink.

I see at https://github.com/serilog/serilog/wiki/Formatting-Output that I can work around the issue (I think, haven't tried it yet), but shouldn't these all be consistent and use CurrentCulture by default?

Instead of using InvariantCulture everywhere, could it keep a CultureInfo variable somewhere (default to InvariantCulture if it has to) and let us override in the setup?

It may seem trivial, but can be really annoying when you're working with dates in the first 12 days of the month or adding the same workaround code in every project just for trace.

@omfgicbf
Copy link
Author

omfgicbf commented Apr 1, 2020

OK, so after trying to implement my own format provider to get the date format right, I noticed you don't have to, and can just .WriteTo.Trace(formatProvider: new CultureInfo("en-AU")), which is great.

So only a few things I guess:-

  • Why does it go out of its way to change the default behaviour of ToString() in the first place?
  • The format provider doco is a bit ambiguous; there are two examples mixed up in one and the suggestion on changing the date format sort of implies that you have to implement your own format provider. It would be good if it just said "defaults to InvariantCulture, so just WriteTo.Trace(formatProvider: new CultureInfo("en-AU")) unless you really need a custom format.
  • Why is the default behaviour of the trace sink different to the other sinks?

@nblumhardt
Copy link
Member

Thanks for the heads-up! Moving this to the Serilog.Sinks.Trace repository; if the behavior's diverged from default Serilog then this would be fair game for a change/bugfix PR, if anyone is interested in digging in further 👍

@nblumhardt nblumhardt transferred this issue from serilog/serilog Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@nblumhardt @omfgicbf and others