Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query time (and other actions) produce arbitrary output #172

Open
mschmitzer opened this issue Jul 10, 2020 · 2 comments
Open

Query time (and other actions) produce arbitrary output #172

mschmitzer opened this issue Jul 10, 2020 · 2 comments

Comments

@mschmitzer
Copy link

Due to the inclusion of the offending Query in the output of the "query_time" action and others (f958d61), the output produced by those actions can contain essentially arbitrary data. The printed SQL query can easily contain pipe characters ("|"), which are used by Nagios and Icinga to separate the Check output from the perfomance data. Having a pipe character in the check output confuses the parser and leads to bogus performance data.

It would be good if there was an option to suppress output of the query string. Alternatively, the query string could be "sanitized" (and possibly truncated to a maximum length).

@machack666
Copy link
Collaborator

I'm fine with adding a query suppression option if one doesn't exist, however I don't think we want to get into the sanitation business for this. Since this isn't an arbitrary query provided by someone else (i.e., it comes from whoever wrote the checks) it would be their responsibility to make sure things are sensible to their own monitoring systems.

@mschmitzer
Copy link
Author

I agree that trying to solve this through sanitation is messy.

But I do think that check_postgres.pl has the responsibility to produce well-formed output here. We're not talking about the "custom query" action where the query is actually part of the check, but about "query time" and "idle in transaction", where the query in question can be any query that happens to run on the cluster. So in a moderately sized organization, the query is not written by whoever wrote the checks, but by application developers who may not be much involved in the monitoring of the database cluster. And telling devs not to use pipes in their queries is not really an option.

So at least in "nagios" Output mode, the query should not be printed by default (unless it is sanitized) because doing so can produce invalid output that has to be processed by the monitoring system.

I patched our copy of check_postgres.pl to do the sanitizing and truncating, but I'm not a Perl programmer, so it's not particularly elegant. I can provide it, though, if anybody cares.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants