feat: add configurable logLevel to install options#283
Conversation
12ff7a7 to
232a734
Compare
erickzhao
left a comment
There was a problem hiding this comment.
API-wise, I think a logLevel option would make more sense and cover all our bases.
Options would be debug, info, warn, error, or none, where enabling a less important log level would make all higher-priority ones.
It feels more ergonomic that way since you don't usually want to ignore non-contiguous log levels.
|
|
||
| ```js | ||
| await devtron.install({ ignoreLogs: ['debug', 'info'] }); | ||
| ``` |
There was a problem hiding this comment.
Agreed with @erickzhao about a logLevel option. Then these two examples could be
devtron.install({ logLevel: [] })
devtron.install({ logLevel: ['info', 'warn'] })
I'm partial to the idea of the quiet option as a shorthand though. Open to either keeping it or removing it.
There was a problem hiding this comment.
In my head it'd look more like a sliding scale:
devtron.install({ logLevel: 'quiet' }) // or `'none'`
devtron.install({ logLevel: 'error' }) // only shows `error`
devtron.install({ logLevel: 'warn' }) // shows `warn`, or `error`, but not `debug` or `info`
devtron.install({ logLevel: 'info' }) // shows `info`, `warn`, or `error`, but not `debug`since it'd be unlikely that you would want to specifically ignore non-contiguous log levels (e.g. ignoring error and debug but not info and warn).
|
@erickzhao I made the requested changes and also updated the NPM Package to |
quiet mode and ignoreLogs supportlogLevel to install options
src/utils/Logger.ts
Outdated
| private readonly logLevelMap: Record<LogLevel, number> = { | ||
| debug: 1, | ||
| info: 2, | ||
| warn: 3, | ||
| error: 4, | ||
| none: 5, | ||
| }; |
There was a problem hiding this comment.
I think we could combine this and the LogLevel type in src/types/shared.ts if we just make LogLevel into a numeric enum:
enum LogLevel {
none,
debug,
info,
warn,
error
}
Enums without assigned values in TS auto-assign to incrementing number values so you can get > comparisons out of the box without defining a separate map.
See:
- TypeScript docs: https://www.typescriptlang.org/docs/handbook/enums.html#numeric-enums
- Playground: https://www.typescriptlang.org/play/?#code/KYOwrgtgBAMg9gcxsAbsANlA3gKCvqAE2ACMwEAaPAgSxADM4qCoB3AQwCcQ7Lr9gnTnE44AvjhwBjOCADOAFyj0wIKAF4oACmAAuWImRp0ASg0A+bPygz5cdMAB06RDqiX4SVBkfEyCEwBucUkcFRAtTyMfOkYTMNVIw290X1JyEyA
There was a problem hiding this comment.
Since we want users to be able to pass log level as a string, like devtron.install({ logLevel: 'warn' }), I'll need to create another type with log level strings as well, right ?
|
I've added the |
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description of Change
Loggerclass withlogLeveloption.devtron.install(options)to configure the logger automatically, allowing users to set a minimum log level. Only logs at the selected level and above will be shown, preventing unwanted Devtron logs from cluttering the terminal.optionstable.Usage: