-
Notifications
You must be signed in to change notification settings - Fork 107
Persistent Tools support for Tool Meister #1824
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
Conversation
|
We have 39 commits in this PR, will rebase down to 2. |
7d660e8 to
aa592c0
Compare
|
@Maxusmusti, this commit has been rebased and squashed so that all of your work is in the first commit, including the persistent tools work, and @keshavm02's work is in the second commit. I believe this is ready to be merged into |
|
Looks like everything is in there. I will run this branch through my personal testing gauntlet in the morning one last time to make sure it's all still good, then I'll stamp with a seal of approval. |
npalaska
left a comment
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.
Looks fine to me
Maxusmusti
left a comment
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.
All testing checked out (aside from the unrelated sysinfo-dump issues)
ndokos
left a comment
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.
Partial review: still need to do the big three at the end.
f314708
aa592c0 to
f314708
Compare
f314708 to
4da712d
Compare
|
I'll make a new PR tomorrow morning post-scrum for just addressing these code review changes, then get those thrown in here so we can get this merged eod tomorrow (or Friday if people would like more time to add additional review comments, whatever works) |
|
See also #1835. |
|
Merge commits from PR #1835. |
This work adds the notion of a "collector" to the Tool Data Sink, and
"persistent tools" which run continuously without cycling through the
"start", "stop", and "send" phases. The collector is responsible for
continuously pulling data from those tools which are now started during
the new "init" phase, and stopped during the new "end" phase.
The first actual implementation of this kind of collector is for the
Prometheus data collection environment, where a `node-exporter`
"persistent tool" is run providing an end-point for a prometheus server
"collector" to pull data from it and store it locally off the run
directory (`${benchmark_run_dir}`,
e.g. `${benchmark_run_dir}/collector/prometheus`).
This work adds the `tool-scripts/meta.json` file which is used to
describe which tools are persistent and which are transient (default).
Co-authored-by: maxusmusti <meyceoz@redhat.com>
zulcss
left a comment
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.
see comments
|
|
||
| self.logger.debug("PROM TERMINATED") | ||
|
|
||
| args = [ |
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.
Should probably catch an exception here.
Delivery mechanism for Mustafa's & Keshav's commits for "persistent tools" support. We provide support for Prometheus based metric collection from
node_exporteranddcgmtools.Persistent tools are those tools started before any iteration runs, and end after all iterations have completed.
This work incorporates the changes from #1787 as well.