From 0e078346742bb5a40d6c0c1027a21589993d5dc5 Mon Sep 17 00:00:00 2001 From: "m.conraux" Date: Fri, 7 Sep 2018 15:36:48 +0200 Subject: [PATCH 1/3] Apply the same logic to all metric type for Hostname prefixing --- metrics.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/metrics.go b/metrics.go index cf9def7..ee3ad74 100644 --- a/metrics.go +++ b/metrics.go @@ -61,8 +61,12 @@ func (m *Metrics) IncrCounter(key []string, val float32) { } func (m *Metrics) IncrCounterWithLabels(key []string, val float32, labels []Label) { - if m.HostName != "" && m.EnableHostnameLabel { - labels = append(labels, Label{"host", m.HostName}) + if m.HostName != "" { + if m.EnableHostnameLabel { + labels = append(labels, Label{"host", m.HostName}) + } else if m.EnableHostname { + key = insert(0, m.HostName, key) + } } if m.EnableTypePrefix { key = insert(0, "counter", key) @@ -86,8 +90,12 @@ func (m *Metrics) AddSample(key []string, val float32) { } func (m *Metrics) AddSampleWithLabels(key []string, val float32, labels []Label) { - if m.HostName != "" && m.EnableHostnameLabel { - labels = append(labels, Label{"host", m.HostName}) + if m.HostName != "" { + if m.EnableHostnameLabel { + labels = append(labels, Label{"host", m.HostName}) + } else if m.EnableHostname { + key = insert(0, m.HostName, key) + } } if m.EnableTypePrefix { key = insert(0, "sample", key) @@ -111,8 +119,12 @@ func (m *Metrics) MeasureSince(key []string, start time.Time) { } func (m *Metrics) MeasureSinceWithLabels(key []string, start time.Time, labels []Label) { - if m.HostName != "" && m.EnableHostnameLabel { - labels = append(labels, Label{"host", m.HostName}) + if m.HostName != "" { + if m.EnableHostnameLabel { + labels = append(labels, Label{"host", m.HostName}) + } else if m.EnableHostname { + key = insert(0, m.HostName, key) + } } if m.EnableTypePrefix { key = insert(0, "timer", key) From 0b3f4e14a8988c24b1829d2cd3c19602535780a9 Mon Sep 17 00:00:00 2001 From: "m.conraux" Date: Fri, 7 Sep 2018 15:49:29 +0200 Subject: [PATCH 2/3] Prevent EnableHostname and EnableHostnameLabel to be true at the same time --- start.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/start.go b/start.go index 32a28c4..c3b4d14 100644 --- a/start.go +++ b/start.go @@ -1,6 +1,7 @@ package metrics import ( + "fmt" "os" "sync" "sync/atomic" @@ -69,6 +70,11 @@ func DefaultConfig(serviceName string) *Config { // New is used to create a new instance of Metrics func New(conf *Config, sink MetricSink) (*Metrics, error) { + + if conf.EnableHostname && conf.EnableHostnameLabel { + return nil, fmt.Errorf("both EnableHostname and EnableHostnameLabel can't be true at the same time") + } + met := &Metrics{} met.Config = *conf met.sink = sink From 36903e81ac45086ad543bea308cc7299bd3b42f3 Mon Sep 17 00:00:00 2001 From: Martin Conraux Date: Mon, 24 Sep 2018 11:20:43 +0200 Subject: [PATCH 3/3] Add tests for hostname prefixing --- metrics_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/metrics_test.go b/metrics_test.go index 3fed729..db5662a 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -3,6 +3,7 @@ package metrics import ( "reflect" "runtime" + "strings" "testing" "time" ) @@ -376,6 +377,75 @@ func HasElem(s interface{}, elem interface{}) bool { return false } +func TestMetrics_HostnameBothProvided(t *testing.T) { + m := &MockSink{} + conf := DefaultConfig("") + conf.EnableHostnameLabel = true + conf.EnableHostname = true + if _, err := New(conf, m); err != nil && !strings.HasPrefix(err.Error(), "both EnableHostname") { + t.Fatalf("New should throw when given both EnableHostname and EnableHostnameLabel as true") + } +} + +func TestMetrics_HostNameLabel(t *testing.T) { + m := &MockSink{} + conf := DefaultConfig("") + hostname := "testhostname" + conf.HostName = hostname + conf.EnableHostname = false + conf.EnableHostnameLabel = true + met, err := New(conf, m) + if err != nil { + t.Fatal(err) + } + + key := []string{"test"} + met.SetGauge(append(key, "gauge"), 1) + met.IncrCounter(append(key, "counter"), 1) + met.AddSample(append(key, "summary"), 1) + + hostnameLabel := Label{Name: "host", Value: hostname} + + if len(m.keys) != 3 { + t.Fatalf("only 3 keys should be present") + } + + for i, metric := range m.labels { + if !HasElem(metric, hostnameLabel) { + t.Fatalf("label hostname should be present and valid in %s. Labels: %+v", strings.Join(m.keys[i], "_"), metric) + } + } + +} + +func TestMetrics_HostName(t *testing.T) { + m := &MockSink{} + conf := DefaultConfig("") + hostname := "testhostname" + conf.HostName = hostname + conf.EnableHostname = true + conf.EnableHostnameLabel = false + met, err := New(conf, m) + if err != nil { + t.Fatal(err) + } + + key := []string{"test"} + met.SetGauge(append(key, "gauge"), 1) + met.IncrCounter(append(key, "counter"), 1) + met.AddSample(append(key, "summary"), 1) + + if len(m.keys) != 3 { + t.Fatalf("only 3 keys should be present") + } + + for i, metric := range m.keys { + if !HasElem(metric, hostname) { + t.Fatalf("%s should be present in metric keys and valid in %s. Keys: %+v", hostname, strings.Join(m.keys[i], "_"), metric) + } + } + +} func TestMetrics_Filter_Whitelist(t *testing.T) { m := &MockSink{}