Skip to content

use bklog.G(ctx) instead of logrus directly#3705

Merged
coryb merged 1 commit intomoby:masterfrom
coryb:bklog
Mar 15, 2023
Merged

use bklog.G(ctx) instead of logrus directly#3705
coryb merged 1 commit intomoby:masterfrom
coryb:bklog

Conversation

@coryb
Copy link
Copy Markdown
Collaborator

@coryb coryb commented Mar 12, 2023

This is a continuation of #2236

I noticed there were several lines in our logs that were missing traceID fields and found they were still using logrus directly.

I have modified a few apis to pass through the context, and in some cases (where a request context was not applicable) I used bklog.L although I could change that to use bklog.G(context.Background()) if that is preferable.

@coryb coryb force-pushed the bklog branch 3 times, most recently from 589dba3 to 4478171 Compare March 12, 2023 19:30
@coryb coryb requested a review from tonistiigi March 12, 2023 21:01
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Sorry, looks like this needs a rebase.

cc @morlay

As a follow-up would be good to get a linter rule that checks this (unless there are still valid exceptions).

@jedevc
Copy link
Copy Markdown
Member

jedevc commented Mar 15, 2023

For a linting rule, I think something like jedevc@6377bbb should do the job?

Signed-off-by: coryb <cbennett@netflix.com>
@coryb
Copy link
Copy Markdown
Collaborator Author

coryb commented Mar 15, 2023

For a linting rule, I think something like jedevc@6377bbb should do the job?

Thanks, I put in something similar, the line anchors ^ and $ I think were preventing matches in my testing, I also added a few more possible matches for logrus to restrict.

@coryb coryb merged commit 13ea5ae into moby:master Mar 15, 2023
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.

3 participants