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

added logs #294

Merged
merged 2 commits into from
May 31, 2021
Merged

added logs #294

merged 2 commits into from
May 31, 2021

Conversation

shashwatj07
Copy link
Contributor

@shashwatj07 shashwatj07 commented May 25, 2021

resolves Part of #267

@shashwatj07 shashwatj07 changed the title added request invoking snapshots added logs May 26, 2021
@ekexium
Copy link
Collaborator

ekexium commented May 26, 2021

Thanks! I think in most cases we don't want to see plenty of such logs. So they'd better be in debug level.

And I think there still be other work to improve logging. So we don't close #267 for now.

@shashwatj07
Copy link
Contributor Author

Thanks! I think in most cases we don't want to see plenty of such logs. So they'd better be in debug level.

I've changed them accordingly.

And I think there still be other work to improve logging. So we don't close #267 for now.

Yes, I know. I'll add some more in this PR. Please suggest what all to add. Thanks for the help.

Signed-off-by: Shashwat Jaiswal <shashwatjaiswal2001@gmail.com>
@shashwatj07 shashwatj07 marked this pull request as ready for review May 27, 2021 04:31
@andylokandy
Copy link
Collaborator

Maybe we need to use slog so that we can connect the logs between user invocation and internal requests.

Copy link
Collaborator

@ekexium ekexium left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

In terms of other logging, I think we can have more info! for non-trivial events. Maybe some warn! for unexpected but recoverable errors.

@shashwatj07 shashwatj07 mentioned this pull request May 30, 2021
@ekexium ekexium merged commit b7b8b8b into tikv:master May 31, 2021
@shashwatj07 shashwatj07 deleted the logs branch May 31, 2021 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants