services/horizon: Add Go and process related metrics #3103
Conversation
| // initGoMetrics registers the Go collector provided by prometheus package which | ||
| // includes Go-related metrics. | ||
| func initGoMetrics(app *App) { | ||
| app.prometheusRegistry.MustRegister(prometheus.NewGoCollector()) |
There was a problem hiding this comment.
@jacekn this will render metrics without horizon_ prefix, like: go_memstats_buck_hash_sys_bytes. Is it OK?
There was a problem hiding this comment.
I think no namespace for the general metrics including the processor ones below is what you should go with. See my comment below.
There was a problem hiding this comment.
@bartekn yeah go_memstats_buck_hash_sys_bytes is absolutely fine, that's how most projects I know, including prometheus itself, export go metrics.
services/horizon/internal/init.go
Outdated
| app.prometheusRegistry.MustRegister( | ||
| prometheus.NewProcessCollector( | ||
| prometheus.ProcessCollectorOpts{ | ||
| Namespace: "horizon", |
There was a problem hiding this comment.
I'm not sure you want to use the namespace on the processor collector. I think with the webauth and recoverysigner services we landed on only using namespaces for metrics that are truly uniquely defined by that service. General metrics should otherwise have no namespace so that the same metric that means the same thing has the same name across all services in a deployment.
Maybe this logic doesn't apply to Horizon's use case though.
There was a problem hiding this comment.
Yeah, this makes sense. It's just not clear to me why setting a prefix is allowed here and not in GoCollector.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Adds Go and process related metrics provided by Prometheus package.
Why
Extra metrics can be useful to debug performance issues.
Known limitations