-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Implement Logger.isLevelEnabled
#144033
Implement Logger.isLevelEnabled
#144033
Conversation
Pinging @elastic/kibana-core (Team:Core) |
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.
LGTM
for (const logLevel of orderedLogLevels) { | ||
it(`returns the correct value for a '${logLevel.id}' level logger`, () => { | ||
const levelLogger = new BaseLogger(context, logLevel, appenderMocks, factory); | ||
for (const level of orderedLogLevels) { | ||
const levelEnabled = logLevel.supports(level); | ||
expect(levelLogger.isLevelEnabled(level.id)).toEqual(levelEnabled); | ||
} | ||
}); | ||
} |
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.
nit:
for (const logLevel of orderedLogLevels) { | |
it(`returns the correct value for a '${logLevel.id}' level logger`, () => { | |
const levelLogger = new BaseLogger(context, logLevel, appenderMocks, factory); | |
for (const level of orderedLogLevels) { | |
const levelEnabled = logLevel.supports(level); | |
expect(levelLogger.isLevelEnabled(level.id)).toEqual(levelEnabled); | |
} | |
}); | |
} | |
it.each(orderedLogLevels)(`returns the correct value for a '$id' level logger`, (logLevel) => { | |
const levelLogger = new BaseLogger(context, logLevel, appenderMocks, factory); | |
for (const level of orderedLogLevels) { | |
const levelEnabled = logLevel.supports(level); | |
expect(levelLogger.isLevelEnabled(level.id)).toEqual(levelEnabled); | |
} | |
}); |
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.
LGTM this is exactly what we need in Fleet.
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.
Response Ops changes LGTM.
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.
LGTM
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
Summary
Fix #144002
Implement
Logger.isLevelEnabled
(similar to log4j'sisEnabled
/isInfoEnabled
and so on...), to allow to wrap log writing inside conditional blocks.