-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Allow loggers to create child loggers via 'get' method #52605
Allow loggers to create child loggers via 'get' method #52605
Conversation
get
method to Logger
Pinging @elastic/kibana-platform (Team:Platform) |
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.
Finally! Spaces 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.
💯
warn: jest.fn(), | ||
get: jest.fn(), | ||
}; | ||
mockLog.get.mockImplementation((...ctx) => ({ |
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.
super nit: use context
for consistency with logging service mock
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.
context is already declared in the upper scope (no-shadow)
😢
@elasticmachine merge upstream |
bbfc591
to
3f2c557
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* add 'Logger.get' method * updates generated doc * resolve merge conflicts
* add 'Logger.get' method * updates generated doc * resolve merge conflicts
Summary
Fix #39695
Avoid having to propagate both
Logger
andLoggerFactory
in nested parts of the code by adding aget
method onLogger
that will creates a child logger from it's context.PR also adds a
createLogger
method to the logging service mock, and uses it in some plugin tests that were manually declaring a mockedLogger
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately