Skip to content

Feature request: Add DateOnly Converter to Logging JsonSerializer #501

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

Closed
1 of 2 tasks
BradKnowles opened this issue Oct 6, 2023 · 6 comments · Fixed by #503
Closed
1 of 2 tasks

Feature request: Add DateOnly Converter to Logging JsonSerializer #501

BradKnowles opened this issue Oct 6, 2023 · 6 comments · Fixed by #503
Labels
area/logging Core logging utility feature-request New or enhancements to existing features Serialization

Comments

@BradKnowles
Copy link

BradKnowles commented Oct 6, 2023

Use case

If the object being logged has a DateOnly property the Logging framework throws an exception.

Serialization and deserialization of 'System.DateOnly' instances are not supported. The unsupported member type is located on type 'System.Nullable``1[System.DateOnly]'

Solution/User Experience

Ideally, the Logging framework would handle DateOnly fields without any additional or custom logic from application developers.

Alternative solutions

At the moment, the workaround is to add a [JsonConverter(typeof(DateOnlyJsonConverter))] attribute to each property of the object being serialized, where DateOnlyConverter is a custom converter that is each of our applications.

Acknowledgment

@BradKnowles BradKnowles added feature-request New or enhancements to existing features triage Pending triage from maintainers labels Oct 6, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 6, 2023

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #dotnet channel on our Powertools for AWS Lambda Discord: Invite link

@hjgraca hjgraca added area/logging Core logging utility Serialization and removed triage Pending triage from maintainers labels Oct 9, 2023
@hjgraca
Copy link
Contributor

hjgraca commented Oct 9, 2023

Hi @BradKnowles , this is a .Net 6 limitation System.Text.Json source generator does not have built-in support for DateOnly/TimeOnly types, this was added to .NET 7. https://devblogs.microsoft.com/dotnet/system-text-json-in-dotnet-7/#source-generation-improvements and dotnet/runtime#53539.

This is an issue that will be present in other utilities like Tracing, due to the way XRay SDK does serialization. aws/aws-xray-sdk-dotnet#212

We will discuss this internally to see if there is anything we can do to improve the experience. At the moment the solution is to create the converters.

@hjgraca
Copy link
Contributor

hjgraca commented Oct 9, 2023

cc: @amirkaws

@BradKnowles
Copy link
Author

I was thinking it could be added like the ByteArrayConverter etc. was added in PR #152.

@hjgraca
Copy link
Contributor

hjgraca commented Oct 11, 2023

@BradKnowles we will be adding custom converter for DateOnly and TmeOnly. Thanks for raising the issue. We will be updating this issue with the progress.

@hjgraca hjgraca linked a pull request Oct 11, 2023 that will close this issue
7 tasks
@hjgraca hjgraca moved this from 🆕 New to 👀 In review in Powertools for AWS Lambda (.NET) Oct 11, 2023
@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Oct 12, 2023
@hjgraca
Copy link
Contributor

hjgraca commented Oct 30, 2023

@BradKnowles released in version 1.8.1 - Logging version 1.3.3.
Really appreciate opening this issue and the feedback.

@hjgraca hjgraca removed the pending-release Fix or implementation already in dev waiting to be released label Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging Core logging utility feature-request New or enhancements to existing features Serialization
Projects
Status: 👀 In review
Development

Successfully merging a pull request may close this issue.

2 participants