-
-
Notifications
You must be signed in to change notification settings - Fork 777
Additional code metrics instrumentation #4310
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
Merged
Merged
Changes from 3 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
0245a18
Fix typo.
Kami 0a975aa
Add missing __all__, instrument rules engine with additional metrics.
Kami 9b5a5af
Use consistent metric name.
Kami 4d1b8f7
Add missing __all__.
Kami 20134d8
Add support for "Gauge" metric type to our metrics drivers and code.
Kami 0b4a4e5
Add new instrumentation middleware which allows us to instrument our API
Kami 5d78d67
Add new instrumentation middleware to all the API services.
Kami b483f75
Add new echo metrics driver which prints out metric calls and use it in
Kami 24f5d1a
Also track total number of the incoming requests.
Kami b16fb32
Fix lint.
Kami c759b63
Add tests for new gauge methods.
Kami 179768e
Use echo driver by default in dev environments.
Kami 8d207ea
Use consistent metric names, add some additional instrumentation.
Kami 9f8cb89
Use consistent method names.
Kami 27e85dc
Fix method arguments.
Kami 478744e
Get rid of format_metric_key() function calls which provide no value and
Kami 4086b5c
Fix typo.
Kami 387cf92
Merge branch 'master' into rules_engine_metrics_instrumentation
Kami 55b16f9
Reduce code duplication.
Kami abe6ca1
Don't decrease counter value on context manager exit.
Kami 74efba2
Increase _counter and _timer suffixes since statsd already correctly
Kami b014d7d
Merge branch 'rules_engine_metrics_instrumentation' of github.com:Sta…
Kami 8354248
Remove unused module.
Kami 82772cc
Remove unused driver for now since it's just causing confusion.
Kami e2883d4
Fix metric name.
Kami 0110721
Update affected tests.
Kami efb7856
Update changelog.
Kami ebc1245
Add sample statsd config.
Kami cacaa42
Add sample metrics configs for statsd config and carbon cache.
Kami 7d04e4e
Fix file extension.
Kami ee8996f
Add new metrics.prefix config option.
Kami eb2646d
Add changelog entry.
Kami 6732471
Remove unused code.
Kami 1170280
Add a comment.
Kami f4b3ca3
Make metric key generation more robust, include prefix after "st2" and
Kami 505106e
Update affected code and tests, add new tests.
Kami 4fb318e
Add missing module.
Kami d4c3aaf
Fix typo.
Kami fb54c2a
Re-gen sample config.
Kami 1f30852
Re-generate sample config.
Kami File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bigmstone I'm open to a better name, something which would also make it consistent with action runner metric names.
I couldn't come up with anything better :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we could also just call it
st2.trigger_instance.processed, but this could also be a bit deceiving, imo, because trigger instances can also come in through the API and can be handled by other services.At some point we might also care about total trigger instances (also the ones which are / will be processed elsewhere), but we definitely care specifically about trigger instances processed by the rules engine so the metrics key should convey that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably create a function to generate these namings in a standard way. Have a few input params so it's not so subjective, but like you I don't have many strong convictions here. I think it mainly just matters that it's consistent.