Commit 7b03cba0 authored by Doug Barrett's avatar Doug Barrett
Browse files

fix(v2/postgres): Address review feedback 

- Rename MinConns to MaxIdleConns to match the database/sql API it
  maps to, avoiding confusion about the field's semantics
- Add db.name span attribute per OpenTelemetry database semantic
  conventions, improving trace filtering for multi-database services
- Add Name() tests for default and custom values
parent a6b3cc21

v2/postgres/client.go

+8 −8
Original line number Diff line number Diff line
@@ -30,9 +30,9 @@ type Config struct { 
	// Maps to [sql.DB.SetMaxOpenConns]. Zero means unlimited.

	MaxConns int



	// MinConns is the minimum number of idle connections maintained in the

	// pool. Maps to [sql.DB.SetMaxIdleConns]. Zero uses the default (2).

	MinConns int

	// MaxIdleConns is the maximum number of idle connections in the pool.

	// Maps to [sql.DB.SetMaxIdleConns]. Zero uses the default (2).

	MaxIdleConns int



	// ConnMaxLifetime is the maximum duration a connection may be reused.

	// Maps to [sql.DB.SetConnMaxLifetime]. Zero means connections are not
@@ -58,7 +58,7 @@ type Client struct { 
	connCfg         *pgx.ConnConfig

	db              *sql.DB

	maxConns        int

	minConns        int

	maxIdleConns    int

	connMaxLifetime time.Duration

	connMaxIdleTime time.Duration

}
@@ -82,7 +82,7 @@ func NewWithConfig(cfg *Config) (*Client, error) { 
	}



	if cfg.Tracer != nil {

		connCfg.Tracer = &queryTracer{tracer: cfg.Tracer}

		connCfg.Tracer = &queryTracer{tracer: cfg.Tracer, dbName: connCfg.Database}

	}



	name := cfg.Name
@@ -94,7 +94,7 @@ func NewWithConfig(cfg *Config) (*Client, error) { 
		name:            name,

		connCfg:         connCfg,

		maxConns:        cfg.MaxConns,

		minConns:        cfg.MinConns,

		maxIdleConns:    cfg.MaxIdleConns,

		connMaxLifetime: cfg.ConnMaxLifetime,

		connMaxIdleTime: cfg.ConnMaxIdleTime,

	}, nil
@@ -126,8 +126,8 @@ func (c *Client) Start(ctx context.Context) error { 
	if c.maxConns > 0 {

		db.SetMaxOpenConns(c.maxConns)

	}

	if c.minConns > 0 {

		db.SetMaxIdleConns(c.minConns)

	if c.maxIdleConns > 0 {

		db.SetMaxIdleConns(c.maxIdleConns)

	}

	if c.connMaxLifetime > 0 {

		db.SetConnMaxLifetime(c.connMaxLifetime)

v2/postgres/client_test.go

+16 −0
Original line number Diff line number Diff line
@@ -47,6 +47,21 @@ func TestDB_NilBeforeStart(t *testing.T) { 
	assert.Nil(t, client.DB())

}



func TestName_Default(t *testing.T) {

	client, err := postgres.New("postgres://user:pass@localhost:5432/mydb")

	require.NoError(t, err)

	assert.Equal(t, "postgres", client.Name())

}



func TestName_Custom(t *testing.T) {

	client, err := postgres.NewWithConfig(&postgres.Config{

		DSN:  "postgres://user:pass@localhost:5432/mydb",

		Name: "analytics",

	})

	require.NoError(t, err)

	assert.Equal(t, "analytics", client.Name())

}



func TestShutdown_BeforeStart(t *testing.T) {

	client, err := postgres.New("postgres://user:pass@localhost:5432/mydb")

	require.NoError(t, err)
@@ -114,6 +129,7 @@ func TestQueryTracer_Attributes(t *testing.T) { 
	require.Len(t, spans, 1)

	assert.Equal(t, "postgresql", spans[0].Attributes["db.system"])

	assert.Equal(t, sql, spans[0].Attributes["db.statement"])

	assert.Equal(t, "mydb", spans[0].Attributes["db.name"])

}



func TestQueryTracer_RecordsError(t *testing.T) {

v2/postgres/tracer.go

+5 −0
Original line number Diff line number Diff line
@@ -12,12 +12,14 @@ import ( 
const (

	attrDBSystem    = "db.system"

	attrDBStatement = "db.statement"

	attrDBName      = "db.name"

)



// queryTracer implements [pgx.QueryTracer] and creates an OpenTelemetry span

// for every query, QueryRow, and Exec call when a Tracer is configured.

type queryTracer struct {

	tracer *trace.Tracer

	dbName string

}



// TraceQueryStart starts a span named after the SQL verb (e.g. "db SELECT")
@@ -26,6 +28,9 @@ func (t *queryTracer) TraceQueryStart(ctx context.Context, _ *pgx.Conn, data pgx 
	ctx, span := t.tracer.Start(ctx, spanName(data.SQL))

	span.SetAttribute(attrDBSystem, "postgresql")

	span.SetAttribute(attrDBStatement, data.SQL)

	if t.dbName != "" {

		span.SetAttribute(attrDBName, t.dbName)

	}

	return ctx

}