WIP do not merge: Dump HTTP operations

[mtrmac]

Quite often it has been useful to dump the full HTTP request and response headers, and sometimes fuill bodies, for debugging; we probably should make that possible either automatically at the “debug” log level, or when the user sets an option.

This is by no means a workable implementation for that, but I’ve written exactly this hack two or three times now, so right now I am at least recording it for posterity, expecting that rebasing it will be easier than rewriting it.

A real implementation should, I guess, wrap a http.RoundTripper or perhaps http.Client (or just http.Client.Do?), or perhaps using net/http/httptrace somehow.

[runcom]

Isn't this adding overhead to the command? I'm fine with this, just want to make sure we don't wait that much dumping req/res.

[rhatdan]

Any way to pass down a debug flag? Or just rebuild with a DEBUG Tag?

[mtrmac]

Isn't this adding overhead to the command?

Yes, and it completely clutters the debugging output. As I said

This is by no means a workable implementation

[mtrmac]

Any way to pass down a debug flag? Or just rebuild with a DEBUG Tag?

We have a types.SystemContext which can be used to pass down such flags, yes.

[rhatdan]

@mtrmac Could you open a trello card with an intern flag, so we could have an intern work on this next summer.

[mtrmac]

@mtrmac Could you open a trello card with an intern flag, so we could have an intern work on this next summer.

https://trello.com/c/xrAwKleu/542-containers-image-http-dumps-intern

[rhatdan]

@mtrmac @vrothberg What is the state of this PR? Should we rebase and move forward with it or should we close it?

[mtrmac]

Rebased, but this still needs turning into a types.SystemContext option + CLI options in users of the library.

[vrothberg]

Meeting notes: use logrus.Trace*

[mtrmac]

logrus has a Trace log level, probably a good way to opt in now.

[mtrmac]

Updated:

• To centralize the dumping implementation (still just a helper method, not a http.RoundTripper)
• In that dumping implementation, to log errors as well
• To use logrus.Tracef, if enabled.

WARNING: Using this will cause trace-level logs to include all credentials (Basic auth, bearer tokens, etc.). Is that OK? Applications including this should probably warn about that at least in documentation.

[vrothberg]

LGTM

Thanks for the warning. Let's add that to the release notes.

[rhatdan]

Unless we can easily hide this information, then yes we should document this in the release notes. As well as in the man pages if possible. Perhaps containers-transports.5.md Under the "docker" section?

[vrothberg]

Unless we can easily hide this information, then yes we should document this in the release notes. As well as in the man pages if possible. Perhaps containers-transports.5.md Under the "docker" section?

And in the man pages of the tools (i.e., the --debug and --log-level flags).

[rhatdan]

Just needs to have an update to the transports man page.

[mtrmac]

In addition to the previously-noted concerns about opt-in, this should almost certainly be reworked to become a http.RoundTripper, wrapper, so that all redirects are logged.

[kwilczynski]

This would be very useful for troubleshooting issues with access to remote registries. At the moment, I tend to opt for a set up with a transparent TLS proxy to snoop on this type of traffic, but having a log that exposes this type of data that can be easily enabled would be ideal.

[mtrmac]

For the record, Red Hat internal tracking: https://issues.redhat.com/browse/RHEL-36783

[jankaluza]

Hi, and thank you for your contribution!

We’ve recently migrated this repository into a new monorepo: containers/container-libs along with other repositories

As part of this migration, this repository is no longer accepting new Pull-Requests and therefore this Pull-Request is being closed.

Thank you very much for your contribution. We would appreciate your continued help in migrating this PR to the new container-libs repository. Please let us know if you are facing any issues.

You can read more about the migration and the reasoning behind it in our blog post: Upcoming migration of three containers repositories to monorepo.

Thanks again for your work and for supporting the containers ecosystem!