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.

Sign up for GitHub

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

Jump to bottom

improv: improve logger extra keys feature #244

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

improv: improve logger extra keys feature #244

wants to merge 4 commits into from

Conversation

amirkaws
Copy link
Contributor

@amirkaws amirkaws commented Apr 14, 2023 •
edited
Loading

Please provide the issue number

Issue number: #166

Summary

This PR improves extra keys feature of logger.

Changes

Please provide a summary of what's being changed

  • it accepts any type as provided not only type class
  • it adds extra data provided under "Extra" key if user does not provide key name

User experience

1. Typed Object Type as Extra Key

For typed class key name will be class name/type name for both generic and non-generic types:

public class LookupInfo => LookupInfo
public class LookupInfo<T> => LookupInfo
public class LookupInfo<T0,T1,T2,...> => LookupInfo

Example:

var lookupInfo = new LookupInfo { LookupId = requestContextRequestId };
Logger.LogInformation(lookupInfo, "This is a log with typed object additional data");

Output:

{
    "cold_start": true,
    "lookup_info": {
       "lookup_id": "4c50eace-8b1e-43d3-92ba-0efacf5d1625"
    },
    "message": "This is a log with typed object additional data",
}

2. Anonymous Object Type as Extra Key

For anonymous class key name will be "Extra" for example:

var lookupInfo = new { LookupId = requestContextRequestId };
Logger.LogInformation(lookupInfo, "This is a log with anonymous type object additional data");

Output:

{
    "cold_start": true,
    "extra": {
       "lookup_id": "4c50eace-8b1e-43d3-92ba-0efacf5d1625"
    },
    "message": "This is a log with anonymous type object additional data",
}

3. Anonymous Object Type as Extra Key With Specified Key

When specify the key name for extra additional data (acceptable are tuple or key/value pair) for example:

var lookupInfo = new { LookupId = requestContextRequestId };
Logger.LogInformation(("LookupInfo", lookupInfo), "This is a log specifying the key name for additional data");

Output:

{
    "cold_start": true,
    "lookup_info": {
       "lookup_id": "4c50eace-8b1e-43d3-92ba-0efacf5d1625"
    },
    "message": "This is a log specifying the key name for additional data",
}

4. Multiple Key/Value Pairs

When passing multiple key/value pairs (acceptable are Dictionary or list or key/value pair or array of key/value pair ) for example:

var extraKeys = new Dictionary<string, object>()
{
     { "LookupInfo", new { LookupId = requestContextRequestId } },
     { "CorrelationIds", new { MyCorrelationId = "correlation_id_value" } }
};
Logger.LogInformation(extraKeys, "This is a log with multiple extra key/value pairs");

Output:

{
    "cold_start": true,
    "lookup_info": {
       "lookup_id": "4c50eace-8b1e-43d3-92ba-0efacf5d1625"
    },
    "correlation_ids": {
       "my_correlation_id": "correlation_id_value"
    },
    "message": "This is a log with multiple extra key/value pairs",
}

Please share what the user experience looks like before and after this change

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.

@auto-assign auto-assign bot requested a review from hjgraca April 14, 2023 16:07
@codecov-commenter
Copy link

codecov-commenter commented Apr 14, 2023 •
edited
Loading

Codecov Report

Patch coverage: 69.23% and project coverage change: +0.19 🎉

Comparison is base (8bb7863) 56.29% compared to head (4d76324) 56.48%.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #244      +/-   ##
===========================================
+ Coverage    56.29%   56.48%   +0.19%     
===========================================
  Files           41       41              
  Lines         1796     1827      +31     
===========================================
+ Hits          1011     1032      +21     
- Misses         785      795      +10
Flag Coverage Δ
unittests 56.48% <69.23%> (+0.19%) ⬆️

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

Impacted Files Coverage Δ
...raries/src/AWS.Lambda.Powertools.Logging/Logger.cs 5.51% <ø> (ø)
.../AWS.Lambda.Powertools.Logging/LoggerExtensions.cs 36.84% <40.00%> (+0.20%) ⬆️
...da.Powertools.Logging/Internal/PowertoolsLogger.cs 87.75% <73.52%> (-3.27%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@amirkaws amirkaws added the do-not-merge PRs that are blocked for varying reasons label Apr 16, 2023
@amirkaws amirkaws marked this pull request as draft April 16, 2023 07:51
This was linked to issues May 30, 2023
If extra keys has property "name", the logger name in the output is overwritten #229
Open
Format of JSON not as expected #230
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjgraca hjgraca Awaiting requested review from hjgraca

At least 1 approving review is required to merge this pull request.

Assignees

@hjgraca hjgraca

Labels
do-not-merge PRs that are blocked for varying reasons
Projects
None yet
Milestone
No milestone
3 participants
@amirkaws @codecov-commenter @hjgraca