[ZBX-19148] Agent2 Postgresql plugin-connector "database" name problems Created: 2021 Mar 22 Updated: 2022 Mar 09 |
|
Status: | Open |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Agent2 plugin (G) |
Affects Version/s: | 5.0.8, 5.4.0alpha2 |
Fix Version/s: | None |
Type: | Incident report | Priority: | Trivial |
Reporter: | Otheus | Assignee: | Aleksandre Sebiskveradze |
Resolution: | Unresolved | Votes: | 0 |
Labels: | None | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified | ||
Environment: |
Unix, Postgresql |
Attachments: |
![]() |
Description |
This is a complaint about an external bug, a minor bug, a major bug, a design flaw, and a flaw in the zabbix-agent2 architecture. Let us start small. The external bug is that if the "database" field is set, it is passed onto the third-party library as a parameter, instead of handling it and updating the connection string with `dbname=` (see t line 211 of the plugin code). The DBNAME field is usually only set when using autodiscovery on per-database instance metrics. However, if set as a parameter in the ZA2 conf file such as with a session, it could cause a problem. I say 'could' because (1) this is actually a bug that underlies the Postgresql plugin, ie, a 3rd party PG library. In that code, if the "database" config parameter is set, it appends "database=<value>" to the connection string, and (2) Postgresql (12) server allows it anyway, despite this behavior being disallowed by `libpq` agents and by `pgsql` (see below) and as stated in the documentation to[ the contrary.|https://www.postgresql.org/docs/12/libpq-connect.html] It is also very unclear what should be the behavior if the session is configured with a database name in the URI but the database name conflicts with that. I propose that as a workaround, ZA2 ignores with a warning the use of a parameter such as The reason the above became an issue is because I wanted to (1) use UNIX sockets to connect to the database and (2) select a database to connect to. Now, it turns out, that #2 was the wrong thing to want in my case. But If I did want that, it wouldn't work because of the above bug AND the following: The ZA2 does not properly parse `unix:` URIs. The code referenced above attempts to treat the UNIX-type URI as a URI, but then complains when there is a query portion. The query portion is supposed to be parsed ala Go's net/url `QueryParse` routine. Instead, we see: The code in `conn.c` treats the string as a URI, so this should work, but the code in metrics.go#129 ensures such a query is invalid. As such, for the unix:// socket, the only way to specify a database name is using the database parameter, which as stated above, has this underlying flaw which exploits an undocumented feature of the postgresql server. So, fix the regexp in metrics.go, add a warning for use of `database`. Now we get to the major bug. It turns out, if we use the "Session" configuration to specify the database path, username, password, the ZA2 will refuse to use auto-discovered rules that are specific to database-instances on the Postgresql cluster. Here is the error: The configuration in ZA2 is: This brings us to the specific design flaw in the Postgresql Plugin. One questions the wisdom of using '`tcp` and `unix` URI specifications in the first place, when for database servers, there exists already a very standard RFC for database connections. Why aren't you using the standard thing instead of reinventing your own? There is also the question of why rely on non-standard third-party libraries, especially which are (1) not audited, (2) not under your control, (3) not from a major 3rd party vendor (such as MySQL/Oracle/Postgresql/Google/IBM). It would be far saner to use a C-lib-to-Go wrapper utility for libraries such as libpq. Again, why aren't you using the standard thing instead of relying on someone else's reinvention? But please, don't let my curmudgeonry get in the way of a solid bug report. |
Comments |
Comment by Otheus [ 2021 Mar 22 ] |
Update. Doing a tcpdump on psql connections to a database, it appears the internal protocol string for selecting the database is in fact `database` and not `dbname`. So it's possible the "external bug" is actually an artifact of the output for a different error condition. Nevertheless, due to the other issues I've raised, I suggest that the connection information be contained in the URI, but overridable by the items, ie, auto-discovered database-specific items. |