Postgres exporter is out of date

[robertbublik]

I noticed that the image of the postgres exporter in the dbinstances chart uses the latest tag and the last update was over 4 years ago. From what I see this is the currently maintained postgres exporter:

https://github.com/prometheus-community/postgres_exporter
https://github.com/prometheus-community/helm-charts/tree/main/charts/prometheus-postgres-exporter

Unfortunately it's not possible to simply change the image in the dbinstances chart because there are many incompatible changes like different ports the application listens on. The current exporter runs on 60000 while the updated one runs on 9187.

I would like to upgrade the exporter and also add podmonitor support.
Is this fine for you @allanger?

[allanger]

Yep, sure, I'm not really using exporters atm, so I couldn't have noticed it.
Though I don't like helm dependencies, maybe this is a good place for adding one? As I see they don't have podMonitors in their chart, but it should be possible to add them via extraManifests, and then we won't have to maintain the exporter anymore at all.

But if you think that it's not a good idea, you can also put all the templates to our chart, and also probably add tests for podMonitor as now we only have serviceMonitors covered

[mario-steinhoff-gcx]

Though I don't like helm dependencies, maybe this is a good place for adding one?

The db-instances chart currently generates one exporter Deployment per DbInstance. I'm not sure if helm can instantiate the same dependency dynamically multiple times.

The current exporter runs on 60000 while the updated one runs on 9187.

Not sure what the default port was back then but the db-instances helm chart overrides this port via a environment variable that was removed last year:

• Update exporter-toolkit prometheus-community/postgres_exporter#746

I dont know if there was a particular reason to choose a non-default port, if not I'd suggest to stick to the defaults.

General question: is the db-instances helm chart strictly tied to the implementation of db-operator and must always be used, or rather provided as a convenient mechanism to create DbInstance custom resources?

[allanger]

The db-instances chart currently generates one exporter Deployment per DbInstance. I'm not sure if helm can instantiate the same dependency dynamically multiple times.

Aye, sure, I haven't though about that, than we need to put them to templates

I dont know if there was a particular reason to choose a non-default port, if not I'd suggest to stick to the defaults.

I am not aware of the reasons, to be honest, I would also use the default one, I don't think there are reasons to stay with the custom one

General question: is the db-instances helm chart strictly tied to the implementation of db-operator and must always be used, or rather provided as a convenient mechanism to create DbInstance custom resources?

Second, you don't have to use the chart, if you don't want

[robertbublik]

@allanger could you please have a quick look at my draft PR if the changes look fine for you? Then I would start working on the test coverage. I'm not sure if you rely on your custom queries, they have been deprecated with lots of other features. 🌞

[allanger]

I've had a look and put a couple of comments there

[allanger]

Can we close it now?

[robertbublik]

Yes I haven't found any issues.