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

Cherry-pick #4098 to 5.3: Fix race in go-metrics adapater #4100

Merged
merged 1 commit into from
Apr 24, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ https://github.com/elastic/beats/compare/v5.3.0...v5.3.1[View commits]
*Affecting all Beats*

- Fix panic when testing regex-AST to match against date patterns. {issue}3889[3889]
- Fix panic due to race condition in kafka output. {pull}4098[4098]

*Filebeat*

Expand Down
25 changes: 23 additions & 2 deletions libbeat/monitoring/adapter/go-metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package adapter
import (
"fmt"
"reflect"
"sync"

"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/libbeat/monitoring"
Expand All @@ -21,6 +22,8 @@ import (
// It's recommended to not mix go-metrics with other metrics types
// in the same namespace.
type GoMetricsRegistry struct {
mutex sync.Mutex

reg *monitoring.Registry
filters *metricFilters

Expand Down Expand Up @@ -80,6 +83,12 @@ func (r *GoMetricsRegistry) find(name string) interface{} {
// It's recommended to not mix go-metrics with other metrics types in one
// namespace.
func (r *GoMetricsRegistry) Get(name string) interface{} {
r.mutex.Lock()
defer r.mutex.Unlock()
return r.get(name)
}

func (r *GoMetricsRegistry) get(name string) interface{} {
m := r.find(name)
if m == nil {
return r.shadow.Get(name)
Expand All @@ -95,7 +104,10 @@ func (r *GoMetricsRegistry) Get(name string) interface{} {
// GetOrRegister retries an existing metric via `Get` or registers a new one
// if the metric is unknown. For lazy instantiation metric can be a function.
func (r *GoMetricsRegistry) GetOrRegister(name string, metric interface{}) interface{} {
v := r.Get(name)
r.mutex.Lock()
defer r.mutex.Unlock()

v := r.get(name)
if v != nil {
return v
}
Expand All @@ -106,7 +118,10 @@ func (r *GoMetricsRegistry) GetOrRegister(name string, metric interface{}) inter
// Register adds a new metric.
// An error is returned if the metric is already known.
func (r *GoMetricsRegistry) Register(name string, metric interface{}) error {
if r.Get(name) != nil {
r.mutex.Lock()
defer r.mutex.Unlock()

if r.get(name) != nil {
return fmt.Errorf("metric '%v' already registered", name)
}

Expand Down Expand Up @@ -139,13 +154,19 @@ func (r *GoMetricsRegistry) RunHealthchecks() {}

// Unregister removes a metric.
func (r *GoMetricsRegistry) Unregister(name string) {
r.mutex.Lock()
defer r.mutex.Unlock()

st := r.rmState(name)
r.reg.Remove(st.name)
r.shadow.Unregister(name)
}

// UnregisterAll calls `Clear` on the underlying monitoring.Registry
func (r *GoMetricsRegistry) UnregisterAll() {
r.mutex.Lock()
defer r.mutex.Unlock()

r.shadow.UnregisterAll()
err := r.reg.Clear()
if err != nil {
Expand Down