From aa7795e0b0283d8eec960aac4c6426848998cdda Mon Sep 17 00:00:00 2001 From: Ben Gentry Date: Sat, 11 May 2024 09:05:03 -0600 Subject: [PATCH 1/4] contrib/jackc/pgx.v5: publish pgxpool stats to statsd client Fixes #2680 --- contrib/jackc/pgx.v5/metrics.go | 67 ++++++++++++++++++++++++++++ contrib/jackc/pgx.v5/option.go | 33 +++++++++++++- contrib/jackc/pgx.v5/option_test.go | 24 ++++++++++ contrib/jackc/pgx.v5/pgx_tracer.go | 2 + contrib/jackc/pgx.v5/pgxpool.go | 12 ++++- contrib/jackc/pgx.v5/pgxpool_test.go | 33 ++++++++++++++ 6 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 contrib/jackc/pgx.v5/metrics.go create mode 100644 contrib/jackc/pgx.v5/option_test.go diff --git a/contrib/jackc/pgx.v5/metrics.go b/contrib/jackc/pgx.v5/metrics.go new file mode 100644 index 0000000000..9b9d29bd65 --- /dev/null +++ b/contrib/jackc/pgx.v5/metrics.go @@ -0,0 +1,67 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2022 Datadog, Inc. + +package pgx + +import ( + "time" + + "github.com/jackc/pgx/v5/pgxpool" + "gopkg.in/DataDog/dd-trace-go.v1/internal" + "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" + "gopkg.in/DataDog/dd-trace-go.v1/internal/log" +) + +const tracerPrefix = "datadog.tracer." + +const ( + AcquireCount = tracerPrefix + "pgx.pool.connections.acquire" + AcquireDuration = tracerPrefix + "pgx.pool.connections.acquire_duration" + AcquiredConns = tracerPrefix + "pgx.pool.connections.acquired_conns" + CanceledAcquireCount = tracerPrefix + "pgx.pool.connections.canceled_acquire" + ConstructingConns = tracerPrefix + "pgx.pool.connections.constructing_conns" + EmptyAcquireCount = tracerPrefix + "pgx.pool.connections.empty_acquire" + IdleConns = tracerPrefix + "pgx.pool.connections.idle_conns" + MaxConns = tracerPrefix + "pgx.pool.connections.max_conns" + TotalConns = tracerPrefix + "pgx.pool.connections.total_conns" + NewConnsCount = tracerPrefix + "pgx.pool.connections.new_conns" + MaxLifetimeDestroyCount = tracerPrefix + "pgx.pool.connections.max_lifetime_destroy" + MaxIdleDestroyCount = tracerPrefix + "pgx.pool.connections.max_idle_destroy" +) + +var interval = 10 * time.Second + +// pollPoolStats calls (*pgxpool).Stats on the pool at a predetermined interval. It pushes the pool Stats off to the statsd client. +func pollPoolStats(statsd internal.StatsdClient, pool *pgxpool.Pool) { + if pool == nil { + log.Debug("No traced pool connection found; cannot pull pool stats.") + return + } + log.Debug("Traced pool connection found: Pool stats will be gathered and sent every %v.", interval) + for range time.NewTicker(interval).C { + log.Debug("Reporting pgxpool.Stat metrics...") + stat := pool.Stat() + statsd.Gauge(AcquireCount, float64(stat.AcquireCount()), []string{}, 1) + statsd.Timing(AcquireDuration, stat.AcquireDuration(), []string{}, 1) + statsd.Gauge(AcquiredConns, float64(stat.AcquiredConns()), []string{}, 1) + statsd.Gauge(CanceledAcquireCount, float64(stat.CanceledAcquireCount()), []string{}, 1) + statsd.Gauge(ConstructingConns, float64(stat.ConstructingConns()), []string{}, 1) + statsd.Gauge(EmptyAcquireCount, float64(stat.EmptyAcquireCount()), []string{}, 1) + statsd.Gauge(IdleConns, float64(stat.IdleConns()), []string{}, 1) + statsd.Gauge(MaxConns, float64(stat.MaxConns()), []string{}, 1) + statsd.Gauge(TotalConns, float64(stat.TotalConns()), []string{}, 1) + statsd.Gauge(NewConnsCount, float64(stat.NewConnsCount()), []string{}, 1) + statsd.Gauge(MaxLifetimeDestroyCount, float64(stat.MaxLifetimeDestroyCount()), []string{}, 1) + statsd.Gauge(MaxIdleDestroyCount, float64(stat.MaxIdleDestroyCount()), []string{}, 1) + } +} + +func statsTags(c *config) []string { + tags := globalconfig.StatsTags() + if c.serviceName != "" { + tags = append(tags, "service:"+c.serviceName) + } + return tags +} diff --git a/contrib/jackc/pgx.v5/option.go b/contrib/jackc/pgx.v5/option.go index 5c88bcae46..fa21f57a18 100644 --- a/contrib/jackc/pgx.v5/option.go +++ b/contrib/jackc/pgx.v5/option.go @@ -5,7 +5,12 @@ package pgx -import "gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema" +import ( + "gopkg.in/DataDog/dd-trace-go.v1/internal" + "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" + "gopkg.in/DataDog/dd-trace-go.v1/internal/log" + "gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema" +) type config struct { serviceName string @@ -14,6 +19,8 @@ type config struct { traceCopyFrom bool tracePrepare bool traceConnect bool + poolStats bool + statsdClient internal.StatsdClient } func defaultConfig() *config { @@ -27,6 +34,21 @@ func defaultConfig() *config { } } +// checkStatsdRequired adds a statsdClient onto the config if poolStats is enabled +// NOTE: For now, the only use-case for a statsdclient is the poolStats feature. If a statsdclient becomes necessary for other items in future work, then this logic should change +func (c *config) checkStatsdRequired() { + if c.poolStats && c.statsdClient == nil { + // contrib/database/pgx's statsdclient should always inherit its address from the tracer's statsdclient via the globalconfig + // destination is not user-configurable + sc, err := internal.NewStatsdClient(globalconfig.DogstatsdAddr(), statsTags(c)) + if err == nil { + c.statsdClient = sc + } else { + log.Warn("Error creating statsd client for database/sql contrib package; Pool stats will be dropped: %v", err) + } + } +} + type Option func(*config) // WithServiceName sets the service name to use for all spans. @@ -81,3 +103,12 @@ func WithTraceConnect(enabled bool) Option { c.traceConnect = enabled } } + +// WithPoolStats enables polling of pgxpool.Stat metrics +// ref: https://pkg.go.dev/github.com/jackc/pgx/v5/pgxpool#Stat +// These metrics are submitted to Datadog and are not billed as custom metrics +func WithPoolStats() Option { + return func(cfg *config) { + cfg.poolStats = true + } +} diff --git a/contrib/jackc/pgx.v5/option_test.go b/contrib/jackc/pgx.v5/option_test.go new file mode 100644 index 0000000000..57741c1aa8 --- /dev/null +++ b/contrib/jackc/pgx.v5/option_test.go @@ -0,0 +1,24 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2022 Datadog, Inc. + +package pgx + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWithPoolStats(t *testing.T) { + t.Run("default off", func(t *testing.T) { + cfg := defaultConfig() + assert.False(t, cfg.poolStats) + }) + t.Run("on", func(t *testing.T) { + cfg := new(config) + WithPoolStats()(cfg) + assert.True(t, cfg.poolStats) + }) +} diff --git a/contrib/jackc/pgx.v5/pgx_tracer.go b/contrib/jackc/pgx.v5/pgx_tracer.go index fc4438c38d..54bc032f8e 100644 --- a/contrib/jackc/pgx.v5/pgx_tracer.go +++ b/contrib/jackc/pgx.v5/pgx_tracer.go @@ -7,6 +7,7 @@ package pgx import ( "context" + "gopkg.in/DataDog/dd-trace-go.v1/ddtrace" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer" @@ -59,6 +60,7 @@ func newPgxTracer(opts ...Option) *pgxTracer { for _, opt := range opts { opt(cfg) } + cfg.checkStatsdRequired() return &pgxTracer{cfg: cfg} } diff --git a/contrib/jackc/pgx.v5/pgxpool.go b/contrib/jackc/pgx.v5/pgxpool.go index f61a162948..095cd3b710 100644 --- a/contrib/jackc/pgx.v5/pgxpool.go +++ b/contrib/jackc/pgx.v5/pgxpool.go @@ -20,6 +20,14 @@ func NewPool(ctx context.Context, connString string, opts ...Option) (*pgxpool.P } func NewPoolWithConfig(ctx context.Context, config *pgxpool.Config, opts ...Option) (*pgxpool.Pool, error) { - config.ConnConfig.Tracer = newPgxTracer(opts...) - return pgxpool.NewWithConfig(ctx, config) + tracer := newPgxTracer(opts...) + config.ConnConfig.Tracer = tracer + pool, err := pgxpool.NewWithConfig(ctx, config) + if err != nil { + return nil, err + } + if tracer.cfg.poolStats { + go pollPoolStats(tracer.cfg.statsdClient, pool) + } + return pool, nil } diff --git a/contrib/jackc/pgx.v5/pgxpool_test.go b/contrib/jackc/pgx.v5/pgxpool_test.go index d8ce0b3bae..5faf9bd6a7 100644 --- a/contrib/jackc/pgx.v5/pgxpool_test.go +++ b/contrib/jackc/pgx.v5/pgxpool_test.go @@ -8,8 +8,11 @@ package pgx import ( "context" "testing" + "time" "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/mocktracer" + "gopkg.in/DataDog/dd-trace-go.v1/internal" + "gopkg.in/DataDog/dd-trace-go.v1/internal/statsdtest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,3 +41,33 @@ func TestPool(t *testing.T) { assert.Len(t, mt.OpenSpans(), 0) assert.Len(t, mt.FinishedSpans(), 5) } + +func TestPoolWithPoolStats(t *testing.T) { + originalInterval := interval + interval = 1 * time.Millisecond + t.Cleanup(func() { + interval = originalInterval + }) + + ctx := context.Background() + statsd := new(statsdtest.TestStatsdClient) + conn, err := NewPool(ctx, postgresDSN, withStatsdClient(statsd), WithPoolStats()) + require.NoError(t, err) + defer conn.Close() + + wantStats := []string{AcquireCount, AcquireDuration, AcquiredConns, CanceledAcquireCount, ConstructingConns, EmptyAcquireCount, IdleConns, MaxConns, TotalConns, NewConnsCount, MaxLifetimeDestroyCount, MaxIdleDestroyCount} + + assert := assert.New(t) + if err := statsd.Wait(assert, len(wantStats), time.Second); err != nil { + t.Fatalf("statsd.Wait(): %v", err) + } + for _, name := range wantStats { + assert.Contains(statsd.CallNames(), name) + } +} + +func withStatsdClient(s internal.StatsdClient) Option { + return func(c *config) { + c.statsdClient = s + } +} From 62d7b9783297f7c123d28a1c1cacaec5f9c50ac3 Mon Sep 17 00:00:00 2001 From: Ben Gentry Date: Sun, 12 May 2024 21:01:30 -0600 Subject: [PATCH 2/4] Fix typo on package name --- contrib/jackc/pgx.v5/option.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/jackc/pgx.v5/option.go b/contrib/jackc/pgx.v5/option.go index fa21f57a18..095b0e6a8e 100644 --- a/contrib/jackc/pgx.v5/option.go +++ b/contrib/jackc/pgx.v5/option.go @@ -38,13 +38,13 @@ func defaultConfig() *config { // NOTE: For now, the only use-case for a statsdclient is the poolStats feature. If a statsdclient becomes necessary for other items in future work, then this logic should change func (c *config) checkStatsdRequired() { if c.poolStats && c.statsdClient == nil { - // contrib/database/pgx's statsdclient should always inherit its address from the tracer's statsdclient via the globalconfig + // contrib/jackc/pgx's statsdclient should always inherit its address from the tracer's statsdclient via the globalconfig // destination is not user-configurable sc, err := internal.NewStatsdClient(globalconfig.DogstatsdAddr(), statsTags(c)) if err == nil { c.statsdClient = sc } else { - log.Warn("Error creating statsd client for database/sql contrib package; Pool stats will be dropped: %v", err) + log.Warn("Error creating statsd client for jackc/pgx contrib package; Pool stats will be dropped: %v", err) } } } From 21a9a206ccc678fd83a6d5484225341599287f4f Mon Sep 17 00:00:00 2001 From: Ben <52796471+bengentree@users.noreply.github.com> Date: Tue, 14 May 2024 15:15:37 -0600 Subject: [PATCH 3/4] Update log statement prefixes Co-authored-by: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> --- contrib/jackc/pgx.v5/metrics.go | 4 ++-- contrib/jackc/pgx.v5/option.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/jackc/pgx.v5/metrics.go b/contrib/jackc/pgx.v5/metrics.go index 9b9d29bd65..d7d800c1d4 100644 --- a/contrib/jackc/pgx.v5/metrics.go +++ b/contrib/jackc/pgx.v5/metrics.go @@ -39,9 +39,9 @@ func pollPoolStats(statsd internal.StatsdClient, pool *pgxpool.Pool) { log.Debug("No traced pool connection found; cannot pull pool stats.") return } - log.Debug("Traced pool connection found: Pool stats will be gathered and sent every %v.", interval) + log.Debug("contrib/jackc/pgx.v5: Traced pool connection found: Pool stats will be gathered and sent every %v.", interval) for range time.NewTicker(interval).C { - log.Debug("Reporting pgxpool.Stat metrics...") + log.Debug("contrib/jackc/pgx.v5: Reporting pgxpool.Stat metrics...") stat := pool.Stat() statsd.Gauge(AcquireCount, float64(stat.AcquireCount()), []string{}, 1) statsd.Timing(AcquireDuration, stat.AcquireDuration(), []string{}, 1) diff --git a/contrib/jackc/pgx.v5/option.go b/contrib/jackc/pgx.v5/option.go index 095b0e6a8e..ec8d52fb43 100644 --- a/contrib/jackc/pgx.v5/option.go +++ b/contrib/jackc/pgx.v5/option.go @@ -44,7 +44,7 @@ func (c *config) checkStatsdRequired() { if err == nil { c.statsdClient = sc } else { - log.Warn("Error creating statsd client for jackc/pgx contrib package; Pool stats will be dropped: %v", err) + log.Warn("contrib/jackc/pgx.v5: Error creating statsd client; Pool stats will be dropped: %v", err) } } } From f09c4519aabd69bb06a524dbc28c2eccb4bd023b Mon Sep 17 00:00:00 2001 From: Ben Gentry Date: Tue, 14 May 2024 15:20:47 -0600 Subject: [PATCH 4/4] Do not publish metrics if statsd client is not set --- contrib/jackc/pgx.v5/metrics.go | 4 ---- contrib/jackc/pgx.v5/pgxpool.go | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/contrib/jackc/pgx.v5/metrics.go b/contrib/jackc/pgx.v5/metrics.go index d7d800c1d4..eb94c50bfc 100644 --- a/contrib/jackc/pgx.v5/metrics.go +++ b/contrib/jackc/pgx.v5/metrics.go @@ -35,10 +35,6 @@ var interval = 10 * time.Second // pollPoolStats calls (*pgxpool).Stats on the pool at a predetermined interval. It pushes the pool Stats off to the statsd client. func pollPoolStats(statsd internal.StatsdClient, pool *pgxpool.Pool) { - if pool == nil { - log.Debug("No traced pool connection found; cannot pull pool stats.") - return - } log.Debug("contrib/jackc/pgx.v5: Traced pool connection found: Pool stats will be gathered and sent every %v.", interval) for range time.NewTicker(interval).C { log.Debug("contrib/jackc/pgx.v5: Reporting pgxpool.Stat metrics...") diff --git a/contrib/jackc/pgx.v5/pgxpool.go b/contrib/jackc/pgx.v5/pgxpool.go index 095cd3b710..f4a8cb7ed0 100644 --- a/contrib/jackc/pgx.v5/pgxpool.go +++ b/contrib/jackc/pgx.v5/pgxpool.go @@ -26,7 +26,7 @@ func NewPoolWithConfig(ctx context.Context, config *pgxpool.Config, opts ...Opti if err != nil { return nil, err } - if tracer.cfg.poolStats { + if tracer.cfg.poolStats && tracer.cfg.statsdClient != nil { go pollPoolStats(tracer.cfg.statsdClient, pool) } return pool, nil