Improve data directory detection

[msakrejda]

Currently, for gathering system stats on self-hosted systems, we look
up the current working directory of the postmaster process. This
usually works, but if customers have an unusual configuration, it can
find the wrong directory.

Instead, use the current method only as a fallback, and look at the
data_directory setting instead. This is readable with
pg_read_all_settings (part of pg_monitor), so it should be accessible
to us in most configurations.

[msakrejda]

I have not tested this yet. Opening for feedback on the approach first.

[lfittl]

I have not tested this yet. Opening for feedback on the approach first.

I'm curious, did you run an end to end test with this?

Wondering whether we should do a full test in a clean container/VM before we merge, or whether we feel its been sufficiently tested.

[msakrejda]

@lfittl I've instrumented the code to verify that the correct setting is picked up, but I have not done an end-to-end test. I'm trying to figure out something that would do the wrong thing on main but the right thing here.

[msakrejda]

Hmm, so I'm actually unable to reproduce the problem. If I start postgres on one filesystem partition and point it to a data directory on a different partition, /proc/<pid>/cwd actually points to the data directory. Tried this on both 15 and 10 with the same results. I think the change is safe, but I can't think of how to test it without being able to reproduce the problem. Maybe we should wait on this patch until we can reproduce the original problem.

[lfittl]

Hmm, so I'm actually unable to reproduce the problem. If I start postgres on one filesystem partition and point it to a data directory on a different partition, /proc/<pid>/cwd actually points to the data directory. Tried this on both 15 and 10 with the same results. I think the change is safe, but I can't think of how to test it without being able to reproduce the problem. Maybe we should wait on this patch until we can reproduce the original problem.

I wonder if one way to reproduce it could be to have multiple Postgres installations?

i.e. if the Postgres PID search in https://github.com/pganalyze/collector/blob/main/helper/main.go#L22 gets the wrong process, it would lead to the detected data directory being incorrect.

[msakrejda]

Oh good point. I'll try that.

[msakrejda]

Okay, confirmed that situation can cause bogus reporting on main but is fixed with this branch. Another case that this should improve is if Postgres is running as a user other than "postgres" (probably not common, but maybe there are some obscure configurations or packaging setups out there that use "postgresql" or something else).

[lfittl]

👍 Some small remaining comments, but looks ready to merge overall.

And thanks for doing the extra testing!