feat(logger): make loglevel types stricter

[dreamorosi]

Description of your changes

The Logger constructor accepts an optional logLevel parameter with default to INFO. While the library already has types for all the log levels, and performs a validation of the value passed to logLevel, the constructor currently accepts any type of value.

As detailed in #1309, this can lead to issues that go undetected at edit/build time since TypeScript is not being leveraged to validate these inputs. For instance, currently it's possible to pass the following value and Logger will accept the value and silently default to INFO:

const logger = new Logger({ logLevel: 'DEBG' }); // instead of DEBUG

This PR, that once merged will close #1309, narrows the types for logLevel so that only certain values both upper and lower case variations are accepted. If an invalid value is passed, users will see errors both in their IDE and at compile time.

For the time being I have decided to not include a warning log in case of invalid value, given that we are already narrowing the types. However I'm open to reconsider this decision in a future PR if there's demand.

How to verify this change

See checks under this PR, as well as the results of the integration tests run.

Related issues, RFCs

Issue number: #1309

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

• My changes meet the tenets criteria
• I have performed a self-review of my own code
• I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding changes to the examples
• My changes generate no new warnings
• The code coverage hasn't decreased
• I have added tests that prove my change is effective and works
• New and existing unit tests pass locally and in Github Actions
• Any dependent changes have been merged and published
• The PR title follows the conventional commit semantics

Breaking change checklist

• I have documented the migration process
• I have added, implemented necessary warnings (if it can live side by side)

---

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

[acdha]

I noticed a regression caused by this improvement where code which used to use the standard environmental variable we use across all of our applications to configure the Logger (e.g. export const logger = new Logger({logLevel: process.env.APPNAME_LOG_LEVEL})) now fails to build because there isn't a way to guarantee that the string from the environmental variable is one of the values in the LogLevel union, and none of the exported interfaces make it easy to check that without copy-pasting the values from Log.ts.

I think it would be useful to have a public method similar to the isValidLogLevel method which would allow you to essentially say “get me the LogLevel for this string or throw an exception” so you could have a robust way to let linters know that execution will halt if that's not a valid value.

[dreamorosi]

Hi @acdha, you should be able to import the LogLevel type using import type { LogLevel } from "@aws-lambda-powertools/logger/types"; (available since v2.0.0).

With this, if you know for sure that the value of your env variable is valid, you should be able to cast the type.

If instead you don't trust/control the value and want to check at runtime, I do agree that at the moment we don't have anything exported to allow you to check easily.

I wouldn't necessarily consider this as a regression, and if anything making the type more strict (which is the content of this PR) goes in the same direction of wanting to make the input safer.

I think exporting the values and possibly adding a method/helper for customers to check this is a valid ask though. Could you please open a feature request so we can consider putting this on the backlog?

[acdha]

If instead you don't trust/control the value and want to check at runtime, I do agree that at the moment we don't have anything exported to allow you to check easily.

I actually do control the inputs completely in our Terraform configuration but the type checker can’t know that. I classed it as a regression since a point release broke our build but am happy to open a feature request tomorrow.