Let empty tag values to the output

[mjf]

Relevant telegraf.conf

[[inputs.postgresql_extensible.query]]
  measurement = "postgres_locks"
  sqlquery = """
    select
      (
        select
          control_system.system_identifier
        from
          pg_catalog.pg_control_system() control_system
      ) cluster_id,
      coalesce(database.name, '') database_name,
      /* lot of SQL code omitted */
      count(locks.*) total_lock_count
    from
      pg_catalog.pg_locks locks
      left outer join (
        select
          datname name,
          oid oid
        from
          pg_catalog.pg_database
      ) database(name, oid)
      on
        locks.database is null or
        locks.database is not distinct from database.oid
    group by
      database.name;
  """
  tagvalue = "cluster_id,database_name"

Logs from Telegraf

N/A

System info

1.21.4

Docker

No response

Steps to reproduce

Collect a tag with empty string as it's value.

Expected behavior

Empty string values for tags present in the output.

Actual behavior

Empty string values for tags missing in the output.

Additional info

If I use some N/A or similar replacement tag value as a workaround (coalesce(database, 'N/A')) I will have that replacement value stored as a database name in the output which it obviously is not. This approach is also very error prone (i.e. there is no database of the name N/A, but even worse - who can tell that there eventually won't be some in the future). In this particular case the null means literally "no database name" which could be best expressed with empty string. Somebody could argue that empty string is still a string, but it's still far more better to use empty strings for expressing an absence of something than to have replacement strings that can potentially mean something one day IMHO (not taking into account that database name in this particular example cannot be empty string in most database systems, including Postgres)...

I could also do a "split" and collect two metrics instead of one (still considering the pg_locks example case) - one for databases and one for transactions. But that would lead to unnecessary bloat and would complicate thinks even more (i.e. now I can easily join with single metric; after the "split" I will have to join with two metrics, et cetera)...

Therefor I suggest to fix this and let empty tag values make it to the output if needed (with an option to either switch it on for individual [[inputs.postgresql_extensible.query]] sections or globally in [[inputs.postgresql_extensible]]). I would leave the current behaviour as default (to be honest some output may not accept empty tag values so that we do not break anything). Thank you.

[powersj]

Hi,

Can you provide some example metrics that you are gathering? I appreciate your lengthy discussion about how this might get fixed, but I would like to see what you see as well.

The plugin does specify that it expects the tag to be present and the code it appears to print a debug message if the tag is missing, so as you said, this would be a fairly large change in behavior that would need to be behind a new option.

However, before that is even considered I want you to demonstrate what the results look like before and after especially given there are numerous other options for resolving this already.

Thanks

[mjf]

Can you provide some example metrics that you are gathering?

There is. The example is an actual configuration that will work with a [[inputs.postgresql_extensible]] properly configured to
connect to some Postgres instance.

However, before that is even considered I want you to demonstrate what the results look like before and after especially given there are numerous other options for resolving this already.

# telegraf -test -config test.conf 
2022-03-17T13:33:45Z I! Starting Telegraf 1.21.4
2022-03-17T13:33:45Z I! Loaded inputs: postgresql_extensible
2022-03-17T13:33:45Z I! Loaded aggregators: 
2022-03-17T13:33:45Z I! Loaded processors: 
2022-03-17T13:33:45Z W! Outputs are not used in testing mode!
2022-03-17T13:33:45Z I! Tags enabled: host=host.example.net
> postgres_locks,cluster_id=123451234512345,db=postgres,host=host.example.net,server=user\=monitor\ host\=localhost\ port\=5432\ database\=postgres\  total_lock_count=3i 1647524026000000000
> postgres_locks,cluster_id=123451234512345,database_name=test,db=postgres,host=host.example.net,server=user\=monitor\ host\=localhost\ port\=5432\ database\=postgres\  total_lock_count=6i 1647524026000000000

As you can see the first output line does not contain the database_name tag despite the coalesce(database.name, '') which sanitizes the null value as '' (empty string). I would expect the tag to be present with a value of the empty string if the value is indeed an empty string and not a null value.

[mjf]

With debug flag set on we can easily see that the input plugin properly parses the empty string to database_name value:

# telegraf -debug -test -config test.conf 2>&1 |grep 'database_name ='
2022-03-31T07:46:45Z D! [inputs.postgresql_extensible] Column: database_name = string: 
2022-03-31T07:46:45Z D! [inputs.postgresql_extensible] Column: database_name = string: test

It also can not be the output plugin because we read W! Outputs are not used in testing mode! in the previous listing above. It must be somewhere between the input plugin's output and the output plugin's input in Telegraf code.

[powersj]

It must be somewhere between the input plugin's output and the output plugin's input in Telegraf code.

Yes, as a matter of fact it is actually two lines after the debug message here. Any column with a nil value is skipped over.

In terms of Telegraf, this does make some sense as an empty tag is not allowed in line protocol and we have had numerous previous bugs where tags with empty values cause issues (e.g. #9196, #4675, #4266).

[mjf]

we have had numerous previous bugs where tags with empty values cause issues

I understand that. But in some cases it can make sense to allow it. So I would let the current behaviour as default and provide some option to allow it if needed. Would that be possible?

[powersj]

Sorry for dropping this one.

Would that be possible?

Could you explain why you would want this? My concern is this is not the expected behavior and given the above mentioned issues, could cause more problems than it is worth.

[telegraf-tiger]

Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Page. Thank you!