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

Add "--enable-profiler" flag to all the StackStorm services + CLI #5199

Merged
merged 9 commits into from
Oct 5, 2021

Conversation

Kami
Copy link
Member

@Kami Kami commented Mar 21, 2021

This pull request adds --enable-profiler flag to all the StackStorm services + CLI which can be used to profiler the code using cProf based profiler during development and troubleshooting.

Keep in mind that cProf offers only basic information and usually when drilling down (into a function) you need to use line profiler, but this may not working as expected our of the box due to our usage of eventlet (need to add some docs on that).

@Kami Kami added the profiling label Mar 21, 2021
@Kami Kami added this to the 3.5.0 milestone Mar 21, 2021
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Mar 21, 2021
@@ -65,7 +65,7 @@
from st2client.utils.logging import LogLevelFilter, set_log_level_for_all_loggers
from st2client.commands.auth import TokenCreateCommand
from st2client.commands.auth import LoginCommand

from st2client.utils.profiler import setup_regular_profiler
Copy link
Member Author

Choose a reason for hiding this comment

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

NOTE: st2client can't depend on st2common that's why we intentionally need to duplicate the code in this package.

This flag can be used to enable cProf based profiler for debugging and
troubleshooting purposes.
@Kami Kami force-pushed the eventlet_profiler branch from e0270f2 to aa0d3b6 Compare March 21, 2021 18:58
@Kami Kami added the debugging label Apr 17, 2021
@blag
Copy link
Contributor

blag commented Apr 22, 2021

Can we use a different profiler? Something like Scalene?

@Kami
Copy link
Member Author

Kami commented May 2, 2021

@blag We probably could for code which doesn't rely on eventlet.

For most rest of the code which relies on eventlet we probably can't use it (unless that profiler knows how to handle eventlet code and context switching).

@amanda11 amanda11 modified the milestones: 3.5.0, 3.6.0 Jun 29, 2021
@CLAassistant
Copy link

CLAassistant commented Sep 6, 2021

CLA assistant check
All committers have signed the CLA.

@cognifloyd cognifloyd enabled auto-merge October 2, 2021 05:26
@cognifloyd
Copy link
Member

@Kami looks like all you have to do is re-sign the CLA (which we changed recently to say Linux Foundation instead of one of the previous StackStorm stewards).

@cognifloyd cognifloyd added the status:needs cla A manual tag to note PRs that are otherwise ready, but the CLA bot says the CLA has not been signed. label Oct 3, 2021
@cognifloyd cognifloyd disabled auto-merge October 5, 2021 17:10
@arm4b arm4b enabled auto-merge October 5, 2021 18:40
@arm4b arm4b disabled auto-merge October 5, 2021 18:40
@arm4b arm4b merged commit 8fdb7dc into master Oct 5, 2021
@arm4b arm4b deleted the eventlet_profiler branch October 5, 2021 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/L PR that changes 100-499 lines. Requires some effort to review. status:needs cla A manual tag to note PRs that are otherwise ready, but the CLA bot says the CLA has not been signed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants