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: Add include_config parameter to remove config field from response #1317

Merged
merged 3 commits into from
Nov 21, 2016

Conversation

repeatedly
Copy link
Member

In our experience, almost users don't need config param because this parameter is not needed for buffer monitoring.
Removing it avoids unnecessary json processing and reduces response size.

config param itself is useful when user wants to check actual buffer configuration.
So add with_config query parameter to keep previous behaviour.

@repeatedly repeatedly added enhancement Feature request or improve operations v0.14 labels Nov 18, 2016
@repeatedly
Copy link
Member Author

@tagomoris How about this change?

@tagomoris
Copy link
Member

This fix changes default behavior, and there's no way to change plugin's default behavior (the only way to change output is to change fetcher - any other scripts or agents which users wrote).

I think it's better to have a new config_param to control whether output includes configuration or not without any query parameters.

@repeatedly repeatedly changed the title in_monitor_agent: config field is removed from response by default in_monitor_agent: Add include_config parameter to remove config field from response Nov 21, 2016
@repeatedly
Copy link
Member Author

Applied review and change PR title.

@tagomoris
Copy link
Member

test_configure should have an assertion about include_config. Others look good to me.

@repeatedly
Copy link
Member Author

Good point. I just added assertion. After test passed, I will merge this patch.

@repeatedly repeatedly merged commit 18cb438 into master Nov 21, 2016
@repeatedly repeatedly deleted the make-config-parame-disabled branch November 21, 2016 12:02
repeatedly added a commit that referenced this pull request Nov 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature request or improve operations v0.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants