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

feat(subp): Measure subprocess command time #4606

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Nov 14, 2023

feat(subp): Measure subprocess command time

@smoser's idea

@holmanb holmanb requested a review from smoser November 14, 2023 16:54
@holmanb
Copy link
Member Author

holmanb commented Nov 14, 2023

Didn't implement this in the same log that subp currently emits since it is useful to see that log before the command is run, for debugging in case cloud-init crashes or hangs.

@holmanb holmanb mentioned this pull request Nov 14, 2023
3 tasks
Copy link

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

@github-actions github-actions bot added the stale-pr Pull request is stale; will be auto-closed soon label Nov 29, 2023
@holmanb holmanb removed the stale-pr Pull request is stale; will be auto-closed soon label Nov 29, 2023
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

This results in a ton of logs like this:

2023-12-01 03:32:20,754 - subp.py[DEBUG]: command ['hostname', 'me'] took 0.0s to run

and also adds a fair amount of extra logs.

I think we should either change %.3s to %.5s, or only log if the time elapsed is >= .1

@TheRealFalcon TheRealFalcon self-assigned this Dec 1, 2023
@holmanb holmanb force-pushed the holmanb/subp-duration branch from a3f7084 to b087341 Compare December 5, 2023 00:32
@holmanb
Copy link
Member Author

holmanb commented Dec 5, 2023

This results in a ton of logs like this:

Hmm, that serves no one.

only log if the time elapsed is >= .1

I prefer this way to avoid the extra logs.

@holmanb holmanb requested a review from TheRealFalcon December 5, 2023 00:34
@holmanb holmanb force-pushed the holmanb/subp-duration branch from b087341 to 7d4e658 Compare December 5, 2023 14:53
@holmanb
Copy link
Member Author

holmanb commented Dec 5, 2023

Rebased to resolve conflict with main.

@holmanb holmanb merged commit 72d6e18 into canonical:main Dec 6, 2023
26 checks passed
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