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

[mysql] pipe logger into mysql for debug printing (#1055) #2217

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

barakmich
Copy link
Contributor

Renames some variables for clarity, remove unused logging functions and add MySQL debug printing support.

@barakmich barakmich requested review from vroldanbet and a team as code owners January 29, 2025 19:31
@github-actions github-actions bot added the area/datastore Affects the storage system label Jan 29, 2025
Copy link

github-actions bot commented Jan 29, 2025

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@barakmich
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@barakmich
Copy link
Contributor Author

recheck

authzedbot added a commit to authzed/cla that referenced this pull request Jan 29, 2025
tstirrat15
tstirrat15 previously approved these changes Jan 29, 2025
Copy link
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

LGTM, had a couple of questions

Comment on lines -42 to -49

func Print(v ...interface{}) {
Logger.Debug().CallerSkipFrame(1).Msg(fmt.Sprint(v...))
}

func Printf(format string, v ...interface{}) {
Logger.Debug().CallerSkipFrame(1).Msgf(format, v...)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I take it these are otherwise unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep!

Comment on lines 676 to 680
type debugLogger struct{}

func (debugLogger) Print(v ...any) {
log.Logger.Debug().CallerSkipFrame(1).Msg(fmt.Sprint(v...))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this specific to MySQL? Is it worth extending to the other datastores as well? It's also okay if that's outside of the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

every datastore seems to have its own interface -- this particular one is specific to mysql, alas....

@barakmich barakmich added this pull request to the merge queue Jan 29, 2025
Merged via the queue into main with commit 542053f Jan 29, 2025
39 checks passed
@barakmich barakmich deleted the mysql_logging branch January 29, 2025 20:14
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/datastore Affects the storage system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants