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

Feature request: add additional log level: TRACE & corresponding logger.trace() method #1589

Closed
2 tasks done
codyfrisch opened this issue Jul 7, 2023 · 7 comments · Fixed by #2902
Closed
2 tasks done
Assignees
Labels
completed This item is complete and has been merged/shipped feature-request This item refers to a feature request for an existing or new utility good-first-issue Something that is suitable for those who want to start contributing logger This item relates to the Logger Utility

Comments

@codyfrisch
Copy link

codyfrisch commented Jul 7, 2023

Use case

Create an additional log level of TRACE below DEBUG

DEBUG contains statements about entering specific functions, iterations of a loop, timing info, etc. as well as actual data such as function parameters, API responses etc.

INFO is reserved for normal informational messages, like logging the results of an invocation but not all the intermediate details.

TRACE level would allow DEBUG to only contain the data, API responses, etc. and TRACE then contains the statements for the execution path.

Solution/User Experience

Add an additional level for TRACE as was done for CRITICAL.

Alternative solutions

No response

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@codyfrisch codyfrisch added triage This item has not been triaged by a maintainer, please wait feature-request This item refers to a feature request for an existing or new utility labels Jul 7, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Jul 7, 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 #typescript channel on our Powertools for AWS Lambda Discord: Invite link

@dreamorosi dreamorosi added need-customer-feedback Requires more customers feedback before making or revisiting a decision discussing The issue needs to be discussed, elaborated, or refined logger This item relates to the Logger Utility and removed triage This item has not been triaged by a maintainer, please wait labels Jul 8, 2023
@dreamorosi
Copy link
Contributor

Hi @codyfrisch, thank you for taking the time to open an issue on our repo and for explaining the reasoning behind this new log level.

At least on the surface I see the addition of a VERBOSE level in between DEBUG and INFO as somewhat confusing as it's not immediately clear (to me at least) where it sits on the level scale.

Having a TRACE level below DEBUG on the other hand, makes me immediately think about more information than DEBUG.

Before making a decision I'd like to leave the conversation open for a while and give the chance to other customers to share what they think about the above and whether this new level could be useful for their use cases.

@codyfrisch
Copy link
Author

codyfrisch commented Jul 8, 2023

@dreamorosi I see what you mean. A TRACE level would cover all of the statements about execution path - which is what I want to hide. Then DEBUG would cover details about all the data involved.

log4j2 and .net core both have TRACE. Python does not have a language level TRACE but does have a NOTSET level below DEBUG. Seems both Python and Typescript loggers could benefit from a TRACE level at least within the Powertools logger.

Since none have verbose, I am going to change my mind and say TRACE is the clearest choice for a path forward.

BTW thanks for the ability to set log level dynamically.

@codyfrisch codyfrisch changed the title Additional log level: VERBOSE Additional log level: TRACE Jul 8, 2023
@timo92
Copy link
Contributor

timo92 commented Jul 30, 2024

I have just started to adopt powertools and configured lambda with advanced logging controls for the first time. I expected the log levels in the logger package to match the levels mentioned by aws. This would mean that TRACE is more detailed than DEBUG and CRITICAL should be renamed or become an alias for FATAL.

@dreamorosi dreamorosi changed the title Additional log level: TRACE Feature request: add additional log level: TRACE & corresponding logger.trace() method Jul 30, 2024
@dreamorosi dreamorosi added good-first-issue Something that is suitable for those who want to start contributing help-wanted We would really appreciate some support from community for this one confirmed The scope is clear, ready for implementation and removed need-customer-feedback Requires more customers feedback before making or revisiting a decision discussing The issue needs to be discussed, elaborated, or refined labels Jul 30, 2024
@dreamorosi
Copy link
Contributor

dreamorosi commented Jul 30, 2024

Hi @timo92 - we have a section in our Logger docs dedicated to explaining how Powertools for AWS Lambda Logger interacts with AWS Lambda Advanced Logging Controls (ALC).

In short, if your function emits logs exclusively using our Logger, the only log levels that make sense from ALC as of today are DEBUG through ERROR, because we don't have corresponding methods to emit logs at FATAL and TRACE levels.

I agree that the current experience is not as good as it could be, and ideally we should support all levels supported by ALC either by adding the ones that are missing (TRACE) or aliasing existing ones (CRITICAL -> FATAL).

Seeing as this is now getting traction, perhaps because people are starting to adopt ALC, I'm going to put this item on the backlog and open it for contributions in case anyone is interested.

@timo92 - For FATAL, if possible, I would ask you to please open a separate feature request so we can address the item separately and keep changes focused/scoped.

--

For this issue, we should make the following changes:

  • Add a new log level threshold here for TRACE with value 6
  • Add a new public method to Logger called trace that has a similar implementation to this method except it uses 6 as threshold value.
  • Modify the LoggerInterface here to include the new method
  • Add the new TRACE method at the top of this LogLevel map
  • Add new test cases for the new method here to ensure that code coverage remains at 100%
  • Update the documentation here to include the new log level

Note

For those interested in contributing, please leave a comment below so that we can assign the issue to you and make sure we don't duplicate efforts. Also, if you have any further questions please don't hesitate to ask here or on our Discord channel.

@timo92
Copy link
Contributor

timo92 commented Aug 8, 2024

@dreamorosi Feel free to asign this issue to me

timo92 added a commit to timo92/powertools-lambda-typescript that referenced this issue Aug 8, 2024
timo92 added a commit to timo92/powertools-lambda-typescript that referenced this issue Aug 8, 2024
timo92 added a commit to timo92/powertools-lambda-typescript that referenced this issue Aug 8, 2024
Copy link
Contributor

⚠️ COMMENT VISIBILITY WARNING ⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

@dreamorosi dreamorosi added completed This item is complete and has been merged/shipped and removed help-wanted We would really appreciate some support from community for this one confirmed The scope is clear, ready for implementation labels Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed This item is complete and has been merged/shipped feature-request This item refers to a feature request for an existing or new utility good-first-issue Something that is suitable for those who want to start contributing logger This item relates to the Logger Utility
Projects
Development

Successfully merging a pull request may close this issue.

3 participants