-
Notifications
You must be signed in to change notification settings - Fork 43
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
logp: add an in-memory logger #191
Conversation
// logs. | ||
// encCfg configures the log format, use logp.ConsoleEncoderConfig for console | ||
// format, logp.JSONEncoderConfig for JSON or any other valid zapcore.EncoderConfig. | ||
func NewInMemory(selector string, encCfg zapcore.EncoderConfig) (*Logger, *bytes.Buffer) { |
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.
question on returning the pointer to the bytes.Buffer. It looks like zap uses it as a WriteSyncer, which does have a Lock
function to make it safe for concurrent use. If someone has the pointer to the underlying Buffer they could get around that. Also is it a problem if someone does something like Reset
on the Buffer?
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.
It looks like zap uses it as a WriteSyncer, which does have a Lock function to make it safe for concurrent use.
Almost, it's a bit confusing, it has a writerWrapper
which just adds a no-op Sync()
to an io.Writer
and also the Lock
function, this adds a lock to a WriteSyncer
, the interface, making it goroutinesafe.
I added the lock so it's safe to be used anywhere and an improved the docs
💚 Build Succeeded
History
cc @AndersonQ |
What does this PR do?
It adds an in memory logger to the
logp
package. It's migrating elastic/elastic-agent#4485 to hereWhy is it important?
An in-memory logger is useful on other places besides the install/uninstall code of the elastic-agent
Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files[ ] I have added tests that prove my fix is effective or that my feature works[ ] I have added an entry in./changelog/fragments
using the changelog tool[ ] I have added an integration test or an E2E testHow to test this PR locally
Related issues
Logs
Comparing the in-memory logger with the zap observer during the Elastic Agent installation process:
Questions to ask yourself