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.

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(promtail): prevent panic due to duplicate metric registration after reloaded #10798

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -49,6 +49,7 @@
##### Fixes

* [10631](https://github.com/grafana/loki/pull/10631) **thampiotr**: Fix race condition in cleaning up metrics when stopping to tail files.
* [10798](https://github.com/grafana/loki/pull/10798) **hainenber**: Fix agent panicking after reloaded due to duplicate metric collector registration.

#### LogCLI

Expand Down
6 changes: 6 additions & 0 deletions clients/pkg/promtail/targets/lokipush/pushtarget.go
Expand Up @@ -82,6 +82,12 @@ func (t *PushTarget) run() error {
serverCfg := &t.config.Server
serverCfg.Log = util_log.InitLogger(serverCfg, prometheus.NewRegistry(), true, false)

// Set new registry for upcoming metric server
// If not, it'll likely panic when the tool gets reloaded.
if t.config.Server.Registerer == nil {
t.config.Server.Registerer = prometheus.NewRegistry()
}

srv, err := server.New(t.config.Server)
if err != nil {
return err
Expand Down
50 changes: 43 additions & 7 deletions clients/pkg/promtail/wal/watcher_metrics.go
@@ -1,6 +1,10 @@
package wal

import "github.com/prometheus/client_golang/prometheus"
import (
"errors"

"github.com/prometheus/client_golang/prometheus"
)

type WatcherMetrics struct {
recordsRead *prometheus.CounterVec
Expand Down Expand Up @@ -69,13 +73,45 @@ func NewWatcherMetrics(reg prometheus.Registerer) *WatcherMetrics {
),
}

// Collectors will be re-registered to registry if it's got reloaded
// Reuse the old collectors instead of panicking out.
if reg != nil {
reg.MustRegister(m.recordsRead)
reg.MustRegister(m.recordDecodeFails)
reg.MustRegister(m.droppedWriteNotifications)
reg.MustRegister(m.segmentRead)
reg.MustRegister(m.currentSegment)
reg.MustRegister(m.watchersRunning)
if err := reg.Register(m.recordsRead); err != nil {
are := &prometheus.AlreadyRegisteredError{}
if errors.As(err, are) {
m.recordsRead = are.ExistingCollector.(*prometheus.CounterVec)
}
}
if err := reg.Register(m.recordDecodeFails); err != nil {
are := &prometheus.AlreadyRegisteredError{}
if errors.As(err, are) {
m.recordDecodeFails = are.ExistingCollector.(*prometheus.CounterVec)
}
}
if err := reg.Register(m.droppedWriteNotifications); err != nil {
are := &prometheus.AlreadyRegisteredError{}
if errors.As(err, are) {
m.droppedWriteNotifications = are.ExistingCollector.(*prometheus.CounterVec)
}
}
if err := reg.Register(m.segmentRead); err != nil {
are := &prometheus.AlreadyRegisteredError{}
if errors.As(err, are) {
m.segmentRead = are.ExistingCollector.(*prometheus.CounterVec)
}
}
if err := reg.Register(m.currentSegment); err != nil {
are := &prometheus.AlreadyRegisteredError{}
if errors.As(err, are) {
m.currentSegment = are.ExistingCollector.(*prometheus.GaugeVec)
}
}
if err := reg.Register(m.watchersRunning); err != nil {
are := &prometheus.AlreadyRegisteredError{}
if errors.As(err, are) {
m.watchersRunning = are.ExistingCollector.(*prometheus.GaugeVec)
}
}
}

return m
Expand Down