Enhance postgres service to understand pgbouncer data types

[johnatannvmd]

Required for all PRs:

Issue #6021

• Signed CLA.
• Associated README.md updated.
• Has appropriate unit tests.

Additional info:

In the company where I currently work this custom solution works for at least 6 months without a hitch.

This fix can solve #5999 issue after migration to the postgresql_extensible plugin.

[danielnelson]

@james-lawrence Would you have time to review this pull request? My concern here is that we might change the output of the regular postgres plugins.

[james-lawrence]

@johnatannvmd @danielnelson sure, its been awhile though I'm a little rusty/out of date on pgx internals. personally I'd avoid combining the configurations entirely. because then we may run into subtle implementation details of info.RegisterDataType there is also very little benefit to doing so code/maintenance wise. I believe this is what has daniel concerned, and I agree, I'm willing to be convienced though, but it'll require explicit code samples from pgx and tests in this repo to convince me.

I think the current code in master is pretty close to what I'd do regardless. with some minor improvements. This doesn't let you use postgresql extensible against pgbouncer directly. but i think that is a good thing. less complexity.

what I'd do:

• change p.IsPgBounder to pgx.ConnConfig, and check for nil/zero if necessary, depends on the mechanics of merge. means other postgresql wire protocols will work here without issue.
• move the pgx.ConnConfig into the pgbouncer package (its pgbouncer specific)
• update connection stuff here to use the more recent features in pgx. (stdlib.OpenDB)

// plugins/inputs/postgresql/service.go

// Start starts the ServiceInput's service, whatever that may be
func (p *Service) Start(telegraf.Accumulator) (err error) {
	const localhost = "host=localhost sslmode=disable"
        var (
             connConfig pgx.ConnConfig
        )

	if p.Address == "" || p.Address == "localhost" {
		p.Address = localhost
        }

      	if connConfig, err = pgx.ParseConnectionString(p.Address); err != nil {
		return err
	}

        p.DB = stdlib.OpenDB(connConfig.Merge(t.ConnConfig))
      
        return nil
}

// plugins/inputs/pgbouncer/pgbouncer.go
func connConfig() pgx.ConnConfig {
       return pgx.ConnConfig{
          // ...
       }
}

[johnatannvmd]

but it'll require explicit code samples from pgx and tests in this repo to convince me.

I totally agree with you. And I will prepare code samples from gpx, Postgres and pgbounce about return types and how the response is prepared.

The reason for these changes is to make possible to query different information from pgbouncer, not only SHOW POOLS and SHOW STATS. But I totally understand what you want:

move the pgx.ConnConfig into the pgbouncer package (its pgbouncer specific)
is a good point for less complexity and division of responsibility.

[james-lawrence]

The reason for these changes is to make possible to query different information from pgbouncer, not only SHOW POOLS and SHOW STATS

I understood the goals of the PR. =)

if you follow the suggestions I offered I believe pgbouncer can actually become a simple configuration setup over postgres_extensible. if we so desire.