From d60b6f54406a7c265e2f5249e76844079e00f2b0 Mon Sep 17 00:00:00 2001
From: Edward Welch <edward.welch@grafana.com>
Date: Fri, 31 May 2019 14:33:18 -0400
Subject: [PATCH] PR feedback
---
pkg/logentry/stages/json.go | 2 +-
pkg/logentry/stages/match.go | 4 ---
pkg/logentry/stages/metrics.go | 16 +++++++---
pkg/logentry/stages/metrics_test.go | 45 +++++++++++++++++++++++++++++
pkg/logentry/stages/regex.go | 2 +-
5 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/pkg/logentry/stages/json.go b/pkg/logentry/stages/json.go
index 2458d744..3b3f5412 100644
--- a/pkg/logentry/stages/json.go
+++ b/pkg/logentry/stages/json.go
@@ -73,7 +73,7 @@ func newJSONStage(logger log.Logger, config interface{}) (*jsonStage, error) {
return &jsonStage{
cfg: cfg,
expressions: expressions,
- logger: log.With(logger, "component", "mutator", "type", "json"),
+ logger: log.With(logger, "component", "stage", "type", "json"),
}, nil
}
diff --git a/pkg/logentry/stages/match.go b/pkg/logentry/stages/match.go
index 59895264..31249405 100644
--- a/pkg/logentry/stages/match.go
+++ b/pkg/logentry/stages/match.go
@@ -73,8 +73,6 @@ func newMatcherStage(logger log.Logger, jobName *string, config interface{}, reg
}
return &matcherStage{
- cfgs: cfg,
- logger: logger,
matchers: matchers,
pipeline: pl,
}, nil
@@ -82,9 +80,7 @@ func newMatcherStage(logger log.Logger, jobName *string, config interface{}, reg
// matcherStage applies Label matchers to determine if the include stages should be run
type matcherStage struct {
- cfgs *MatcherConfig
matchers []*labels.Matcher
- logger log.Logger
pipeline Stage
}
diff --git a/pkg/logentry/stages/metrics.go b/pkg/logentry/stages/metrics.go
index b5149844..4410456c 100644
--- a/pkg/logentry/stages/metrics.go
+++ b/pkg/logentry/stages/metrics.go
@@ -26,6 +26,7 @@ const (
MetricTypeHistogram = "histogram"
ErrEmptyMetricsStageConfig = "empty metric stage configuration"
+ ErrMetricsStageInvalidType = "invalid metric type '%s', metric type must be one of 'counter', 'gauge', or 'histogram'"
)
// MetricConfig is a single metrics configuration.
@@ -51,6 +52,13 @@ func validateMetricsConfig(cfg MetricsConfig) error {
cp.Source = &nm
cfg[name] = cp
}
+
+ config.MetricType = strings.ToLower(config.MetricType)
+ if config.MetricType != MetricTypeCounter &&
+ config.MetricType != MetricTypeGauge &&
+ config.MetricType != MetricTypeHistogram {
+ return errors.Errorf(ErrMetricsStageInvalidType, config.MetricType)
+ }
}
return nil
}
@@ -144,7 +152,7 @@ func (m *metricStage) recordCounter(name string, counter *metric.Counters, label
case metric.CounterAdd:
f, err := getFloat(v)
if err != nil || f < 0 {
- level.Debug(m.logger).Log("msg", "failed to convert extracted value to float", "metric", name, "err", err)
+ level.Debug(m.logger).Log("msg", "failed to convert extracted value to positive float", "metric", name, "err", err)
return
}
counter.With(labels).Add(f)
@@ -171,7 +179,7 @@ func (m *metricStage) recordGauge(name string, gauge *metric.Gauges, labels mode
case metric.GaugeSet:
f, err := getFloat(v)
if err != nil || f < 0 {
- level.Debug(m.logger).Log("msg", "failed to convert extracted value to float", "metric", name, "err", err)
+ level.Debug(m.logger).Log("msg", "failed to convert extracted value to positive float", "metric", name, "err", err)
return
}
gauge.With(labels).Set(f)
@@ -182,14 +190,14 @@ func (m *metricStage) recordGauge(name string, gauge *metric.Gauges, labels mode
case metric.GaugeAdd:
f, err := getFloat(v)
if err != nil || f < 0 {
- level.Debug(m.logger).Log("msg", "failed to convert extracted value to float", "metric", name, "err", err)
+ level.Debug(m.logger).Log("msg", "failed to convert extracted value to positive float", "metric", name, "err", err)
return
}
gauge.With(labels).Add(f)
case metric.GaugeSub:
f, err := getFloat(v)
if err != nil || f < 0 {
- level.Debug(m.logger).Log("msg", "failed to convert extracted value to float", "metric", name, "err", err)
+ level.Debug(m.logger).Log("msg", "failed to convert extracted value to positive float", "metric", name, "err", err)
return
}
gauge.With(labels).Sub(f)
diff --git a/pkg/logentry/stages/metrics_test.go b/pkg/logentry/stages/metrics_test.go
index 9d7c4b37..b404611e 100644
--- a/pkg/logentry/stages/metrics_test.go
+++ b/pkg/logentry/stages/metrics_test.go
@@ -6,6 +6,7 @@ import (
"time"
"github.com/cortexproject/cortex/pkg/util"
+ "github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/prometheus/common/model"
@@ -59,6 +60,50 @@ func TestMetricsPipeline(t *testing.T) {
}
}
+func Test(t *testing.T) {
+ tests := map[string]struct {
+ config MetricsConfig
+ err error
+ }{
+ "empty": {
+ nil,
+ errors.New(ErrEmptyMetricsStageConfig),
+ },
+ "invalid metric type": {
+ MetricsConfig{
+ "metric1": MetricConfig{
+ MetricType: "Piplne",
+ },
+ },
+ errors.Errorf(ErrMetricsStageInvalidType, "piplne"),
+ },
+ "valid": {
+ MetricsConfig{
+ "metric1": MetricConfig{
+ MetricType: "Counter",
+ Description: "some description",
+ Config: metric.CounterConfig{
+ Action: "inc",
+ },
+ },
+ },
+ nil,
+ },
+ }
+
+ for name, test := range tests {
+ test := test
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+ err := validateMetricsConfig(test.config)
+ if (err != nil) && (err.Error() != test.err.Error()) {
+ t.Errorf("Metrics stage validation error, expected error = %v, actual error = %v", test.err, err)
+ return
+ }
+ })
+ }
+}
+
var labelFoo = model.LabelSet(map[model.LabelName]model.LabelValue{"foo": "bar", "bar": "foo"})
var labelFu = model.LabelSet(map[model.LabelName]model.LabelValue{"fu": "baz", "baz": "fu"})
diff --git a/pkg/logentry/stages/regex.go b/pkg/logentry/stages/regex.go
index 10d8cc2f..3ef563ca 100644
--- a/pkg/logentry/stages/regex.go
+++ b/pkg/logentry/stages/regex.go
@@ -61,7 +61,7 @@ func newRegexStage(logger log.Logger, config interface{}) (Stage, error) {
return ®exStage{
cfg: cfg,
expression: expression,
- logger: log.With(logger, "component", "mutator", "type", "regex"),
+ logger: log.With(logger, "component", "stage", "type", "regex"),
}, nil
}
--
GitLab