-
Notifications
You must be signed in to change notification settings - Fork 5
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
agents: add metrics transform API to agent #123
agents: add metrics transform API to agent #123
Conversation
7df5555
to
f08389e
Compare
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.
This looks much saner to me. LGTM!!
if (Inst().get() == this) { | ||
ASSERT_EQ(0, OnConfigurationHook(config_agent_cb, weak_from_this())); | ||
} |
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.
This is because of the test right? Why aren't you adding the ThreadAdded/RemovedHook methods inside that conditional as well?
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.
Mostly because I am being lazy. There are two main assumptions:
- A
StatsDAgent
instance will be used for the entire process lifetime - At most 2
StatsDAgent
instances will ever be created.
Using these assumptions, I decided it would be easier for each StatsDAgent
instance to set up its own hooks to check when a new thread is created/destroyed rather than have a single global hook that would iterate through a list of StatsDAgent
instances when a thread is created/destroyed. This simplified the logic a lot and will be easy to fix if we ever add the ability to remove a hook.
Though, now that I type this up I realize that all the stuff I added for the agent_list_
can be removed. Since it was never used. Will make that change.
Add API to allow retrieving all currently available SharedEnvInst instances. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
The user shouldn't have access to start/stop. No need to expose them. Also remove stop() completely. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Allow users to register a callback that will transform the metrics into a std::string that will be written to the endpoint. In order to do this some of the internals needed to be rewritten so that config() could be called from any thread. The internal usage of config() has been renamed since it will always be called from the StatsDAgent thread. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
f08389e
to
9b03aaa
Compare
Add API to allow retrieving all currently available SharedEnvInst instances. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
The user shouldn't have access to start/stop. No need to expose them. Also remove stop() completely. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Allow users to register a callback that will transform the metrics into a std::string that will be written to the endpoint. In order to do this some of the internals needed to be rewritten so that config() could be called from any thread. The internal usage of config() has been renamed since it will always be called from the StatsDAgent thread. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Add API to allow retrieving all currently available SharedEnvInst instances. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
The user shouldn't have access to start/stop. No need to expose them. Also remove stop() completely. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Allow users to register a callback that will transform the metrics into a std::string that will be written to the endpoint. In order to do this some of the internals needed to be rewritten so that config() could be called from any thread. The internal usage of config() has been renamed since it will always be called from the StatsDAgent thread. PR-URL: #123 Reviewed-by: Santiago Gimeno <santiago.gimeno@gmail.com>
Allow users to register a callback that will transform the metrics into a std::string that will be written to the endpoint.
Still working on tests.