-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add log level config #87
Conversation
Having some build error on the CI: https://github.com/well-known-components/logger/runs/7034017036?check_suite_focus=true Solved locally by updating the Update: solved here 8cb8acf |
src/helpers.ts
Outdated
const orderedLevels = ["ALL", "LOG", "DEBUG", "INFO", "WARN", "ERROR", "OFF"] | ||
|
||
const minLogLevelIndex = orderedLevels.findIndex((level) => level === minLogLevel) | ||
const functionLogLevelIndex = orderedLevels.findIndex((level) => level === functionLogLevel) | ||
|
||
// Print every log greater than or equal to a certain level | ||
return functionLogLevelIndex >= minLogLevelIndex | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a critical code path and using the findIndex may not be the optimal choice, could we store those const
as numbers in an object or enum and prevent the findIndex?
index = orderedLevels.findIndex((level) => level === functionLogLevel)
// is the same as
const enum = { "OFF": 0, "LOG": 1, "DEBUG": 2, "INFO": 4, "WARN": 8, "ERROR": 16, "ALL": 1 | 2 | 4 | 8 | 16 }
numericLevel = enum[key]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Done. I had to interchange the values of OFF
and ALL
const levelsEnum = { ALL: 0, LOG: 1, DEBUG: 2, INFO: 4, WARN: 8, ERROR: 16, OFF: 1 | 2 | 4 | 8 | 16 }
Add log level in a similar style to log4js
"ALL" > "LOG" > "DEBUG" > "INFO" > "WARN" > "ERROR" > "OFF"
I'm passing a config object with the
logLevel
instead of the env-config-component because it would require changing the external API to use Promises, leading to breaking changes. In the future, we could pass either this config | WKC config