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 some metric instrumentation to the notifer service code #4536

Merged
merged 6 commits into from
Feb 18, 2019

Conversation

Kami
Copy link
Member

@Kami Kami commented Feb 5, 2019

This pull request adds some metrics instrumentation to the st2notifier service code.

We don't have any now so it would be good to have some. Especially because we noticed some "interesting" CPU spikes in the notifier service on our CICD server (could be related to blocking Jinja template rendering which maybe we can offload to a thread pool to avoid blocking main thread and reduce the main process CPU usage).

TODO

  • Document those new metrics in the docs

@Kami Kami added the metrics label Feb 5, 2019
@Kami Kami added this to the 2.10.2 milestone Feb 5, 2019
@Kami
Copy link
Member Author

Kami commented Feb 6, 2019

Docs at StackStorm/st2docs#846.

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

👍

Just wondering if we need that amount of granular metrics.
For example, st2.notifier.transform_message and st2.notifier.transform_data. How useful that might be for average user to track?

@Kami
Copy link
Member Author

Kami commented Feb 11, 2019

@armab Yeah that one is a bit of an exception, we usually don't need such granular metrics, but this one is a bit special.

I hope we can remove it once we get a better picture (it shouldn't cause any overhead and it's not documented as a public exposed metric either).

@Kami Kami merged commit 97e0241 into master Feb 18, 2019
@Kami Kami deleted the notifier_service_instrumentation branch February 18, 2019 09:01
Kami added a commit that referenced this pull request Mar 13, 2019
to only be dispatched on completed states, but not on custom states if
action_sensor.emit_when config option was configured.

Issue was inadverently introduced in
#4536,

Issue reported by Hiroyasu OHYAMA (@userlocalhost).
Kami added a commit that referenced this pull request Mar 13, 2019
to only be dispatched on completed states, but not on custom states if
action_sensor.emit_when config option was configured.

Issue was inadverently introduced in
#4536,

Issue reported by Hiroyasu OHYAMA (@userlocalhost).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants