add the Acquire connection tracer

[taran507]

trafficstars

Hello, I found that pgpool does not track the time for connection allocation. I often have to deal with the fact that the request takes a long time to work out precisely because of the long receipt of the connection. I suggest adding AcquireTracer, which will help detect a similar problem

package pgx

// AcquireTracer - traces Acquire.
type AcquireTracer interface {
	// TraceAcquireStart is called at the beginning of Acquire.
	// The returned context is used for the rest of the call and will be passed to the TraceAcquireEnd.
	TraceAcquireStart(ctx context.Context, data TraceAcquireStartData) context.Context
	TraceAcquireEnd(ctx context.Context, data TraceAcquireEndData)
}

type TraceAcquireStartData struct {
	ConnConfig *pgconn.Config
}

type TraceAcquireEndData struct {
	Err error
}

Add the acquireTracer variable to the Pool structure

type Pool struct {
	// ...
	acquireTracer pgx.AcquireTracer
	// ...
}

Declare it in the same way as other tracers, but only when creating pgxpool

func NewWithConfig(ctx context.Context, config *Config) (*Pool, error) {
	// Default values are set in ParseConfig. Enforce initial creation by ParseConfig rather than setting defaults from
	// zero values.
	if !config.createdByParseConfig {
		panic("config must be created by ParseConfig")
	}

	p := &Pool{
		config:                config,
		beforeConnect:         config.BeforeConnect,
		afterConnect:          config.AfterConnect,
		beforeAcquire:         config.BeforeAcquire,
		afterRelease:          config.AfterRelease,
		beforeClose:           config.BeforeClose,
		minConns:              config.MinConns,
		maxConns:              config.MaxConns,
		maxConnLifetime:       config.MaxConnLifetime,
		maxConnLifetimeJitter: config.MaxConnLifetimeJitter,
		maxConnIdleTime:       config.MaxConnIdleTime,
		healthCheckPeriod:     config.HealthCheckPeriod,
		healthCheckChan:       make(chan struct{}, 1),
		closeChan:             make(chan struct{}),
	}

	if t, ok := config.ConnConfig.Tracer.(pgx.AcquireTracer); ok {
		p.acquireTracer = t
	}

And call tracer methods at the beginning and end of connection allocation

func (p *Pool) Acquire(ctx context.Context) (*Conn, error) {
	if p.acquireTracer != nil {
		ctx = p.acquireTracer.TraceAcquireStart(ctx, pgx.TraceAcquireStartData{ConnConfig: p.config.ConnConfig.Config.Copy()})
	}
	for {
		res, err := p.p.Acquire(ctx)
		if err != nil {
			if p.acquireTracer != nil {
				p.acquireTracer.TraceAcquireEnd(ctx, pgx.TraceAcquireEndData{Err: err})
			}
			return nil, err
		}

		cr := res.Value()

		if res.IdleDuration() > time.Second {
			err := cr.conn.Ping(ctx)
			if err != nil {
				res.Destroy()
				continue
			}
		}

		if p.beforeAcquire == nil || p.beforeAcquire(ctx, cr.conn) {
			if p.acquireTracer != nil {
				p.acquireTracer.TraceAcquireEnd(ctx, pgx.TraceAcquireEndData{Err: nil})
			}
			return cr.getConn(p, res), nil
		}

		res.Destroy()
	}
}

[jackc]

The idea seems reasonable. Here's a few thoughts:

• I would expect the type definition for AcquireTracer would be defined in pgxpool not pgx package.
• I'm not sure about TraceAcquireStartData having a *pgconn.Config. If it has anything I would expect it to be a *pgx.ConnConfig. But even that I'm a little unsure of. Most Acquire calls will be getting a connection that is already established.
• Configuration for the pool being set in the connection's config (i.e. config.ConnConfig.Tracer) feels a little funny. I suppose it is the pragmatic choice though.

[ngavinsir]

hey what's the progress on this, I'm really interested with this feature, I would be happy to contribute

[jackc]

@ngavinsir I do not know of anyone working on this.