Skip to content

Conversation

alexeyklyukin
Copy link
Collaborator

Having a structure in the logs helps to highlight important data
(like errors or LSNs), especially when importing to the external log
storage.

Another long-standing addition is debug levels, allowing to distinguish
between debug and info messages.

This implementation wraps zap around our own logger module. The module
does not define an interface, as there seem no need for it, although it
would be trivial to add when necessary (i.e. if we consider plugging
another logger or use the same set of functions to pass structured error
messages up to the calling functions).

The changes also include new configuration options for backup and
restore to setup logging level, whether to show code locations in logs
and whether to enable development logging (output to console instead of
json, debug level by default and some other changes set in
zap.NewDevelopmentConfig()).

The changes does not convert existing logging solution to the new one.
This would be done after the changes in
https://github.com/ikitiki/logical_backup/commits/fixes are committed.

Having a structure in the logs helps to highlight important data
(like errors or LSNs), especially when importing to the external log
storage.

Another long-standing addition is debug levels, allowing to distinguish
between debug and info messages.

This implementation wraps zap around our own logger module. The module
does not define an interface, as there seem no need for it, although it
would be trivial to add when necessary (i.e. if we consider plugging
another logger or use the same set of functions to pass structured error
messages up to the calling functions).

The changes also include new configuration options for backup and
restore to setup logging level, whether to show code locations in logs
and whether to enable development logging (output to console instead of
json, debug level by default and some other changes set in
zap.NewDevelopmentConfig()).

The changes does not convert existing logging solution to the new one.
This would be done after the changes in
https://github.com/ikitiki/logical_backup/commits/fixes are committed.
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

}

func New(filename string) (*Config, error) {
func New(filename string, developmentMode bool) (*Config, error) {
Copy link

Choose a reason for hiding this comment

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

exported identifier "New" should have comment

Suggested change
func New(filename string, developmentMode bool) (*Config, error) {
// New ...
func New(filename string, developmentMode bool) (*Config, error) {

@alexeyklyukin
Copy link
Collaborator Author

@ikitiki I think you should be able to merge it; the code isolations the logger class and goes only as far as to initialize the logger in main of backup/restore and add a few configuration parameters to setup what to log.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant