Description of problem: Move all "Load plugin" to the top of the collectd configuration file so it will be easier for the user to understand what plugins are loaded.
Correct me if I'm wrong but I would imagine all LoadPLugin would be in single file. Engine: [root@ls-engine1 ~]# grep -R "^LoadPlugin" /etc/collectd.* /etc/collectd.conf:LoadPlugin syslog /etc/collectd.conf:LoadPlugin cpu /etc/collectd.conf:LoadPlugin interface /etc/collectd.conf:LoadPlugin load /etc/collectd.conf:LoadPlugin memory /etc/collectd.d/postgresql.conf:LoadPlugin postgresql /etc/collectd.d/30-write_http.conf:LoadPlugin write_http /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin disk /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin cpu /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin memory /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin load /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin nfs /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin entropy /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin swap /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin df /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin interface /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin aggregation /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin processes /etc/collectd.d/20-builtins-conf-for-ovirt-engine.conf:LoadPlugin uptime /etc/collectd.d/20-collectd_postgresql.conf:LoadPlugin postgresql Host: [root@localhost ~]# grep -R "^LoadPlugin" /etc/collectd.* /etc/collectd.conf:LoadPlugin syslog /etc/collectd.conf:LoadPlugin cpu /etc/collectd.conf:LoadPlugin interface /etc/collectd.conf:LoadPlugin load /etc/collectd.conf:LoadPlugin memory /etc/collectd.d/30-write_http.conf:LoadPlugin write_http /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin disk /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin cpu /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin memory /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin load /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin nfs /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin entropy /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin swap /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin df /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin interface /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin aggregation /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin processes /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin uptime /etc/collectd.d/10-read-statsd.conf:LoadPlugin statsd tested in ovirt-engine-metrics-1.1.1-0.0.master.20170919065728.el7.centos.noarch
No. Sorry. This fix only refers to the main files: 20-builtins-conf-for-ovirt-engine.conf and 20-builtins-conf-for-ovirt.conf. There are additional files that load additional files before this change that we did not touch.
Don't think this helps in any way on readability, admin still needs to look through different files to search where which plugins is enabled. I would even say it might confuse the admin to not to see the configuration of the collectd plugin right after where it's enabled, and have to search for it somewhere else. I would go with the common collectd structure: Load Plugin A Configuration A Load PLugin B Configuration B Plugins like memory, cpu, interface, load... are loaded in multiple config files: /etc/collectd.conf:LoadPlugin memory /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin memory We should have single point of entry, thus if admin wants to edit this he should be able to in single config files. Please add feature page where each configuration of collectd plugins are located and what are the predefined values. For now we have only this one[1], which is not sufficient. [1] https://www.ovirt.org/develop/release-management/features/metrics/metrics-store-collected-metrics/
(In reply to Lukas Svaty from comment #3) > Don't think this helps in any way on readability, admin still needs to look > through different files to search where which plugins is enabled. I would > even say it might confuse the admin to not to see the configuration of the > collectd plugin right after where it's enabled, and have to search for it > somewhere else. I would go with the common collectd structure: > > Load Plugin A > Configuration A > Load PLugin B > Configuration B There is no "commom" collectd structure. This was discussed also with PM and this gives a one look to see which plugins are loaded at the top on the config file. > > > Plugins like memory, cpu, interface, load... are loaded in multiple config > files: This is due to packaging that add these files as part of the plugin. It was already discussed with Didi, Sandro and PM and was decided to keep as is. If we decide to stop using a plugin it will have to be uninstalled and the file will be removed with it. In this case the "/etc/collectd.conf:LoadPlugin memory" is the file that was deployed with the plugin package. > > /etc/collectd.conf:LoadPlugin memory > /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin memory > > We should have single point of entry, thus if admin wants to edit this he > should be able to in single config files. > Having a single file is not a requirement. Admin is not supposed to edit any of these files. We do not support that. > Please add feature page where each configuration of collectd plugins are > located and what are the predefined values. For now we have only this > one[1], which is not sufficient. > > [1] > https://www.ovirt.org/develop/release-management/features/metrics/metrics- > store-collected-metrics/ wip. Not part of this RFE. Please move back to ON_QA.
(In reply to Shirly Radco from comment #4) > (In reply to Lukas Svaty from comment #3) > > Don't think this helps in any way on readability, admin still needs to look > > through different files to search where which plugins is enabled. I would > > even say it might confuse the admin to not to see the configuration of the > > collectd plugin right after where it's enabled, and have to search for it > > somewhere else. I would go with the common collectd structure: > > > > Load Plugin A > > Configuration A > > Load PLugin B > > Configuration B > > There is no "commom" collectd structure. With common, I meant similar to /etc/collectd.conf > > This was discussed also with PM and this gives a one look to see which > plugins are loaded at the top on the config file. > This does not seem to me as user-friendly, as the configuration is an important part of each collectd plugin and admin does not want to run around the file after he enabled the collectd plugin. Yaniv can we revisit this decision? > > > > > > Plugins like memory, cpu, interface, load... are loaded in multiple config > > files: > > > This is due to packaging that add these files as part of the plugin. > It was already discussed with Didi, Sandro and PM and was decided to keep as > is. > If we decide to stop using a plugin it will have to be uninstalled and the > file will be removed with it. In this case the > "/etc/collectd.conf:LoadPlugin memory" is the file that was deployed with > the plugin package. > Still seems like a mess in the configuration file, Yaniv what do you think? > > > > /etc/collectd.conf:LoadPlugin memory > > /etc/collectd.d/20-builtins-conf-for-ovirt.conf:LoadPlugin memory > > > > We should have single point of entry, thus if admin wants to edit this he > > should be able to in single config files. > > > > Having a single file is not a requirement. > I agree, however, at the moment we have most of the plugins in this file, while they can be spread among multiple files for more visibility, or cleaned up in 1 file where it would make more sense for admins etc.. 20-ovirt-postgres.conf 20-ovirt-resources.conf ... > Admin is not supposed to edit any of these files. We do not support that. > Fact that we do not support is not always the deciding factor for admins if they can get few more statistics from collectd or configure the collectd plugins to the format they prefer we might want to do rather do a QuickFix. Thoughts Yaniv? > > Please add feature page where each configuration of collectd plugins are > > located and what are the predefined values. For now we have only this > > one[1], which is not sufficient. > > > > [1] > > https://www.ovirt.org/develop/release-management/features/metrics/metrics- > > store-collected-metrics/ > > wip. Not part of this RFE. > Do we have any other BZ where this feature page is considered as necessary. This one seems to be like the best RFE where we can document these collectors. > Please move back to ON_QA.
There is an option of having per plugin configuration file. Pros: It can perhaps give a better visualization to which plugins are loaded. Cons: Handling of this file in case we decide to stop reporting a specific plugin. We will have to remove this file, since just commenting out the "loadPlugin" text will confuse the user since the file still exists. We decided to avoid cleaning up files, so having per plugin configuration file will have to reconsider this decition. Can you please open an FRE with requirements and I'll discuss this with Yaniv L?
Bug requested - https://bugzilla.redhat.com/show_bug.cgi?id=1498579 I don't see a problem in removing a conf file when deprecating a plugin. I still disagree all LoadPlugin should be in the top of the file as it would be harder to find a configuration for the relevant plugin. And you can easily search for the Loaded plugins with 1 command of basic Admin skills - grep -R "^LoadPlugin" /etc/collectd.* I disagree this feature should go through and suggest for a revert to the old version. However, the patch included is correct even if the design is questionable. I'll leave the decision to PM. Yaniv thoughts?
(In reply to Lukas Svaty from comment #7) > Bug requested - https://bugzilla.redhat.com/show_bug.cgi?id=1498579 > > I don't see a problem in removing a conf file when deprecating a plugin. > > I still disagree all LoadPlugin should be in the top of the file as it would > be harder to find a configuration for the relevant plugin. And you can > easily search for the Loaded plugins with 1 command of basic Admin skills - > grep -R "^LoadPlugin" /etc/collectd.* > > I disagree this feature should go through and suggest for a revert to the > old version. However, the patch included is correct even if the design is > questionable. > > I'll leave the decision to PM. Yaniv thoughts? I also agree that LoadPlugin is more readable with sections over putting everything on top and suggest a revert. Shirly, please discuss with integration.
I tend to agree with Lukas here, I find having the conf of a plugin right after loading it easier to read. No strong opinion, though, and since these files are not meant to be manually edited by admins, but mostly by the ovirt-engine-metrics maintainer, perhaps the maintainer should decide...
verified in ovirt-engine-metrics-1.1.1-0.3.beta2.20171114114644.el7ev.noarch
This bugzilla is included in oVirt 4.2.0 release, published on Dec 20th 2017. Since the problem described in this bug report should be resolved in oVirt 4.2.0 release, published on Dec 20th 2017, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report.