Description of problem: Dont think we need /etc/collectd.d/postgresql.conf, thus can be removed as its obsolete by /etc/collectd.d/20-collectd_postgresql.conf Version-Release number of selected component (if applicable): ovirt-engine-metrics-1.0.4.2-1.el7ev.noarch How reproducible: 100% Steps to Reproduce: 1. [root@ls-engine1 ~]# cat /etc/collectd.d/postgresql.conf LoadPlugin postgresql 2. [root@ls-engine1 ~]# cat /etc/collectd.d/20-collectd_postgresql.conf [root@ls-engine1 ~]# cat /etc/collectd.d/20-collectd_postgresql.conf LoadPlugin postgresql <Plugin postgresql> <Query custom_deadlocks> Statement "SELECT deadlocks as num_deadlocks \ FROM pg_stat_database \ WHERE datname = $1;" Param database <Result> Type "pg_xact" InstancePrefix "num_deadlocks" ValuesFrom "num_deadlocks" </Result> </Query> <Database engine> Query custom_deadlocks # Query backends # Query transactions # Query queries # Query queries_by_table # Query query_plans Query table_states # Query query_plans_by_table # Query table_states_by_table Query disk_io # Query disk_io_by_table Query disk_usage </Database> </Plugin> Actual results: Leftover from previous patches still included Expected results: /etc/collectd.d/postgresql.conf should be removed
Is the file there in new installation as well? or in upgrade only?
fresh installation, I dont think we would mind leftovers after upgrade as we are not cleaning preexisting conf files
(In reply to Lukas Svaty from comment #2) > fresh installation, I dont think we would mind leftovers after upgrade as we > are not cleaning preexisting conf files I don't see this file in the ovirt-engine-metrics repo. We have /ansible/roles/ovirt_collectd/collectd_postgresql/templates/collectd_postgresql.conf.j2 that is deployed as /etc/collectd.d/20-collectd_postgresql.conf on the engine machine. Both files should exists once we run the ansible script. Can you please recheck this on a clean machine?
It seems to be added by package collectd-postgresql-5.7.1-4.el7.x86_64. Question is do we want to remove it on setting up our own conf file?
I will add to this that collectd-virt plugin also adds libvirt.conf which we also don't need. I would like to consider cleaning up all f the content of /etc/collectd.d/ before deploying the configuration files. This will allow us to make sure there are no unneeded plugins that are loaded. If a user would like to add additional plugins he should be instructed to include a separate directory in the main collectd.conf. Same for fluentd.
In this case I would also add a note on top of Include "/etc/collectd.d" in /etc/collectd.conf that this whole directory is under managment of ovirt and if desired to create new custom config directory
(In reply to Shirly Radco from comment #5) > I will add to this that collectd-virt plugin also adds libvirt.conf which we > also don't need. But is this a problem? > > I would like to consider cleaning up all f the content of /etc/collectd.d/ > before deploying the configuration files. I'd rather not. This isn't common behavior. > > This will allow us to make sure there are no unneeded plugins that are > loaded. > > If a user would like to add additional plugins he should be instructed to > include a separate directory in the main collectd.conf. And what if then the packagers of e.g. collectd-virt add their stuff there? You are starting a virtual fight here. I do not think you want to. > > Same for fluentd.
Some collectd plugin packages add a file to config.d directory once they are installed, that loads the plugin. This does not interfere, unless we decide not to use that plugin. After discussion it was decided that we will not remove unneeded collectd packages and/or the files and will not touch the files created by the plugin. This is to avoid deleting packages that were installed by the user. In fresh installation the package will not be installed and will add documentation on what should be cleaned on upgrade. Added a bug to collectd https://github.com/collectd/collectd/issues/2368
This seem to be a distributive packaging issue. Who should we ask to change this behavior?
(In reply to Shirly Radco from comment #9) > This seem to be a distributive packaging issue. What does this mean? > Who should we ask to change this behavior? If you're asking who's packaging collectd for oVirt, it's me, mrunge and rmeggins for CentOS OpsTools SIG. For Fedora Ruben Kerkhof <ruben> is the maintainer. But I've not understood what you want to change and why.
(In reply to Sandro Bonazzola from comment #10) > (In reply to Shirly Radco from comment #9) > > This seem to be a distributive packaging issue. > > What does this mean? It means it is dependent on the packaging and not the project. > > > Who should we ask to change this behavior? > > If you're asking who's packaging collectd for oVirt, it's me, > mrunge and rmeggins for CentOS OpsTools SIG. > For Fedora Ruben Kerkhof <ruben> is the maintainer. > > But I've not understood what you want to change and why. For some collectd plugins, when the package is installed, a load plugin file is added to /etc/collectd.d/.It load the file we no relation to our defined collectd configuration. So if we decide to disable the plugin in our conf file it will still be loaded by the pre-defined file.
(In reply to Shirly Radco from comment #11) > (In reply to Sandro Bonazzola from comment #10) > > (In reply to Shirly Radco from comment #9) > > > This seem to be a distributive packaging issue. > > > > What does this mean? > > It means it is dependent on the packaging and not the project. > > > > > Who should we ask to change this behavior? > > > > If you're asking who's packaging collectd for oVirt, it's me, > > mrunge and rmeggins for CentOS OpsTools SIG. > > For Fedora Ruben Kerkhof <ruben> is the maintainer. > > > > But I've not understood what you want to change and why. > > For some collectd plugins, when the package is installed, a load plugin file > is added to /etc/collectd.d/.It load the file we no relation to our defined > collectd configuration. So if we decide to disable the plugin in our conf > file it will still be loaded by the pre-defined file. I think having the default config enabling the plugin is expected behavior. You should be able to override the default config with an higher priority config file. If it doesn't work, you'll have to file a bug on collectd to allow overriding of other conf files based on priority.
(In reply to Sandro Bonazzola from comment #12) > I think having the default config enabling the plugin is expected behavior. > You should be able to override the default config with an higher priority > config file. If it doesn't work, you'll have to file a bug on collectd to > allow overriding of other conf files based on priority. There is a difference between "allowing overriding" (e.g. one file sets k=v1, a later file sets k=v2) and "unloading" a module. I do not think many services that allow loading DLLs at runtime/start time also allow "unloading" them. I do not think this is a problem in itself. It might be a problem if there are settings that can only be set in the Load command and cannot be changed/overridden later. Not sure this is the case.