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.

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

log debug, optimization #406

Merged
merged 1 commit into from
Mar 25, 2021
Merged

log debug, optimization #406

merged 1 commit into from
Mar 25, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Mar 23, 2021

linked to #331

  • resolved config and satrtegy is logged by DEBUG
  • merge Split brain TXT time stamp into one log.Info() message, refactor code

... feel free to come with places where log.Debug() is desirable

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

Couple of suggestions

controllers/providers/assistant/gslb.go Outdated Show resolved Hide resolved
controllers/providers/assistant/gslb.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

@kuritka, looks good, just few suggestions and general questions about log improvements.

@kuritka kuritka force-pushed the logger-DBG branch 4 times, most recently from 7a7a30a to 930a93a Compare March 24, 2021 14:13
@kuritka
Copy link
Collaborator Author

kuritka commented Mar 24, 2021

Amended.

I also added to factory

	logger.Debug().
		Str("Format", l.log.Format.String()).
		Str("Level", l.log.Level.String()).
		Msg("Logger settings")

The reason I use Level instead of level is because level keyword is already used by zerolog context and is not printed.

@somaritane somaritane linked an issue Mar 24, 2021 that may be closed by this pull request
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

@kuritka , left some extra comments for affected log commands

controllers/providers/assistant/gslb.go Outdated Show resolved Hide resolved
controllers/gslb_controller.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
related to #331
- resolved config and satrtegy is logged by DEBUG
- merge Split brain TXT time stamp into one log.Info() message, refactor code

... feel free to come with places where log.Debug() is desirable

Co-authored-by: Timofey Ilinykh <timofey.ilinykh@absa.africa>
@kuritka
Copy link
Collaborator Author

kuritka commented Mar 25, 2021

@somaritane, thx for suggestions, amended

@kuritka kuritka requested a review from somaritane March 25, 2021 07:23
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

@kuritka, thanks for updates, lgtm!
P.S. There's one comment I left about gopkg ToString() JSON formatting, but I don't think it's in the scope of this PR.

@kuritka kuritka merged commit 172323f into master Mar 25, 2021
@kuritka kuritka deleted the logger-DBG branch March 25, 2021 15:19
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.

Logger Enhancements
3 participants