Skip to content
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.

Sign up for GitHub

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

Add Core function to logp.Logger to allow access to backing zapcore.Core. #88

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Add Core function to logp.Logger to allow access to backing zapcore.Core. #88

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions logp/logger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ func (l *Logger) Sync() error {
return l.logger.Sync()
}

// Core returns the backend zapcore.Core for the logger.
func (l *Logger) Core() zapcore.Core {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better to define a type alias like it's done with the zap.Option:

elastic-agent-libs/logp/logger.go

Lines 27 to 28 in 89215ad

// LogOption configures a Logger.
type LogOption = zap.Option

I don't know how far we want to go exposing the zap types in our library. I see that the Logger struct just has zap types directly:

elastic-agent-libs/logp/logger.go

Lines 30 to 34 in 89215ad

// Logger logs messages to the configured output.
type Logger struct {
logger *zap.Logger
sugar *zap.SugaredLogger
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see a need to hide it. It is zap and zapcore under the hood and the go.mod for any project that imports elastic-agent-libs will show zap and zapcore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would rather an abstraction should we decide to switch from zap to something else. I guess that's the point. But I'm okay with this.

return l.logger.Core()
}

// L returns an unnamed global logger.
func L() *Logger {
return loadLogger().logger
Expand Down