From 7cd97429b9f587042a162e26c6d1e106b6e4e95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Wed, 2 Sep 2020 14:27:34 +0100 Subject: [PATCH] fix panic on invalid tag names + metrics godocs. (#30) --- runtime/metrics.go | 32 +++++++++-------- runtime/metrics_api.go | 80 ++++++++++++++++++++++++++++++++++++++++++ runtime/runenv_test.go | 27 ++++++++++++++ 3 files changed, 125 insertions(+), 14 deletions(-) diff --git a/runtime/metrics.go b/runtime/metrics.go index 755f5e5..d286c65 100644 --- a/runtime/metrics.go +++ b/runtime/metrics.go @@ -132,15 +132,24 @@ func (m *Metrics) logSinkJSON(filename string) MetricSinkFn { } } -func (m *Metrics) processTags(customtags []string) map[string]string { - mp := make(map[string]string, len(m.tags)+len(customtags)) +func (m *Metrics) computeTags(name string, customtags []string) map[string]string { + ret := make(map[string]string, len(m.tags)+len(customtags)) + // copy global tags. + for k, v := range m.tags { + ret[k] = v + } + + // process custom tags. for _, t := range customtags { kv := strings.Split(t, "=") - - mp[kv[0]] = kv[1] + if len(kv) != 2 { + m.re.SLogger().Warnf("skipping invalid tag for metric; name: %s, tag: %s", name, t) + continue + } + ret[kv[0]] = kv[1] } - return mp + return ret } func (m *Metrics) writeToInfluxDBSink(measurementType string) MetricSinkFn { @@ -149,18 +158,13 @@ func (m *Metrics) writeToInfluxDBSink(measurementType string) MetricSinkFn { for k, v := range metric.Measures { fields[k] = v - vals := strings.Split(metric.Name, ",") - var tags map[string]string - // check if we have custom metric tags + vals := strings.Split(metric.Name, ",") if len(vals) > 1 { - tags = m.processTags(vals[1:]) // ignore first, which is measurement name - - // copy global tags - for k, v := range m.tags { - tags[k] = v - } + // we have custom metric tags; inject global tags + provided tags. + tags = m.computeTags(vals[0], vals[1:]) } else { + // we have no custom metric tags; inject global tags only. tags = m.tags } diff --git a/runtime/metrics_api.go b/runtime/metrics_api.go index 47b12bd..ad3fd49 100644 --- a/runtime/metrics_api.go +++ b/runtime/metrics_api.go @@ -140,38 +140,118 @@ func (m *MetricsApi) SetFrequency(freq time.Duration) { m.freqChangeCh <- freq } +// RecordPoint records a float64 point under the provided metric name + tags. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) RecordPoint(name string, value float64) { m.broadcast(name, Point(value)) } +// Counter creates a measurement of counter type. The returned type is an +// alias of go-metrics' Counter type. Refer to godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) Counter(name string) Counter { return m.reg.GetOrRegister(name, newResettingCounter()).(metrics.Counter) } +// EWMA creates a measurement of exponential-weighted moving average type. +// The returned type is an alias of go-metrics' EWMA type. Refer to godocs +// there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) EWMA(name string, alpha float64) EWMA { return m.reg.GetOrRegister(name, metrics.NewEWMA(alpha)).(metrics.EWMA) } +// Gauge creates a measurement of gauge type (float64). +// The returned type is an alias of go-metrics' GaugeFloat64 type. Refer to +// godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) Gauge(name string) Gauge { return m.reg.GetOrRegister(name, metrics.NewGaugeFloat64()).(metrics.GaugeFloat64) } +// GaugeF creates a measurement of functional gauge type (float64). +// The returned type is an alias of go-metrics' GaugeFloat64 type. Refer to +// godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) GaugeF(name string, f func() float64) Gauge { return m.reg.GetOrRegister(name, metrics.NewFunctionalGaugeFloat64(f)).(metrics.GaugeFloat64) } +// Histogram creates a measurement of histogram type. +// The returned type is an alias of go-metrics' Histogram type. Refer to +// godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) Histogram(name string, s Sample) Histogram { return m.reg.GetOrRegister(name, metrics.NewHistogram(s)).(metrics.Histogram) } +// Meter creates a measurement of meter type. +// The returned type is an alias of go-metrics' Meter type. Refer to +// godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) Meter(name string) Meter { return m.reg.GetOrRegister(name, metrics.NewMeter()).(metrics.Meter) } +// Timer creates a measurement of timer type. +// The returned type is an alias of go-metrics' Timer type. Refer to +// godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) Timer(name string) Timer { return m.reg.GetOrRegister(name, metrics.NewTimer()).(metrics.Timer) } +// ResettingHistogram creates a measurement of histogram type, which cyclically +// resets to zero when its values are harvested. +// +// The returned type is an alias of go-metrics' Histogram type. Refer to +// godocs there for details. +// +// The format of the metric name is a comma-separated list, where the first +// element is the metric name, and optionally, an unbounded list of +// key-value pairs. Example: +// +// requests_received,tag1=value1,tag2=value2,tag3=value3 func (m *MetricsApi) ResettingHistogram(name string) Histogram { return m.reg.GetOrRegister(name, newResettingHistogram()).(Histogram) } diff --git a/runtime/runenv_test.go b/runtime/runenv_test.go index 6717dfe..197569b 100644 --- a/runtime/runenv_test.go +++ b/runtime/runenv_test.go @@ -300,3 +300,30 @@ func TestFrequencyChange(t *testing.T) { require.Greater(len(tc.batchPoints), 5) tc.RUnlock() } + +func TestInvalidMetricName(t *testing.T) { + InfluxBatching = true + tc := &testClient{} + TestInfluxDBClient = tc + + re, cleanup := RandomTestRunEnv(t) + t.Cleanup(cleanup) + + re.R().RecordPoint("foo,i_am_an_invalid_tag_because_i_have_no_value", 1234) + re.R().RecordPoint("foo,i_am_an_invalid_tag_because_i_have_no_value,another,one_more", 1234) + re.R().RecordPoint("foo,", 1234) + + require := require.New(t) + + tc.RLock() + require.Empty(tc.batchPoints) + tc.RUnlock() + + _ = re.Close() + + tc.RLock() + require.NotEmpty(tc.batchPoints) + require.Len(tc.batchPoints, 1) + require.Len(tc.batchPoints[0].Points(), 3) + tc.RUnlock() +}