Skip to content

remotes/docker: log registry URLs as info instead of debug#5681

Merged
fuweid merged 1 commit intocontainerd:mainfrom
kzys:info-url
Apr 20, 2022
Merged

remotes/docker: log registry URLs as info instead of debug#5681
fuweid merged 1 commit intocontainerd:mainfrom
kzys:info-url

Conversation

@kzys
Copy link
Copy Markdown
Member

@kzys kzys commented Jul 2, 2021

Registry URLs are important and should be logged even the logging level is info.

Fixes #5486.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Jul 2, 2021

Build succeeded.

Comment thread remotes/docker/fetcher.go
continue
}
log.G(ctx).Debug("trying alternative url")
log.G(ctx).Info("request")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe add more information here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure. It may result in leaking some secret stuff.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should at least be clear the request is from dest.URLs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it should contain the URL....there is not too much information in ctx...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Because of log.WithLogger(ctx, log.G(ctx).WithField("url", us)), log.G returns a logger with url=us. Let me double-check.

Registry URLs are important and should be logged even the logging level is
info.

Fixes containerd#5486.

Signed-off-by: Kazuyoshi Kato <[email protected]>
@kzys kzys changed the title remotes/docker: log URLs as info remotes/docker: log registry URLs as info instead of debug Dec 3, 2021
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Dec 3, 2021

Build succeeded.

@estesp estesp closed this Mar 15, 2022
@estesp estesp reopened this Mar 15, 2022
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Mar 15, 2022

Build succeeded.

@kzys
Copy link
Copy Markdown
Member Author

kzys commented Apr 18, 2022

@AkihiroSuda @fuweid Can you take a look?

@fuweid fuweid merged commit 9b33526 into containerd:main Apr 20, 2022
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.

Change URL fetcher logging from Debug to Info

5 participants