Skip to content

Commit

Permalink
Fix metric keys
Browse files Browse the repository at this point in the history
Calling methods from Metrics in go-metrics has a nasty side-effect
where the implementation is modifying the input "key" slice passed to it.
The implementation inserts the service name (and metric type for M3) to the
beginning of the "key" slice.

This is causing confusing metrics names when emitting to multiple
implementations of Metrics, since each time the "key" is passed to
a method of Metrics, the key changes. Since there is always an in-memory
sink initialized, if any metrics sink configuration is specified, the key
is different when passed to different sinks.

Mitigating this issue in go-metrics for now by always copying the "key" slice
before passing it to go-metrics.

Signed-off-by: Ryan Turner <turner@uber.com>
  • Loading branch information
rturner3 authored and R. Tyler Julian committed Nov 12, 2019
1 parent 7778da0 commit 985b00c
Showing 1 changed file with 28 additions and 9 deletions.
37 changes: 28 additions & 9 deletions pkg/common/telemetry/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,62 +100,81 @@ func (m *MetricsImpl) ListenAndServe(ctx context.Context) error {

func (m *MetricsImpl) SetGauge(key []string, val float32) {
for _, s := range m.metricsSinks {
s.SetGauge(key, val)
keyCopy := copyKey(key)
s.SetGauge(keyCopy, val)
}
}

// SetGaugeWithLabels delegates to embedded metrics, sanitizing labels
func (m *MetricsImpl) SetGaugeWithLabels(key []string, val float32, labels []Label) {
sanitizedLabels := SanitizeLabels(labels)
for _, s := range m.metricsSinks {
s.SetGaugeWithLabels(key, val, sanitizedLabels)
keyCopy := copyKey(key)
s.SetGaugeWithLabels(keyCopy, val, sanitizedLabels)
}
}

func (m *MetricsImpl) EmitKey(key []string, val float32) {
for _, s := range m.metricsSinks {
s.EmitKey(key, val)
keyCopy := copyKey(key)
s.EmitKey(keyCopy, val)
}
}

func (m *MetricsImpl) IncrCounter(key []string, val float32) {
for _, s := range m.metricsSinks {
s.IncrCounter(key, val)
keyCopy := copyKey(key)
s.IncrCounter(keyCopy, val)
}
}

// IncrCounterWithLabels delegates to embedded metrics, sanitizing labels
func (m *MetricsImpl) IncrCounterWithLabels(key []string, val float32, labels []Label) {
sanitizedLabels := SanitizeLabels(labels)
for _, s := range m.metricsSinks {
s.IncrCounterWithLabels(key, val, sanitizedLabels)
keyCopy := copyKey(key)
s.IncrCounterWithLabels(keyCopy, val, sanitizedLabels)
}
}

func (m *MetricsImpl) AddSample(key []string, val float32) {
for _, s := range m.metricsSinks {
s.AddSample(key, val)
keyCopy := copyKey(key)
s.AddSample(keyCopy, val)
}
}

// AddSampleWithLabels delegates to embedded metrics, sanitizing labels
func (m *MetricsImpl) AddSampleWithLabels(key []string, val float32, labels []Label) {
sanitizedLabels := SanitizeLabels(labels)
for _, s := range m.metricsSinks {
s.AddSampleWithLabels(key, val, sanitizedLabels)
keyCopy := copyKey(key)
s.AddSampleWithLabels(keyCopy, val, sanitizedLabels)
}
}

func (m *MetricsImpl) MeasureSince(key []string, start time.Time) {
for _, s := range m.metricsSinks {
s.MeasureSince(key, start)
keyCopy := copyKey(key)
s.MeasureSince(keyCopy, start)
}
}

// MeasureSinceWithLabels delegates to embedded metrics, sanitizing labels
func (m *MetricsImpl) MeasureSinceWithLabels(key []string, start time.Time, labels []Label) {
sanitizedLabels := SanitizeLabels(labels)
for _, s := range m.metricsSinks {
s.MeasureSinceWithLabels(key, start, sanitizedLabels)
keyCopy := copyKey(key)
s.MeasureSinceWithLabels(keyCopy, start, sanitizedLabels)
}
}

// TODO: [rturner3] Remove this if/when go-metrics is fixed to not modify its input "key" slice
func copyKey(key []string) []string {
var newKey []string
for _, e := range key {
newKey = append(newKey, e)
}

return newKey
}

0 comments on commit 985b00c

Please sign in to comment.