Skip to content

Feature request: export LogLevel values as object #2780

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
adityamatt opened this issue Jul 16, 2024 · 8 comments · Fixed by #2787
Closed
1 of 2 tasks

Feature request: export LogLevel values as object #2780

adityamatt opened this issue Jul 16, 2024 · 8 comments · Fixed by #2787
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

@adityamatt
Copy link

Summary

LogLevel for the lambda is currently a type

type LogLevelDebug = 'DEBUG';
type LogLevelInfo = 'INFO';
type LogLevelWarn = 'WARN';
type LogLevelError = 'ERROR';
type LogLevelSilent = 'SILENT';
type LogLevelCritical = 'CRITICAL';

type LogLevel = LogLevelDebug | Lowercase<LogLevelDebug> | LogLevelInfo | Lowercase<LogLevelInfo> | LogLevelWarn | Lowercase<LogLevelWarn> | LogLevelError | Lowercase<LogLevelError> | LogLevelSilent | Lowercase<LogLevelSilent> | LogLevelCritical | Lowercase<LogLevelCritical>;

Can we turn this into an enum and export it ?

Why is this needed?

We can import this enum and use it

Which area does this relate to?

Logger

Solution

enum LogLevel {
    DEBUG = "DEBUG",
    INFO = "INFO",
    ERROR = "ERROR",
    SILENT = "SILENT",
    CRITICAL= "CRITICAL"
}


type LogLevelType = LogLevel | Lowercase<LogLevel>

Acknowledgment

Future readers

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

@adityamatt adityamatt added internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) triage This item has not been triaged by a maintainer, please wait labels Jul 16, 2024
Copy link

boring-cyborg bot commented Jul 16, 2024

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

@am29d
Copy link
Contributor

am29d commented Jul 16, 2024

Hey @adityamatt , thanks for raising the issue. What is the problem you are trying to solve? Can you share a bit more of context and info why it has to be enum and why it does not work with the current implementation?

@am29d am29d added need-more-information Requires more information before making any calls logger This item relates to the Logger Utility and removed triage This item has not been triaged by a maintainer, please wait internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) labels Jul 16, 2024
@adityamatt
Copy link
Author

Hi @am29d ,
There is no blocking problem, for now , we need to define a common variable in our codebase and use it as default log level, We add the type check by importing the types from the library. but it could be improved if we could import the enum itself rather than the type and then create the value ourself, it seems too verbose.

Our current use:

const DEFAULT_LOG_LEVEL: LogLevel = 'ERROR'


const logger = new Logger({
  logLevel : DEFAULT_LOG_LEVEL
});

I propose rather than importing a type and defining a variable, if we can import the enum itself, it would simplify things

const logger = new Logger({
  logLevel :  LogLevel.ERROR
});

@dreamorosi
Copy link
Contributor

dreamorosi commented Jul 19, 2024

Thank you for providing the additional info, I think adding the log levels in a similar way to the one you describe is valid.

As a project, we have been trying to move away from enums and instead use objects with the as const notation (see here as for why).

For this specific feature, I would like to see the following:

  • Add a LogLevel object in packages/logger/src/constants.ts like this:
const LogLevel = {
  DEBUG: 'DEBUG',
  INFO: 'INFO',
  WARN: 'WARN',
  ERROR: 'ERROR',
  SILENT: 'SILENT',
  CRITICAL: 'CRITICAL',
} as const;
  • Import that constant in packages/logger/src/types/Log.ts and replace the existing types with this:
+ import { LogLevel } from '../constants.js';

- type LogLevelDebug = 'DEBUG';
- type LogLevelInfo = 'INFO';
- type LogLevelWarn = 'WARN';
- type LogLevelError = 'ERROR';
- type LogLevelSilent = 'SILENT';
- type LogLevelCritical = 'CRITICAL';

type LogLevel =
-   | LogLevelDebug
-   | Lowercase<LogLevelDebug>
-   | LogLevelInfo
-   | Lowercase<LogLevelInfo>
-   | LogLevelWarn
-   | Lowercase<LogLevelWarn>
-   | LogLevelError
-   | Lowercase<LogLevelError>
-   | LogLevelSilent
-   | Lowercase<LogLevelSilent>
-   | LogLevelCritical
-   | Lowercase<LogLevelCritical>;
+   | (typeof LogLevel)[keyof typeof LogLevel]
+   | Lowercase<(typeof LogLevel)[keyof typeof LogLevel]>;
  • Export the newly added LogLevel constant in the packages/logger/src/index.ts file
  • Modify this test case here to use the new LogLevel.DEBUG value (this will require importing it here and modifying import { LogLevelThresholds, LogLevel } from '../../src/types/Log.js'; to something like LogLevel as LogLevelType so that they don't clash.

I'm adding the help-wanted and good-first-issue labels to the issue, if anyone is interested in contributing with a PR, please leave a comment here so I can assign the issue to you.

@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 feature-request This item refers to a feature request for an existing or new utility confirmed The scope is clear, ready for implementation and removed need-more-information Requires more information before making any calls labels Jul 19, 2024
@dreamorosi dreamorosi moved this from Triage to Backlog in Powertools for AWS Lambda (TypeScript) Jul 19, 2024
@dreamorosi dreamorosi changed the title Maintenance: Export LogLevel values as enum rather than Type Feature request: export LogLevel values as object Jul 19, 2024
@marlapativ
Copy link
Contributor

@dreamorosi, Can I work on this issue?

Also, from the code I've gone through in the tests, the log levels are set via string as of now. Something like this:

const logger = new Logger({
  logLevel: 'ERROR',
});

Would it make sense to update all of them to use newly defined LogLevel constant ?
I mean something like this:

const logger = new Logger({
  logLevel: LogLevel.ERROR,
});

Or should I leave it be?

@dreamorosi
Copy link
Contributor

Hi @marlapativ, absolutely you can!

Regarding the tests, I'd like to leave some of them with string so that we can cover that case as well.

Not all users will adopt this new method, but I don't see any issue in converting some to use it.

I'll assign the issue to you, thank you for picking it up!

@marlapativ
Copy link
Contributor

@dreamorosi,

I've raised a PR with the changes. I've also randomly updated some tests to use the new constant.

#2787

Please merge if it looks good.

Thanks.

@dreamorosi dreamorosi linked a pull request Jul 20, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from Working on it to Coming soon in Powertools for AWS Lambda (TypeScript) Jul 20, 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 removed the help-wanted We would really appreciate some support from community for this one label Sep 16, 2024
@dreamorosi dreamorosi added completed This item is complete and has been merged/shipped and removed confirmed The scope is clear, ready for implementation labels Sep 16, 2024
@dreamorosi dreamorosi moved this from Coming soon to Shipped in Powertools for AWS Lambda (TypeScript) 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.

4 participants