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

chore: Logging AOT Support #628

Open
wants to merge 28 commits into
base: develop
Choose a base branch
from

Conversation

hjgraca
Copy link
Contributor

@hjgraca hjgraca commented Aug 2, 2024

Please provide the issue number

Issue number: #212

Summary

Changes

  • Logging support for native AOT
  • .NET 8 specific serialization paths
  • New helper method to convert anonymous types to dictionary, use for source generation .NET 8
  • Removed PowertoolsLambdaContext now uses ILambdaContext
  • Removed dependency from IMethodAspectHandler, now using LoggingAspectFactory to boostrap Logging Aspect
  • Added a new Source Generator - PowertoolsSourceGeneratorSerializer to be used in Lambda bootstrap

User experience

With the new PowertoolsSourceGeneratorSerializer, we can combine Powertools SerlializationContext with the customer Serialization context, this enables Powertools to serialize customer only types, this is useful for LogEvent feature.

image

Checklist

Please leave checklist items unchecked if they do not apply to your change.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

…fic paths. New helper method to convert anonymous types to dictionary, use for source generation .NET 8
@boring-cyborg boring-cyborg bot added area/logging Core logging utility tests labels Aug 2, 2024
@auto-assign auto-assign bot requested review from amirkaws and sliedig August 2, 2024 10:13
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 2, 2024
@hjgraca hjgraca added the aot label Aug 2, 2024
@hjgraca hjgraca added this to the .NET AOT milestone Aug 2, 2024
@github-actions github-actions bot added the internal Maintenance changes label Aug 2, 2024
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 61 lines in your changes missing coverage. Please review.

Project coverage is 73.01%. Comparing base (d99a3e0) to head (86b8f85).
Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
...ambda.Powertools.Logging/Internal/LoggingAspect.cs 76.92% 28 Missing and 5 partials ⚠️
...ging/Internal/PowertoolsConfigurationsExtension.cs 96.07% 1 Missing and 5 partials ⚠️
...Serializers/PowertoolsSourceGeneratorSerializer.cs 68.75% 3 Missing and 2 partials ⚠️
.../Internal/Converters/ExceptionPropertyExtractor.cs 66.66% 4 Missing ⚠️
...raries/src/AWS.Lambda.Powertools.Logging/Logger.cs 63.63% 1 Missing and 3 partials ⚠️
....Logging/Internal/Converters/ExceptionConverter.cs 50.00% 3 Missing ⚠️
...da.Powertools.Logging/Internal/PowertoolsLogger.cs 96.00% 1 Missing and 2 partials ⚠️
...Powertools.Common/Core/PowertoolsConfigurations.cs 50.00% 0 Missing and 1 partial ⚠️
...owertools.Logging/Internal/LoggingLambdaContext.cs 97.14% 0 Missing and 1 partial ⚠️
...Logging/Serializers/PowertoolsLoggingSerializer.cs 98.55% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #628      +/-   ##
===========================================
+ Coverage    72.76%   73.01%   +0.25%     
===========================================
  Files          190      195       +5     
  Lines         7886     7972      +86     
  Branches       850      852       +2     
===========================================
+ Hits          5738     5821      +83     
- Misses        1859     1866       +7     
+ Partials       289      285       -4     
Flag Coverage Δ
unittests 73.01% <88.88%> (+0.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@boring-cyborg boring-cyborg bot added area/common Core Powertools utility documentation Improvements or additions to documentation labels Aug 29, 2024
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 29, 2024
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 11, 2024
@hjgraca hjgraca changed the title chore: Logging AOT Support WIP chore: Logging AOT Support Sep 16, 2024
@hjgraca
Copy link
Contributor Author

hjgraca commented Sep 18, 2024

  • documentation

/// </param>
private void CaptureLambdaContext(AspectEventArgs eventArgs)
{
_clearLambdaContext = LoggingLambdaContext.Extract(eventArgs);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check where LambdaContext is used, if just here just use the Ilambda here


#if NET8_0_OR_GREATER

/// <summary>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge methods so consumer dont need to have #if

Copy link

sonarcloud bot commented Sep 20, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aot area/common Core Powertools utility area/logging Core logging utility documentation Improvements or additions to documentation internal Maintenance changes size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tests
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

1 participant