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

postgresql / postgresql_extensible: db label (dbname) changed #14007

Closed
hloeung opened this issue Sep 26, 2023 · 5 comments · Fixed by #14010
Closed

postgresql / postgresql_extensible: db label (dbname) changed #14007

hloeung opened this issue Sep 26, 2023 · 5 comments · Fixed by #14010
Labels
bug unexpected problem or unintended behavior

Comments

@hloeung
Copy link

hloeung commented Sep 26, 2023

Relevant telegraf.conf

[[inputs.postgresql_extensible]]
  address = "dbname=telegraf host=10.131.124.172 password=my-very-secure-password-redacted port=5432 user=jujuadmin_telegraf"
  # Using the charm default server.
  outputaddress = "10.131.124.172"

Logs from Telegraf

2023-09-21T04:16:06Z I! Starting Telegraf 1.28.1 brought to you by InfluxData the makers of InfluxDB
2023-09-21T04:16:06Z I! Available plugins: 240 inputs, 9 aggregators, 29 processors, 24 parsers, 59 outputs, 5 secret-stores
2023-09-21T04:16:06Z I! Loaded inputs: bond cgroup conntrack cpu disk diskio exec (7x) internal kernel_vmstat mem net netstat nstat postgresql_extensible processes socket_listener swap system
2023-09-21T04:16:06Z I! Loaded aggregators:
2023-09-21T04:16:06Z I! Loaded processors:
2023-09-21T04:16:06Z I! Loaded secretstores:
2023-09-21T04:16:06Z I! Loaded outputs: prometheus_client
2023-09-21T04:16:06Z I! Tags enabled: host=prod-ua-db:postgresql-4 juju_application=postgresql juju_model=prod-ua-db juju_unit=postgresql-4
2023-09-21T04:16:06Z I! [agent] Config: Interval:10s, Quiet:false, Hostname:"prod-ua-db:postgresql-4", Flush Interval:10s
2023-09-21T04:16:06Z D! [agent] Initializing plugins
2023-09-21T04:16:06Z W! DeprecationWarning: Value "false" for option "ignore_protocol_stats" of plugin "inputs.net" deprecated since version 1.27.3 and will be removed in 1.36.0: use the 'inputs.nstat' plugin instead
2023-09-21T04:16:06Z D! [agent] Connecting outputs
2023-09-21T04:16:06Z D! [agent] Attempting connection to [outputs.prometheus_client]
2023-09-21T04:16:06Z I! [outputs.prometheus_client] Listening on http://[::]:9103/metrics
2023-09-21T04:16:06Z D! [agent] Successfully connected to outputs.prometheus_client
2023-09-21T04:16:06Z D! [agent] Starting service inputs
2023-09-21T04:16:06Z I! [inputs.socket_listener] Listening on tcp://127.0.0.1:8094

System info

Telegraf 1.28.1, Ubuntu 20.04.6 LTS

Docker

No response

Steps to reproduce

  1. Install Telegraf
  2. Setup Postgresql
  3. Add inputs.postgresql_extensible config snippet using a DSN config string to connect
    ...

Expected behavior

Using an older telegraf, 1.25.0, we get this:

$ curl -s localhost:9103/metrics | grep '^postgresql_lag'
postgresql_lag_bytes{db="postgres",host="prod-ua-db:postgresql-2",replica="master",server="10.xxx.xxx.xxx",standby="10.xxx.xxx.xxx"} 0

Actual behavior

Using a newer telegraf, 1.28.1 with patches from Josh per #13965), we now get this:

$ curl -s localhost:9103/metrics | grep '^postgresql_lag'
postgresql_lag_bytes{db="10.xxx.xxx",host="prod-ua-db:postgresql-2",replica="master",server="10.xxx.xxx.xxx",standby="10.xxx.xxx.xxx"} 0

Additional info

It looks like another fallout from #13602 where the db label was once postgres is now changed:

            default:
-                   if _, err := dbname.WriteString("postgres"); err != nil {
+                   database, err := p.GetConnectDatabase(tagAddress)
+                   if err != nil {
+                           return err
+                   }
+                   if _, err := dbname.WriteString(database); err != nil {
                            return err
                    }
@hloeung hloeung added the bug unexpected problem or unintended behavior label Sep 26, 2023
powersj added a commit to powersj/telegraf that referenced this issue Sep 27, 2023
The database name should either be postgres or the value from a
correctly parsed dsn.

fixes: influxdata#14007
@powersj
Copy link
Contributor

powersj commented Sep 27, 2023

This whole logic is a bit of a mess :\ I think I have restored the behavior given that you are setting the output address, we should instead return postgres. I have updated the test case and put up #14007.

@powersj
Copy link
Contributor

powersj commented Sep 28, 2023

@hloeung hoping to land this with Monday's v1.27.2. Would you have time to confirm the new diff restores the previous functionality?

Thanks!

@hloeung
Copy link
Author

hloeung commented Sep 28, 2023

Unfortunately, I'm going away this long weekend without access to work stuff, public holiday here in AU. Happy to test it Tuesday when I return, if you can wait?

Thanks for your time.

@powersj
Copy link
Contributor

powersj commented Sep 28, 2023

No worries, we can grab it in the next point release. Enjoy your holiday!

@hloeung
Copy link
Author

hloeung commented Oct 9, 2023

@powersj, tested package looks good to me:

$ curl -s localhost:9103/metrics | grep '^postgresql_lag'
postgresql_lag_bytes{db="10.xxx.xxx.xxx",host="prod-ua-db:postgresql-6",juju_application="postgresql",juju_model="prod-ua-db",juju_model_uuid="036db83c-6077-4ae0-8dcc-ebeb2bdda852",juju_unit="postgresql-6",replica="master",server="10.xxx.xxx.xxx",standby="10.xxx.xxx.xxx"} 8208
$ sudo dpkg -i telegraf_1.29.0~529afb53-0_amd64.deb
(Reading database ... 103685 files and directories currently installed.)
Preparing to unpack telegraf_1.29.0~529afb53-0_amd64.deb ...
Unpacking telegraf (1.29.0-0) over (1.28.1+git-868f75eee~202309220113~ubuntu23.04.1) ...
Setting up telegraf (1.29.0-0) ...
$ curl -s localhost:9103/metrics | grep '^postgresql_lag'
postgresql_lag_bytes{db="postgres",host="prod-ua-db:postgresql-6",juju_application="postgresql",juju_model="prod-ua-db",juju_model_uuid="036db83c-6077-4ae0-8dcc-ebeb2bdda852",juju_unit="postgresql-6",replica="master",server="10.xxx.xxx.xxx",standby="10.xxx.xxx.xxx"} 17872

So db="postgres" from the above. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants