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

in_monitor_agent: enable to select instance variables of plugins #1393

Merged

Conversation

autopp
Copy link

@autopp autopp commented Dec 28, 2016

I often use in_monitor_agent with /api/plugins.json&debug=1 for more complexity plugin monitoring and inspecting.
In this case, only some instance variables of a specific plugin is needed.
Unfortunary, &debug=1 makes responce size too large. (Also, pretty printing is not needed...)

To reduce size of response, this PR add query parameter with_ivars for /api/plugins.json.
with_ivars should be a names sepalated by comma.
When with_ivars is given, the response has instance_variables that contains only specified instance variables for each plugin(This is the same as '&debug=1').

E.g.

$ curl 'http//localhost:24220/api/plugins.json?with_ivars=var1,var2'

If with_ivars is not good name, please give me other idea. (E.g. with_instance_variables)


data(:with_config_and_retry_yes => [true, true, false, "?with_config=yes&with_retry"],
:with_config_and_retry_no => [false, false, false, "?with_config=no&with_retry=no"],
:with_ivars_given => [false, false, true, "?with_config=no&with_retry=no&with_ivars=id,num_errors"])
Copy link
Member

@repeatedly repeatedly Jan 3, 2017

Choose a reason for hiding this comment

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

Please separate tests.

Copy link
Author

Choose a reason for hiding this comment

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

elsif ivars = opts[:ivars]
iv = {}
ivars.each {|name|
iname = :"@#{name}"
Copy link
Member

@repeatedly repeatedly Jan 3, 2017

Choose a reason for hiding this comment

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

"@#{name}" should work.

Copy link
Author

Choose a reason for hiding this comment

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

@repeatedly I chose symbol literal instead of string because reduce garbage. Is this too careful?

Copy link
Member

Choose a reason for hiding this comment

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

What does 'garbage' mean?
:"@#{name}" is dynamic symbol genaration via string, not static literal.

Copy link
Author

Choose a reason for hiding this comment

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

@repeatedly Sorry, I had misunderstood. I will update patch, so please wait.

@repeatedly repeatedly merged commit f54cab2 into fluent:master Jan 5, 2017
@autopp
Copy link
Author

autopp commented Jan 5, 2017

@repeatedly Thanks for review and merge! But, I need this feature for v0.12.x also.
Can I create a pull request for the v0.12 branch with the same patch?

@repeatedly
Copy link
Member

@autopp Yeah. This is good > for v0.12

@autopp
Copy link
Author

autopp commented Jan 5, 2017

@repeatedly OK, please wait...

@autopp autopp mentioned this pull request Jan 5, 2017
@autopp
Copy link
Author

autopp commented Jan 5, 2017

I created pull request #1402 to port this feature.

repeatedly added a commit that referenced this pull request Jan 7, 2017
@autopp autopp deleted the select_instance_variables_in_monitor_agent branch January 8, 2017 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants