Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the postgres prom GC metrics to respect enable prom option #197

Merged
merged 1 commit into from
Oct 19, 2021
Merged

fix the postgres prom GC metrics to respect enable prom option #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions internal/datastore/postgres/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ func GCMaxOperationTime(time time.Duration) Option {

// EnablePrometheusStats enables Prometheus metrics provided by the Postgres
// clients being used by the datastore.
//
// Prometheus metrics are disable by default.
func EnablePrometheusStats() Option {
return func(po *postgresOptions) {
po.enablePrometheusStats = true
Expand All @@ -183,6 +185,8 @@ func EnablePrometheusStats() Option {

// EnableTracing enables trace-level logging for the Postgres clients being
// used by the datastore.
//
// Tracing is disabled by default.
func EnableTracing() Option {
return func(po *postgresOptions) {
po.logger = &tracingLogger{}
Expand Down
33 changes: 25 additions & 8 deletions internal/datastore/postgres/postgres.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,30 @@ const (

var (
gcDurationHistogram = prometheus.NewHistogram(prometheus.HistogramOpts{
Name: "postgres_gc_duration",
Help: "postgres garbage collection duration distribution in seconds.",
Buckets: []float64{0.01, 0.1, 0.5, 1, 5, 10, 25, 60, 120},
Namespace: "spicedb",
Subsystem: "datastore",
Name: "postgres_gc_duration",
Help: "postgres garbage collection duration distribution in seconds.",
Buckets: []float64{0.01, 0.1, 0.5, 1, 5, 10, 25, 60, 120},
})

gcRelationshipsClearedGauge = prometheus.NewGauge(prometheus.GaugeOpts{
Name: "postgres_relationships_cleared",
Help: "number of relationships cleared by postgres garbage collection.",
Namespace: "spicedb",
Subsystem: "datastore",
Name: "postgres_relationships_cleared",
Help: "number of relationships cleared by postgres garbage collection.",
})

gcTransactionsClearedGauge = prometheus.NewGauge(prometheus.GaugeOpts{
Name: "postgres_transactions_cleared",
Help: "number of transactions cleared by postgres garbage collection.",
Namespace: "spicedb",
Subsystem: "datastore",
Name: "postgres_transactions_cleared",
Help: "number of transactions cleared by postgres garbage collection.",
})
)

func init() {
dbsql.Register(tracingDriverName, sqlmw.Driver(stdlib.GetDefaultDriver(), new(traceInterceptor)))
prometheus.MustRegister(gcDurationHistogram, gcRelationshipsClearedGauge, gcTransactionsClearedGauge)
}

var (
Expand Down Expand Up @@ -144,6 +149,18 @@ func NewPostgresDatastore(
if err != nil {
return nil, fmt.Errorf(errUnableToInstantiate, err)
}
err = prometheus.Register(gcDurationHistogram)
if err != nil {
return nil, fmt.Errorf(errUnableToInstantiate, err)
}
err = prometheus.Register(gcRelationshipsClearedGauge)
if err != nil {
return nil, fmt.Errorf(errUnableToInstantiate, err)
}
err = prometheus.Register(gcTransactionsClearedGauge)
if err != nil {
return nil, fmt.Errorf(errUnableToInstantiate, err)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to make this a runtime error or just use promauto?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know... it's not clear to me when prom registering can fail just based on the API. It's going to bubble up to a fatal in our binary anyway but probably with better wrapping.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this is fine, given it'll bubble up almost immediately if an error is returned here

}

gcCtx, cancelGc := context.WithCancel(context.Background())
Expand Down