Use the Stater interface instead of pgxpool.Pool structure

[inebritov]

This pull request refactors the cmackenzie1/pgxpool-prometheus library to use the Stater interface instead of directly using the pgxpool.Pool struct. This change aims to improve the flexibility and testability of the code by allowing different implementations of the Stat method. This change promotes better abstraction and separation of concerns, making the codebase cleaner and more modular. The usage is not changed.

[cmackenzie1]

Hey @inebritov, thanks for the PR! Would you be able to provide an example of how you would use this new Stater interface that the current implementation doesn't support?

[inebritov]

Hi!

Using interfaces in Go is a common practice as it simplifies injecting different implementations. For example, when writing a test for the PgxPoolStatsCollector, you typically need to instantiate the entire connection pool. However, if you only require the Stat method, you can mock just the Stat method for the test, thereby covering the entire implementation.

Please feel free to ask for any clarifications or modifications to this PR. I will do my best to contribute effectively!

[cmackenzie1]

@inebritov please provide an example where and how one would use this interface. I do not understand why one would have a stat collector on a mocked pool for tests.

[inebritov]

Sure. I am looking to integrate the library into an existing project. The project has the following interface:

type Pool interface {
	Exec(ctx context.Context, sql string, arguments ...interface{}) (pgconn.CommandTag, error)
	Close()
	Query(ctx context.Context, sql string, args ...interface{}) (pgx.Rows, error)
	QueryRow(ctx context.Context, sql string, args ...interface{}) pgx.Row
	BeginTx(ctx context.Context, sql pgx.TxOptions) (pgx.Tx, error)
}

And the following routine for application initialization:

func (s *DB) Init(ctx context.Context, cfgInt interface{}) error {
//...
	var pool Pool
	pool, err = pgxpool.ConnectConfig(ctx, poolCFG)
	if err != nil {
		return fmt.Errorf("could not initialize pgx pool: %w", err)
	}
	s.Pool = pool // type of s.Pool: Pool interface

	return nil
}

Creating the pool in this context means that NewPgxPoolStatsCollector should be called:

s.Pool = NewPgxPoolStatsCollector(pool)

At first glance, it seems easier to change the pool variable type to *pgxpool.Pool, but considering integration tests, the entire connection might be initialized (even though it is lazy).

When using a mock library, it typically generates dummy structures for each method:

type Pool struct {
	mock.Mock
}

func (p *Pool) Exec(ctx context.Context, sql string, arguments ...interface{}) (pgconn.CommandTag, error) {
	args := p.Called(sql, arguments)
	return args.Get(0).(pgconn.CommandTag), args.Error(1)
}

func (p *Pool) Close(_ context.Context) {
	args := c.Called()
}

//...

Integration tests in this scenario will call the App.Init method without initializing the database connection. However, there is a significant concern with merging this pull request as it may cause compatibility issues for existing users.

[inebritov]

I see ai-generated label on the PR. BTW, I am not AI 😁

[cmackenzie1]

A few things here, this library targets pgxpool v5, your example is using ConnectConfig, which is a v4 method. Second, this assignment doesn't do what I think you intend for it to do:

s.Pool = NewPgxPoolStatsCollector(pool) 

I am assuming that NewPgxPoolStatsCollector in your example refers to the method provided by this library

pgxpool-prometheus/collector.go

Line 30 in c94a268

func NewPgxPoolStatsCollector(db *pgxpool.Pool, dbName string) *PgxPoolStatsCollector {

which returns a *PgxPoolStatsCollector which is in no way compatible with your Pool interface.

Instead, it should be used like so to register the collector to a Prometheus registry.

pgxpool-prometheus/_example/example.go

Lines 21 to 28 in c94a268

	// Create a pgxpool.Pool
	pool, err := pgxpool.NewWithConfig(context.Background(), config)
	if err != nil {
	panic(err)
	}
	defer pool.Close()
	prometheus.MustRegister(pgxpool_prometheus.NewPgxPoolStatsCollector(pool, "database"))

so in your example, it would look like:

func (s *DB) Init(ctx context.Context, cfgInt interface{}) error {
//...
	var pool Pool
	pool, err = pgxpool.ConnectConfig(ctx, poolCFG)
	if err != nil {
		return fmt.Errorf("could not initialize pgx pool: %w", err)
	}
        
        // register pool with prometheus
        prometheus.MustRegister(pgxpool_prometheus.NewPgxPoolStatsCollector(pool, "database"))
	s.Pool = pool // type of s.Pool: Pool interface

	return nil
}

---

Regarding the ai-generated label, I ran your PR description and comments through https://gptzero.me/ and it classified it as AI Generated. This is just used for classification on my end, it isn't meant to lessen your contribution at all!