From 354953accb3ada4ce88da2128616cd9fbf723220 Mon Sep 17 00:00:00 2001 From: Alexander Knipping Date: Tue, 12 May 2026 08:09:47 +0200 Subject: [PATCH 1/2] fix(stats): allow WithStatsAttributes to override library defaults Library defaults were appended after user attrs in recordStats, so any override for db.client.connection.pool.name supplied via WithStatsAttributes was silently dropped by attribute.NewSet's last-value-wins dedup. Seed both defaults (db.system.name and db.client.connection.pool.name) into statsOptions before options are applied so user attrs land at the end of the slice and win on key collision. Add a regression test using OTel's manual reader that verifies user overrides take effect while non-overridden defaults are preserved. --- go.mod | 1 + meter.go | 16 ++++----- meter_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 meter_test.go diff --git a/go.mod b/go.mod index 4cfe0eb..a39b172 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( go.opentelemetry.io/otel v1.43.0 go.opentelemetry.io/otel/metric v1.43.0 go.opentelemetry.io/otel/sdk v1.43.0 + go.opentelemetry.io/otel/sdk/metric v1.43.0 go.opentelemetry.io/otel/trace v1.43.0 ) diff --git a/meter.go b/meter.go index 6f90e26..c091c1b 100644 --- a/meter.go +++ b/meter.go @@ -36,12 +36,20 @@ const ( // RecordStats records database statistics for provided pgxpool.Pool at a default 1 second interval // unless otherwise specified by the WithMinimumReadDBStatsInterval StatsOption. +// +// Attributes provided via WithStatsAttributes override the library-supplied defaults +// (db.system.name, db.client.connection.pool.name) on key collision, since +// attribute.NewSet applies last-value-wins semantics over the resulting slice. func RecordStats(db PoolStats, opts ...StatsOption) error { + connCfg := db.Config().ConnConfig + poolName := fmt.Sprintf("%s:%d/%s", connCfg.Host, connCfg.Port, connCfg.Database) + o := statsOptions{ meterProvider: otel.GetMeterProvider(), minimumReadDBStatsInterval: defaultMinimumReadDBStatsInterval, defaultAttributes: []attribute.KeyValue{ semconv.DBSystemNamePostgreSQL, + semconv.DBClientConnectionPoolName(poolName), }, } @@ -93,12 +101,6 @@ func recordStats( lock sync.Mutex ) - serverAddress := semconv.ServerAddress(db.Config().ConnConfig.Host) - serverPort := semconv.ServerPort(int(db.Config().ConnConfig.Port)) - dbNamespace := semconv.DBNamespace(db.Config().ConnConfig.Database) - poolName := fmt.Sprintf("%s:%d/%s", serverAddress.Value.AsString(), serverPort.Value.AsInt64(), dbNamespace.Value.AsString()) - dbClientConnectionPoolName := semconv.DBClientConnectionPoolName(poolName) - lock.Lock() defer lock.Unlock() @@ -195,8 +197,6 @@ func recordStats( return fmt.Errorf("failed to create asynchronous metric: %s with error: %w", pgxPoolEmptyAcquireWaitTime, err) } - attrs = append(attrs, dbClientConnectionPoolName) - observeOptions = []metric.ObserveOption{ metric.WithAttributeSet(attribute.NewSet(attrs...)), } diff --git a/meter_test.go b/meter_test.go new file mode 100644 index 0000000..b9c08fb --- /dev/null +++ b/meter_test.go @@ -0,0 +1,99 @@ +package otelpgx + +import ( + "context" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "go.opentelemetry.io/otel/attribute" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + semconv "go.opentelemetry.io/otel/semconv/v1.40.0" +) + +func TestRecordStats_UserAttrsOverrideLibraryDefaults(t *testing.T) { + ctx := context.Background() + + cfg, err := pgxpool.ParseConfig("postgres://user@127.0.0.1:5432/somedb") + if err != nil { + t.Fatalf("pgxpool.ParseConfig: %v", err) + } + + pool, err := pgxpool.NewWithConfig(ctx, cfg) + if err != nil { + t.Fatalf("pgxpool.NewWithConfig: %v", err) + } + t.Cleanup(pool.Close) + + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + + const overridePoolName = "my-logical-pool" + + err = RecordStats(pool, + WithStatsMeterProvider(provider), + WithStatsAttributes(semconv.DBClientConnectionPoolName(overridePoolName)), + ) + if err != nil { + t.Fatalf("RecordStats: %v", err) + } + + var rm metricdata.ResourceMetrics + if err := reader.Collect(ctx, &rm); err != nil { + t.Fatalf("reader.Collect: %v", err) + } + + if len(rm.ScopeMetrics) == 0 { + t.Fatal("expected at least one scope metric to be collected") + } + + var checked int + for _, sm := range rm.ScopeMetrics { + for _, m := range sm.Metrics { + for _, attrs := range dataPointAttributes(m.Data) { + poolNameVal, ok := attrs.Value(semconv.DBClientConnectionPoolNameKey) + if !ok { + t.Errorf("metric %q: missing %s attribute", m.Name, semconv.DBClientConnectionPoolNameKey) + continue + } + if poolNameVal.AsString() != overridePoolName { + t.Errorf("metric %q: %s = %q, want user-supplied override %q", + m.Name, semconv.DBClientConnectionPoolNameKey, poolNameVal.AsString(), overridePoolName) + } + + // db.system.name should still carry the library default since the user did not override it. + dbSystemVal, ok := attrs.Value(semconv.DBSystemNameKey) + if !ok { + t.Errorf("metric %q: missing %s attribute", m.Name, semconv.DBSystemNameKey) + } else if dbSystemVal.AsString() != semconv.DBSystemNamePostgreSQL.Value.AsString() { + t.Errorf("metric %q: %s = %q, want library default %q", + m.Name, semconv.DBSystemNameKey, dbSystemVal.AsString(), semconv.DBSystemNamePostgreSQL.Value.AsString()) + } + + checked++ + } + } + } + + if checked == 0 { + t.Fatal("collection produced no data points to verify attributes on") + } +} + +// dataPointAttributes returns the attribute set attached to every data point +// in the given aggregation, across the metric shapes that recordStats emits +// (Sum[int64] for counters/up-down counters, Gauge[int64] for the max gauge). +func dataPointAttributes(data metricdata.Aggregation) []attribute.Set { + var sets []attribute.Set + switch d := data.(type) { + case metricdata.Sum[int64]: + for _, dp := range d.DataPoints { + sets = append(sets, dp.Attributes) + } + case metricdata.Gauge[int64]: + for _, dp := range d.DataPoints { + sets = append(sets, dp.Attributes) + } + } + return sets +} From 6212d3981a6fff8b1cd2a74a360bf4aa78eb7c1b Mon Sep 17 00:00:00 2001 From: Alexander Knipping Date: Thu, 21 May 2026 10:23:31 +0200 Subject: [PATCH 2/2] fix: Update meter.go comment Co-authored-by: Leo Antunes --- meter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meter.go b/meter.go index c091c1b..11ebd22 100644 --- a/meter.go +++ b/meter.go @@ -34,7 +34,7 @@ const ( pgxPoolEmptyAcquireWaitTime = "pgxpool.empty_acquire_wait_time" ) -// RecordStats records database statistics for provided pgxpool.Pool at a default 1 second interval +// RecordStats records database statistics for provided [pgxpool.Pool] at a default 1 second interval // unless otherwise specified by the WithMinimumReadDBStatsInterval StatsOption. // // Attributes provided via WithStatsAttributes override the library-supplied defaults