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

(maint) trace docker transport run_cmd #3271

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

h0tw1r3
Copy link
Contributor

@h0tw1r3 h0tw1r3 commented Feb 18, 2024

while tracing uploading files with the docker transport, I noticed many of the docker commands being run were not trace logged.

@donoghuc
Copy link
Contributor

Are you saying that the trace logging with upload_file

@logger.trace { "Uploading #{source} to #{destination}" }
is not working? I dont think we log run_command in the other transports in the connection class, was this a parity feature or something you wanted generally for all the transports?

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Feb 29, 2024

Are you saying that the trace logging with upload_file

@logger.trace { "Uploading #{source} to #{destination}" }

is not working? I dont think we log run_command in the other transports in the connection class, was this a parity feature or something you wanted generally for all the transports?

The executed docker commands are being trace logged in all cases (that I can see) except the upload/download commands. Very helpful to see when tracing/reproducing issues and maintains consistency.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Mar 4, 2024

For reference, the reason this came up is because the underlying command docker cp does not work with tmpfs volumes. When I did a bolt run with trace logging, I could see all of the executed docker commands except these, which was confusing and lead me to understand why the upload was not showing up in the container.

I dont think we log run_command in the other transports in the connection class, was this a parity feature or something you wanted generally for all the transports?

While this PR is for parity, it would be wonderful if all transports logged exec calls.

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.

2 participants