NULL fields spamming warnings

[qfritz]

Hello 😊

I jumped from v0.16.0 to v0.18.0 and sql exporter spammed the logs with warnings at each poll. Example:

{..."level":"WARN",
"source":"query.go:226",
"msg":"Value column is NULL",
"logContext":"job=pg-database-metrics-▮▮▮▮▮▮,
target=▮▮▮▮▮▮▮▮▮▮,
collector=postgres_13_database_pg_statio_user_tables,
query=query_pg_statio_user_tables",
"column":"tidx_blks_read"}

Indeed, looking at query.go each null value raise a warning.

slog.Warn("Value column is NULL", "logContext", q.logContext, "column", column)

My questions are the following:

• Are NULL values in results still supported (I recall this issue which was fixed)?
• What's the "happy path" around null values? Using SQL to work around the issue sounds tedious. Maybe we could improve the message to give more details about what's expected from us to resolve it.
• Could we disable those warnings somehow?

I worked around it by using -log.level error, but there might be some important warnings I'm missing now. 😄

Thanks for reading and best regards,
Fritz

[burningalchemist]

Hey @qfritz,

Answering your questions:

• Yeah, the NULL values are supported (or at least I'm trying my best supporting them); The main difference just for clarity is that we cannot really treat NULL as 0 in the metric definition. And for the Prometheus metric the value must be float64. So we are able to handle the result set with NULL values, but discard it later when it's about to turn into a metric;
• The idea with such queries was that there are queries where some columns contain NULL values (but not permanent), so when there is a value - we scrape it, otherwise we skip and inform the user in the log;
• I'd say if your the constantly returns NULL, that probably it's easier to rework the query.

I think we have several options:

• we may try to put it behind Debug log level (i.e. sort of disabling it unless users want debug logs to discover this);
• maybe set it to Info log level, so it's a bit more visible, but users still have an option to reduce the log stream by moving to Warning/Error log level;
• add a command-line flag to disable this particular message. I'm not really in fond of this solution as it adds those edge case flags and makes it look ambiguous and confusing.

The only motivation behind this message was to provide enough visibility to the users.

I'm generally open to suggestions. What do you think?

[qfritz]

Here are my thoughts:

• Warning might be a tad too high, I'm in favor of lowering this to Info level or Debug.
• We might want to inform in the FAQ the behavior around nulls with some paragraph:

Null values are supported but we cannot treat NULL as 0 in metric definition, so we discard them when turning the result into a metric. The idea with such queries was that there are queries where some columns contain NULL values (but not permanent), so when there is a value - we scrape it, otherwise we skip and inform the user in the log at Debug level. This is a workaround so if your query constantly returns NULL, you should probably rework your query.

What do you think?

Thanks for the details, I do agree with your motivation and the visibility provided to users. 😊
Have a nice day,

[burningalchemist]

@qfritz Sounds good to me! I'll move it to the Debug log level in the next release, and we'll see when/if users come for answers haha 😃👍

Thanks for your feedback! 👍

[burningalchemist]

@qfritz JFYI, the new version v0.18.1 is available. 👍