Proper Tool Metadata Abstraction #1787
Conversation
Maxusmusti
added
Agent
enhancement
tools
|
Created a more proper and up-to-date branch for this work. |
npalaska
left a comment
npalaska
left a comment
There was a problem hiding this comment.
Its still a draft but I left few comments
|
@Maxusmusti, is this really ready for review? Typically, if a PR is in |
|
Also what necessitated a new PR? Typically, there are steps one can take with |
portante
force-pushed
the
tool-meister
branch
from
d930bac to
ecbe594
Compare
|
The reason I had assigned reviewers was simply for consistency (added the same people who were marked as reviewers on the old PR). Also I was (and still am) unaware of a means by which one switches the branch that a PR originates from, so I made a fresh PR for the more properly named and updated branch (old one had some issues). |
portante
force-pushed
the
tool-meister
branch
from
ecbe594 to
e756ca2
Compare
Maxusmusti
force-pushed
the
metadata
branch
from
bdc5374 to
0d72d40
Compare
portante
left a comment
portante
left a comment
There was a problem hiding this comment.
As this is a Draft, it goes without saying this is not ready for merging, but I'd like to know what the intent is for using metaclass module?
There was a problem hiding this comment.
This actually made me think: should we sort the list of keys so dcgm and node-exporter don't get moved to the end of the list?
There was a problem hiding this comment.
@dbutenhof, just do confirm in writing, do you think we can do that later, or would you want to see the sorting done in this PR?
There was a problem hiding this comment.
@portante @Maxusmusti I approved the PR, which I figured would be clear enough; but for the record, yes, sure. It's a pretty trivial UX issue; it'd be more consistent and maybe a bit more clear, but I doubt most people really pay attention to the full list of tools in the usage.
There was a problem hiding this comment.
I can easily change that once Chuck's work has been integrated, I'll be sure to sort in the future
portante
left a comment
portante
left a comment
There was a problem hiding this comment.
Looks good! Just needs a rebase and a squash.
|
It should already be rebased on the tool-meister branch, unless there was another new merge today. Should I squash the last chunk of updates and force push to metadata, or would it be better to squash through the "squash and merge" option |
This work adds the notion of a "collector" to the Tool Data Sink, and
"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` "tool" is
run providing a 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`).
Co-authored-by: maxusmusti <meyceoz@redhat.com>
…mentation within ToolMeister obj)
…and fixed code reviews
* Fixed logic flaw resulting in unecessary error logging (#5) * Fixed most code review suggestions and tm-start logic flaw
Maxusmusti
force-pushed
the
metadata
branch
from
ebe964d to
dc862b4
Compare
Maxusmusti
force-pushed
the
metadata
branch
from
dc862b4 to
7ef2b32
Compare
|
Ok now it's ACTUALLY rebased, it seems as though I had botched it earlier |
5 participants
Complete tool metadata abstraction work!
(Still code review changes to iron out)
On new branch, same as PR #1778