serilog / serilog-aspnetcore

Serilog integration for ASP.NET Core
Apache License 2.0
1.32k stars 209 forks source link

7.0.0 broke structured logging to `ElasticSearch` because of `DateOnly` #331

Closed tautvydasversockas closed 1 year ago

tautvydasversockas commented 1 year ago

Description

My logging stack is a combination of Serilog and ELK stack. I upgraded Serilog.AspNetCore form version 6.1.0 to version 7.0.0. I believe this update might have impacted how structured logging treats DateOnly properties. I couldn't find any documentation about it though. As a result of this upgrade, ElasticSearch indexes couldn't parse DateOnly data types which meant that the logs were not visible.

Code that was used for logging (obviously I simplified it a bit):

public record MyDto(DateOnly Date);

var dateTime = new DateTime(2022, 12, 7);
var dateOnly = DateOnly.FromDateTime(dateTime);
var myDto = new MyDto(dateOnly);

_logger.LogInformation("Message {@myDto}.", myDto);

What I saw in the logs before the Serilog.AspNetCore upgrade:

Message MyDto { Date: DateOnly { Year: 2022, Month: 12, Day: 7, DayOfWeek: Wednesday, DayOfYear: 341, DayNumber: 738495 } }.

What I saw in the logs after the Serilog.AspNetCore upgrade:

What I saw in ElasticSearch logs:

...
org.elasticsearch.index.mapper.MapperParsingException: object mapping for [fields.MyDto.Date] tried to parse field [Date] as object, but found a concrete value
...

What I saw in the logs after the Serilog.AspNetCore upgrade and after I rebuilt my indexes in ElasticSearch:

Message MyDto { Date: 07/12/2022 }.

Question

Is this breaking change expected? Should this be documented somewhere (i.e. release notes)?

Versions

.NET - net7.0 Serilog.AspNetCore - 7.0.0 Serilog.Sinks.LogstashHttp - 1.2.0 Serilog.Sinks.PeriodicBatching - 3.1.0 ElasticSearch - v 7.17.5

nblumhardt commented 1 year ago

Hello! Thanks for dropping us a line.

DateOnly is treated as a scalar value in Serilog 2.12, in line with how DateTime, DateTimeOffset, and friends are handled:

https://github.com/serilog/serilog/blob/dev/src/Serilog/Formatting/Json/JsonValueFormatter.cs#L509

This has been the case for a long time, but my guess is that the earlier version of this package that you were depending on brought in an earlier version of Serilog, released before DateOnly was in wide use.

Sorry about the breakage, here; there are two workarounds you might try.

First, you might change the fields.MyDto field name so that Elastic keeps separate indexes for the old and new versions of the data.

Or second, you can Destructure.ByTransforming<MyDto>(() => ...), using the callback to convert MyDto into a similar type that uses a replacement struct instead of DateOnly with the various properties (Year, Month, ...) exposed.

Let me know if either does the trick!

tautvydasversockas commented 1 year ago

Hello, thanks for a quick reply. I really appreciate you providing multiple workarounds! However, at the moment I fixed the problem by rebuilding ElasticSearch indexes so there's no urgent problem from my side. The reason for this bug report is to inform you (and other users that might face the same issue in the future) about this breakage. Have a lovely day!

nblumhardt commented 1 year ago

Awesome, thanks. Closing this as by design 👍