Description of problem: The postgresql collectd plugin requires the engine database name, host, user and password, in order to query it. We need to add a way for the collectd.conf can consume this information.
We might be able to do this by making engine-setup create a file [1] with something like: [Service] ExecStartPre=/path/to/create_pgpass.sh Environment=PGHOST=dbhost Environment=PGPORT=dbport Environment=PGDATABASE=dbname Environment=PGUSER=dbuser Environment=PGPASSFILE=[2] where /path/to/create_pgpass.sh will be a script that securely creates pgpass in [2] and the other credentials are copied from existing. [1] /etc/systemd/system/collectd.service.d/postgresql.conf [2] /var/lib/ovirt-collectd/pgpass/engine.pgpass
Isn't a read only account sufficient?
(In reply to Yaniv Kaul from comment #2) > Isn't a read only account sufficient? I think it is a good solution for user and password. But we need to maintain the host and db name in case they change. With the solution Didi is proposing it should be easier in case of an update.
(In reply to Shirly Radco from comment #3) > (In reply to Yaniv Kaul from comment #2) > > Isn't a read only account sufficient? Probably yes, depending on what we want to do there. > > I think it is a good solution for user and password. > But we need to maintain the host and db name in case they change. > With the solution Didi is proposing it should be easier in case of an update. Indeed. If we want to allow using custom credentials, and use comment 1, it might be enough for the user to add another similar file to /etc/systemd/system/collectd.service.d/ with these credentials, need to check about the order. I guess last wins, so would name it zzz-postgresql.conf. Perhaps we should name ours 10-postgresql.conf for more explicit ordering.
Yaniv, Do we want to support local db only? If yes, It will require adding its config file only for local db installations.
(In reply to Shirly Radco from comment #5) > Yaniv, Do we want to support local db only? > If yes, It will require adding its config file only for local db > installations. We discussed in scrub meeting. We will add it only to local db.
Note: There is no (simple/robust) way to know if the db is local. We'll test if its host equals 'localhost', which is what engine-setup configures if choosing 'Local'. This means, among other things, that users that chose "Remote", then input e.g. '127.0.0.1', will cause their db to be considered remote. OK?
(In reply to Yedidyah Bar David from comment #7) > Note: There is no (simple/robust) way to know if the db is local. We'll test > if its host equals 'localhost', which is what engine-setup configures if > choosing 'Local'. This means, among other things, that users that chose > "Remote", then input e.g. '127.0.0.1', will cause their db to be considered > remote. OK? Can we check both localhost and '127.0.0.1' ?
(In reply to Shirly Radco from comment #8) > Can we check both localhost and '127.0.0.1' ? We can do whatever we want, but why? A default setup using automatic provisioning will have 'localhost'. Users doing anything else, most likely have their reasons. Personally I do not see why we should monitor only if local. I didn't attend the meeting in which this was decided, sorry - but perhaps the reason should be documented here as well. If we go that route, do we also check other less or more complex options? E.g. you can have in /etc/hosts '127.0.0.1 localhost dbserver' and use 'dbserver'. If you then want us to also consider that one as local, we need to also lookup. And you can also obviously set it to an address of another interface of the local machine. Thinking about this, I think we'll allow enforcing this anyway, so that users that want to monitor the db can do that regardless of where it is.
Shirly opened another bug for this, bug 1436598. So I think we can move the discussion there.
From bug regarding creating different files for postgres load BZ#1436001: we need to tackle renaming this file to standard priority format 40-postgresql.conf and also filling the file, as it at the moment only contains loading of the plugin: [root@ls-engine1 ~]# cat /etc/collectd.d/postgresql.conf LoadPlugin postgresql
(In reply to Yedidyah Bar David from comment #9) > > Personally I do not see why we should monitor only if local. I didn't attend > the meeting in which this was decided, sorry - but perhaps the reason should > be documented here as well. > - We can not assume we are the owners of the postgres and have permission to the stats. - Most likely it is monitored on the remote site - We need to be able to work out of the box and not have collectd fail on no prmissions to stats table for example.
(In reply to Yaniv Dary from comment #12) > - We can not assume we are the owners of the postgres and have permission to > the stats. Indeed, and this is true also for a local db. We are going to use engine db creds, which should work the same whether local or remote. > - Most likely it is monitored on the remote site Indeed. But IMO there is some added value in monitoring and keeping stats etc. in our own store even then. E.g. one day we might base decisions on these stats, and would like to be able to make these decisions also for remote dbs. > - We need to be able to work out of the box and not have collectd fail on no > prmissions to stats table for example. See above. Do we need permissions that the engine db user does not have? If so, I suggest to open a bug for that and we'll discuss it there. For current bug, I suggest to start with queries that the engine user can query.
Seems like we already have bug 1436598 for this - please reply there. Thanks. For current bug I do not separate remote/local.
Lukas, why move from post to assigned/failedqa? Patch is still pending merge.
hmm, seem like a wrong updated bug, moving back to post, sorry for the inconvenience
Sorry, Shirly found a bug.
Note to QE: Please test this with both local and remote db. Verify that you do not get errors in collectd logs (perhaps syslog) about connecting to db.
tested with local and remote database both reporting correctly by plugin postgresql searchable by collectd.plugin == postgresql these value types: "type": "pg_db_size" "type": "pg_blks", " "type": "pg_n_tup_g" "type": "pg_xact" last two with different type_instances Is this correct? Or should we expect some information regarding data in tables as well, I dont see it configured in the collectd config anyway. Example sample: { "_index": "ovirt-metrics-2017.06.21", "_type": "fluentd", "_id": "AVzKmJd-GNi9AvMt77i4", "_score": null, "_source": { "hostname": "ls-example.com", "collectd": { "dstypes": [ "derive" ], "interval": 10, "plugin": "postgresql", "plugin_instance": "engine", "type": "pg_blks", "type_instance": "heap_hit", "postgresql": { "pg_blks": 2.69932371916975 } }, "tag": "project.ovirt-metrics-engine", "ovirt": { "entity": "engine" }, "ipaddr4": "10.34.63.61", "ipaddr6": "2620:52:0:223c:21a:4aff:fe80:3f6c", "@timestamp": "2017-06-21T14:20:03+02:00" }, "fields": { "@timestamp": [ 1498047603000 ] }, "highlight": { "collectd.plugin": [ "@kibana-highlighted-field@postgresql@/kibana-highlighted-field@" ] }, "sort": [ 1498047603000 ] }
moving to VERIFIED for now, if you believe further verification is requried please reopen
IMO Current bug is only about making collectd able to access engine's db (automatically, unlike when verifying bug 1434570). Perhaps your question should be asked in the scope of bug 1436087, or a new bug. Moving needinfo to Shirly for review.
Answered in bug 1436087.
The documentation updates for this bug is being tracked in https://bugzilla.redhat.com/show_bug.cgi?id=1451625