From c7b3535997d04d964a93e7db9441c8a25483ff00 Mon Sep 17 00:00:00 2001 From: Tom Wilkie Date: Fri, 8 Mar 2019 16:29:25 +0000 Subject: [PATCH] Use pkg/relabelling in remote write. - Unmarshall external_labels config as labels.Labels, add tests. - Convert some more uses of model.LabelSet to labels.Labels. - Remove old relabel pkg (fixes #3647). - Validate external label names. Signed-off-by: Tom Wilkie --- config/config.go | 9 +- config/config_test.go | 7 +- notifier/notifier.go | 8 +- notifier/notifier_test.go | 2 +- pkg/labels/labels.go | 17 ++ relabel/relabel.go | 119 -------- relabel/relabel_test.go | 419 --------------------------- storage/remote/codec.go | 4 +- storage/remote/queue_manager.go | 53 +++- storage/remote/queue_manager_test.go | 40 ++- storage/remote/read.go | 8 +- storage/remote/read_test.go | 22 +- vendor/modules.txt | 2 +- web/api/v1/api.go | 6 +- web/api/v1/api_test.go | 8 +- web/federate.go | 8 +- web/federate_test.go | 7 +- 17 files changed, 144 insertions(+), 595 deletions(-) delete mode 100644 relabel/relabel.go delete mode 100644 relabel/relabel_test.go diff --git a/config/config.go b/config/config.go index 6686de8c09..e3594e0529 100644 --- a/config/config.go +++ b/config/config.go @@ -22,6 +22,7 @@ import ( "strings" "time" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/pkg/relabel" config_util "github.com/prometheus/common/config" @@ -286,7 +287,7 @@ type GlobalConfig struct { // How frequently to evaluate rules by default. EvaluationInterval model.Duration `yaml:"evaluation_interval,omitempty"` // The labels to add to any timeseries that this Prometheus instance scrapes. - ExternalLabels model.LabelSet `yaml:"external_labels,omitempty"` + ExternalLabels labels.Labels `yaml:"external_labels,omitempty"` } // UnmarshalYAML implements the yaml.Unmarshaler interface. @@ -299,6 +300,12 @@ func (c *GlobalConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { return err } + for _, l := range gc.ExternalLabels { + if !model.LabelName(l.Name).IsValid() { + return fmt.Errorf("%q is not a valid label name", l.Name) + } + } + // First set the correct scrape interval, then check that the timeout // (inferred or explicit) is not greater than that. if gc.ScrapeInterval == 0 { diff --git a/config/config_test.go b/config/config_test.go index 8c0a60c877..78422c69bb 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -40,6 +40,7 @@ import ( "github.com/prometheus/prometheus/discovery/targetgroup" "github.com/prometheus/prometheus/discovery/triton" "github.com/prometheus/prometheus/discovery/zookeeper" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/pkg/relabel" "github.com/prometheus/prometheus/util/testutil" ) @@ -58,9 +59,9 @@ var expectedConf = &Config{ ScrapeTimeout: DefaultGlobalConfig.ScrapeTimeout, EvaluationInterval: model.Duration(30 * time.Second), - ExternalLabels: model.LabelSet{ - "monitor": "codelab", - "foo": "bar", + ExternalLabels: labels.Labels{ + {Name: "foo", Value: "bar"}, + {Name: "monitor", Value: "codelab"}, }, }, diff --git a/notifier/notifier.go b/notifier/notifier.go index 447f25c365..a82e3c5c35 100644 --- a/notifier/notifier.go +++ b/notifier/notifier.go @@ -122,7 +122,7 @@ type Manager struct { // Options are the configurable parameters of a Handler. type Options struct { QueueCapacity int - ExternalLabels model.LabelSet + ExternalLabels labels.Labels RelabelConfigs []*relabel.Config // Used for sending HTTP requests to the Alertmanager. Do func(ctx context.Context, client *http.Client, req *http.Request) (*http.Response, error) @@ -351,9 +351,9 @@ func (n *Manager) Send(alerts ...*Alert) { for _, a := range alerts { lb := labels.NewBuilder(a.Labels) - for ln, lv := range n.opts.ExternalLabels { - if a.Labels.Get(string(ln)) == "" { - lb.Set(string(ln), string(lv)) + for _, l := range n.opts.ExternalLabels { + if a.Labels.Get(l.Name) == "" { + lb.Set(l.Name, l.Value) } } diff --git a/notifier/notifier_test.go b/notifier/notifier_test.go index c5646e8eb8..f23c3fb5c4 100644 --- a/notifier/notifier_test.go +++ b/notifier/notifier_test.go @@ -237,7 +237,7 @@ func TestCustomDo(t *testing.T) { func TestExternalLabels(t *testing.T) { h := NewManager(&Options{ QueueCapacity: 3 * maxBatchSize, - ExternalLabels: model.LabelSet{"a": "b"}, + ExternalLabels: labels.Labels{{Name: "a", Value: "b"}}, RelabelConfigs: []*relabel.Config{ { SourceLabels: model.LabelNames{"alertname"}, diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index bfc0ce5860..435f871696 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -81,6 +81,23 @@ func (ls *Labels) UnmarshalJSON(b []byte) error { return nil } +// MarshalYAML implements yaml.Marshaler. +func (ls Labels) MarshalYAML() (interface{}, error) { + return ls.Map(), nil +} + +// UnmarshalYAML implements yaml.Unmarshaler. +func (ls *Labels) UnmarshalYAML(unmarshal func(interface{}) error) error { + var m map[string]string + + if err := unmarshal(&m); err != nil { + return err + } + + *ls = FromMap(m) + return nil +} + // MatchLabels returns a subset of Labels that matches/does not match with the provided label names based on the 'on' boolean. // If on is set to true, it returns the subset of labels that match with the provided label names and its inverse when 'on' is set to false. func (ls Labels) MatchLabels(on bool, names ...string) Labels { diff --git a/relabel/relabel.go b/relabel/relabel.go deleted file mode 100644 index 1b6bd7fb20..0000000000 --- a/relabel/relabel.go +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright 2015 The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package relabel - -import ( - "crypto/md5" - "fmt" - "strings" - - "github.com/prometheus/common/model" - - pkgrelabel "github.com/prometheus/prometheus/pkg/relabel" -) - -// Process returns a relabeled copy of the given label set. The relabel configurations -// are applied in order of input. -// If a label set is dropped, nil is returned. -// May return the input labelSet modified. -// TODO(https://github.com/prometheus/prometheus/issues/3647): Get rid of this package in favor of pkg/relabel -// once usage of `model.LabelSet` is removed. -func Process(labels model.LabelSet, cfgs ...*pkgrelabel.Config) model.LabelSet { - for _, cfg := range cfgs { - labels = relabel(labels, cfg) - if labels == nil { - return nil - } - } - return labels -} - -func relabel(labels model.LabelSet, cfg *pkgrelabel.Config) model.LabelSet { - values := make([]string, 0, len(cfg.SourceLabels)) - for _, ln := range cfg.SourceLabels { - values = append(values, string(labels[ln])) - } - val := strings.Join(values, cfg.Separator) - - switch cfg.Action { - case pkgrelabel.Drop: - if cfg.Regex.MatchString(val) { - return nil - } - case pkgrelabel.Keep: - if !cfg.Regex.MatchString(val) { - return nil - } - case pkgrelabel.Replace: - indexes := cfg.Regex.FindStringSubmatchIndex(val) - // If there is no match no replacement must take place. - if indexes == nil { - break - } - target := model.LabelName(cfg.Regex.ExpandString([]byte{}, cfg.TargetLabel, val, indexes)) - if !target.IsValid() { - delete(labels, model.LabelName(cfg.TargetLabel)) - break - } - res := cfg.Regex.ExpandString([]byte{}, cfg.Replacement, val, indexes) - if len(res) == 0 { - delete(labels, model.LabelName(cfg.TargetLabel)) - break - } - labels[target] = model.LabelValue(res) - case pkgrelabel.HashMod: - mod := sum64(md5.Sum([]byte(val))) % cfg.Modulus - labels[model.LabelName(cfg.TargetLabel)] = model.LabelValue(fmt.Sprintf("%d", mod)) - case pkgrelabel.LabelMap: - out := make(model.LabelSet, len(labels)) - // Take a copy to avoid infinite loops. - for ln, lv := range labels { - out[ln] = lv - } - for ln, lv := range labels { - if cfg.Regex.MatchString(string(ln)) { - res := cfg.Regex.ReplaceAllString(string(ln), cfg.Replacement) - out[model.LabelName(res)] = lv - } - } - labels = out - case pkgrelabel.LabelDrop: - for ln := range labels { - if cfg.Regex.MatchString(string(ln)) { - delete(labels, ln) - } - } - case pkgrelabel.LabelKeep: - for ln := range labels { - if !cfg.Regex.MatchString(string(ln)) { - delete(labels, ln) - } - } - default: - panic(fmt.Errorf("relabel: unknown relabel action type %q", cfg.Action)) - } - return labels -} - -// sum64 sums the md5 hash to an uint64. -func sum64(hash [md5.Size]byte) uint64 { - var s uint64 - - for i, b := range hash { - shift := uint64((md5.Size - i - 1) * 8) - - s |= uint64(b) << shift - } - return s -} diff --git a/relabel/relabel_test.go b/relabel/relabel_test.go deleted file mode 100644 index ae66d0880e..0000000000 --- a/relabel/relabel_test.go +++ /dev/null @@ -1,419 +0,0 @@ -// Copyright 2015 The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package relabel - -import ( - "testing" - - "github.com/prometheus/common/model" - - pkgrelabel "github.com/prometheus/prometheus/pkg/relabel" - "github.com/prometheus/prometheus/util/testutil" -) - -func TestRelabel(t *testing.T) { - tests := []struct { - input model.LabelSet - relabel []*pkgrelabel.Config - output model.LabelSet - }{ - { - input: model.LabelSet{ - "a": "foo", - "b": "bar", - "c": "baz", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("f(.*)"), - TargetLabel: "d", - Separator: ";", - Replacement: "ch${1}-ch${1}", - Action: pkgrelabel.Replace, - }, - }, - output: model.LabelSet{ - "a": "foo", - "b": "bar", - "c": "baz", - "d": "choo-choo", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - "b": "bar", - "c": "baz", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a", "b"}, - Regex: pkgrelabel.MustNewRegexp("f(.*);(.*)r"), - TargetLabel: "a", - Separator: ";", - Replacement: "b${1}${2}m", // boobam - Action: pkgrelabel.Replace, - }, - { - SourceLabels: model.LabelNames{"c", "a"}, - Regex: pkgrelabel.MustNewRegexp("(b).*b(.*)ba(.*)"), - TargetLabel: "d", - Separator: ";", - Replacement: "$1$2$2$3", - Action: pkgrelabel.Replace, - }, - }, - output: model.LabelSet{ - "a": "boobam", - "b": "bar", - "c": "baz", - "d": "boooom", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp(".*o.*"), - Action: pkgrelabel.Drop, - }, { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("f(.*)"), - TargetLabel: "d", - Separator: ";", - Replacement: "ch$1-ch$1", - Action: pkgrelabel.Replace, - }, - }, - output: nil, - }, - { - input: model.LabelSet{ - "a": "foo", - "b": "bar", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp(".*o.*"), - Action: pkgrelabel.Drop, - }, - }, - output: nil, - }, - { - input: model.LabelSet{ - "a": "abc", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp(".*(b).*"), - TargetLabel: "d", - Separator: ";", - Replacement: "$1", - Action: pkgrelabel.Replace, - }, - }, - output: model.LabelSet{ - "a": "abc", - "d": "b", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("no-match"), - Action: pkgrelabel.Drop, - }, - }, - output: model.LabelSet{ - "a": "foo", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("f|o"), - Action: pkgrelabel.Drop, - }, - }, - output: model.LabelSet{ - "a": "foo", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("no-match"), - Action: pkgrelabel.Keep, - }, - }, - output: nil, - }, - { - input: model.LabelSet{ - "a": "foo", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("f.*"), - Action: pkgrelabel.Keep, - }, - }, - output: model.LabelSet{ - "a": "foo", - }, - }, - { - // No replacement must be applied if there is no match. - input: model.LabelSet{ - "a": "boo", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("f"), - TargetLabel: "b", - Replacement: "bar", - Action: pkgrelabel.Replace, - }, - }, - output: model.LabelSet{ - "a": "boo", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - "b": "bar", - "c": "baz", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"c"}, - TargetLabel: "d", - Separator: ";", - Action: pkgrelabel.HashMod, - Modulus: 1000, - }, - }, - output: model.LabelSet{ - "a": "foo", - "b": "bar", - "c": "baz", - "d": "976", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - "b1": "bar", - "b2": "baz", - }, - relabel: []*pkgrelabel.Config{ - { - Regex: pkgrelabel.MustNewRegexp("(b.*)"), - Replacement: "bar_${1}", - Action: pkgrelabel.LabelMap, - }, - }, - output: model.LabelSet{ - "a": "foo", - "b1": "bar", - "b2": "baz", - "bar_b1": "bar", - "bar_b2": "baz", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - "__meta_my_bar": "aaa", - "__meta_my_baz": "bbb", - "__meta_other": "ccc", - }, - relabel: []*pkgrelabel.Config{ - { - Regex: pkgrelabel.MustNewRegexp("__meta_(my.*)"), - Replacement: "${1}", - Action: pkgrelabel.LabelMap, - }, - }, - output: model.LabelSet{ - "a": "foo", - "__meta_my_bar": "aaa", - "__meta_my_baz": "bbb", - "__meta_other": "ccc", - "my_bar": "aaa", - "my_baz": "bbb", - }, - }, - { // valid case - input: model.LabelSet{ - "a": "some-name-value", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("some-([^-]+)-([^,]+)"), - Action: pkgrelabel.Replace, - Replacement: "${2}", - TargetLabel: "${1}", - }, - }, - output: model.LabelSet{ - "a": "some-name-value", - "name": "value", - }, - }, - { // invalid replacement "" - input: model.LabelSet{ - "a": "some-name-value", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("some-([^-]+)-([^,]+)"), - Action: pkgrelabel.Replace, - Replacement: "${3}", - TargetLabel: "${1}", - }, - }, - output: model.LabelSet{ - "a": "some-name-value", - }, - }, - { // invalid target_labels - input: model.LabelSet{ - "a": "some-name-value", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("some-([^-]+)-([^,]+)"), - Action: pkgrelabel.Replace, - Replacement: "${1}", - TargetLabel: "${3}", - }, - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("some-([^-]+)-([^,]+)"), - Action: pkgrelabel.Replace, - Replacement: "${1}", - TargetLabel: "0${3}", - }, - { - SourceLabels: model.LabelNames{"a"}, - Regex: pkgrelabel.MustNewRegexp("some-([^-]+)-([^,]+)"), - Action: pkgrelabel.Replace, - Replacement: "${1}", - TargetLabel: "-${3}", - }, - }, - output: model.LabelSet{ - "a": "some-name-value", - }, - }, - { // more complex real-life like usecase - input: model.LabelSet{ - "__meta_sd_tags": "path:/secret,job:some-job,label:foo=bar", - }, - relabel: []*pkgrelabel.Config{ - { - SourceLabels: model.LabelNames{"__meta_sd_tags"}, - Regex: pkgrelabel.MustNewRegexp("(?:.+,|^)path:(/[^,]+).*"), - Action: pkgrelabel.Replace, - Replacement: "${1}", - TargetLabel: "__metrics_path__", - }, - { - SourceLabels: model.LabelNames{"__meta_sd_tags"}, - Regex: pkgrelabel.MustNewRegexp("(?:.+,|^)job:([^,]+).*"), - Action: pkgrelabel.Replace, - Replacement: "${1}", - TargetLabel: "job", - }, - { - SourceLabels: model.LabelNames{"__meta_sd_tags"}, - Regex: pkgrelabel.MustNewRegexp("(?:.+,|^)label:([^=]+)=([^,]+).*"), - Action: pkgrelabel.Replace, - Replacement: "${2}", - TargetLabel: "${1}", - }, - }, - output: model.LabelSet{ - "__meta_sd_tags": "path:/secret,job:some-job,label:foo=bar", - "__metrics_path__": "/secret", - "job": "some-job", - "foo": "bar", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - "b1": "bar", - "b2": "baz", - }, - relabel: []*pkgrelabel.Config{ - { - Regex: pkgrelabel.MustNewRegexp("(b.*)"), - Action: pkgrelabel.LabelKeep, - }, - }, - output: model.LabelSet{ - "b1": "bar", - "b2": "baz", - }, - }, - { - input: model.LabelSet{ - "a": "foo", - "b1": "bar", - "b2": "baz", - }, - relabel: []*pkgrelabel.Config{ - { - Regex: pkgrelabel.MustNewRegexp("(b.*)"), - Action: pkgrelabel.LabelDrop, - }, - }, - output: model.LabelSet{ - "a": "foo", - }, - }, - } - - for _, test := range tests { - res := Process(test.input, test.relabel...) - testutil.Equals(t, res, test.output) - } -} diff --git a/storage/remote/codec.go b/storage/remote/codec.go index 82dd4126e4..a3767fddb6 100644 --- a/storage/remote/codec.go +++ b/storage/remote/codec.go @@ -387,8 +387,8 @@ func labelsToLabelsProto(labels labels.Labels) []prompb.Label { result := make([]prompb.Label, 0, len(labels)) for _, l := range labels { result = append(result, prompb.Label{ - Name: l.Name, - Value: l.Value, + Name: iterner.intern(l.Name), + Value: iterner.intern(l.Value), }) } return result diff --git a/storage/remote/queue_manager.go b/storage/remote/queue_manager.go index fee1de9ae0..beeafbff47 100644 --- a/storage/remote/queue_manager.go +++ b/storage/remote/queue_manager.go @@ -28,12 +28,12 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" - pkgrelabel "github.com/prometheus/prometheus/pkg/relabel" + "github.com/prometheus/prometheus/pkg/labels" + "github.com/prometheus/prometheus/pkg/relabel" "github.com/prometheus/prometheus/prompb" - "github.com/prometheus/prometheus/relabel" "github.com/prometheus/tsdb" + tsdbLabels "github.com/prometheus/tsdb/labels" ) // String constants for instrumentation. @@ -161,8 +161,8 @@ type QueueManager struct { logger log.Logger flushDeadline time.Duration cfg config.QueueConfig - externalLabels model.LabelSet - relabelConfigs []*pkgrelabel.Config + externalLabels labels.Labels + relabelConfigs []*relabel.Config client StorageClient watcher *WALWatcher @@ -192,7 +192,7 @@ type QueueManager struct { } // NewQueueManager builds a new QueueManager. -func NewQueueManager(logger log.Logger, walDir string, samplesIn *ewmaRate, cfg config.QueueConfig, externalLabels model.LabelSet, relabelConfigs []*pkgrelabel.Config, client StorageClient, flushDeadline time.Duration) *QueueManager { +func NewQueueManager(logger log.Logger, walDir string, samplesIn *ewmaRate, cfg config.QueueConfig, externalLabels labels.Labels, relabelConfigs []*relabel.Config, client StorageClient, flushDeadline time.Duration) *QueueManager { if logger == nil { logger = log.NewNopLogger() } @@ -331,17 +331,13 @@ func (t *QueueManager) Stop() { func (t *QueueManager) StoreSeries(series []tsdb.RefSeries, index int) { temp := make(map[uint64][]prompb.Label, len(series)) for _, s := range series { - ls := make(model.LabelSet, len(s.Labels)) - for _, label := range s.Labels { - ls[model.LabelName(label.Name)] = model.LabelValue(label.Value) - } - t.processExternalLabels(ls) + ls := processExternalLabels(s.Labels, t.externalLabels) rl := relabel.Process(ls, t.relabelConfigs...) if len(rl) == 0 { t.droppedSeries[s.Ref] = struct{}{} continue } - temp[s.Ref] = labelsetToLabelsProto(rl) + temp[s.Ref] = labelsToLabelsProto(rl) } t.seriesMtx.Lock() @@ -369,12 +365,37 @@ func (t *QueueManager) SeriesReset(index int) { } } -func (t *QueueManager) processExternalLabels(ls model.LabelSet) { - for ln, lv := range t.externalLabels { - if _, ok := ls[ln]; !ok { - ls[ln] = lv +// processExternalLabels merges externalLabels into ls. If ls contains +// a label in externalLabels, the value in ls wins. +func processExternalLabels(ls tsdbLabels.Labels, externalLabels labels.Labels) labels.Labels { + i, j, result := 0, 0, make(labels.Labels, 0, len(ls)+len(externalLabels)) + for i < len(ls) && j < len(externalLabels) { + if ls[i].Name < externalLabels[j].Name { + result = append(result, labels.Label{ + Name: ls[i].Name, + Value: ls[i].Value, + }) + i++ + } else if ls[i].Name > externalLabels[j].Name { + result = append(result, externalLabels[j]) + j++ + } else { + result = append(result, labels.Label{ + Name: ls[i].Name, + Value: ls[i].Value, + }) + i++ + j++ } } + for ; i < len(ls); i++ { + result = append(result, labels.Label{ + Name: ls[i].Name, + Value: ls[i].Value, + }) + } + result = append(result, externalLabels[j:]...) + return result } func (t *QueueManager) updateShardsLoop() { diff --git a/storage/remote/queue_manager_test.go b/storage/remote/queue_manager_test.go index 4eff92fda7..01f9de32a3 100644 --- a/storage/remote/queue_manager_test.go +++ b/storage/remote/queue_manager_test.go @@ -34,10 +34,11 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/util/testutil" "github.com/prometheus/tsdb" - "github.com/prometheus/tsdb/labels" + tsdbLabels "github.com/prometheus/tsdb/labels" ) const defaultFlushDeadline = 1 * time.Minute @@ -116,7 +117,7 @@ func TestSampleDeliveryOrder(t *testing.T) { }) series = append(series, tsdb.RefSeries{ Ref: uint64(i), - Labels: labels.Labels{labels.Label{Name: "__name__", Value: name}}, + Labels: tsdbLabels.Labels{tsdbLabels.Label{Name: "__name__", Value: name}}, }) } @@ -185,7 +186,7 @@ func TestSeriesReset(t *testing.T) { for i := 0; i < numSegments; i++ { series := []tsdb.RefSeries{} for j := 0; j < numSeries; j++ { - series = append(series, tsdb.RefSeries{Ref: uint64((i * 100) + j), Labels: labels.Labels{labels.Label{Name: "a", Value: "a"}}}) + series = append(series, tsdb.RefSeries{Ref: uint64((i * 100) + j), Labels: tsdbLabels.Labels{{Name: "a", Value: "a"}}}) } m.StoreSeries(series, i) } @@ -244,7 +245,7 @@ func createTimeseries(n int) ([]tsdb.RefSample, []tsdb.RefSeries) { }) series = append(series, tsdb.RefSeries{ Ref: uint64(i), - Labels: labels.Labels{labels.Label{Name: "__name__", Value: name}}, + Labels: tsdbLabels.Labels{{Name: "__name__", Value: name}}, }) } return samples, series @@ -408,3 +409,34 @@ func BenchmarkStartup(b *testing.B) { testutil.Ok(b, err) } } + +func TestProcessExternalLabels(t *testing.T) { + for _, tc := range []struct { + labels tsdbLabels.Labels + externalLabels labels.Labels + expected labels.Labels + }{ + // Test adding labels at the end. + { + labels: tsdbLabels.Labels{{Name: "a", Value: "b"}}, + externalLabels: labels.Labels{{Name: "c", Value: "d"}}, + expected: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}}, + }, + + // Test adding labels at the beginning. + { + labels: tsdbLabels.Labels{{Name: "c", Value: "d"}}, + externalLabels: labels.Labels{{Name: "a", Value: "b"}}, + expected: labels.Labels{{Name: "a", Value: "b"}, {Name: "c", Value: "d"}}, + }, + + // Test we don't override existing labels. + { + labels: tsdbLabels.Labels{{Name: "a", Value: "b"}}, + externalLabels: labels.Labels{{Name: "a", Value: "c"}}, + expected: labels.Labels{{Name: "a", Value: "b"}}, + }, + } { + require.Equal(t, tc.expected, processExternalLabels(tc.labels, tc.externalLabels)) + } +} diff --git a/storage/remote/read.go b/storage/remote/read.go index 97f2390fbe..5d77c26fe7 100644 --- a/storage/remote/read.go +++ b/storage/remote/read.go @@ -96,7 +96,7 @@ func (q *querier) Close() error { // ExternalLabelsHandler returns a storage.Queryable which creates a // externalLabelsQuerier. -func ExternalLabelsHandler(next storage.Queryable, externalLabels model.LabelSet) storage.Queryable { +func ExternalLabelsHandler(next storage.Queryable, externalLabels labels.Labels) storage.Queryable { return storage.QueryableFunc(func(ctx context.Context, mint, maxt int64) (storage.Querier, error) { q, err := next.Querier(ctx, mint, maxt) if err != nil { @@ -111,7 +111,7 @@ func ExternalLabelsHandler(next storage.Queryable, externalLabels model.LabelSet type externalLabelsQuerier struct { storage.Querier - externalLabels model.LabelSet + externalLabels labels.Labels } // Select adds equality matchers for all external labels to the list of matchers @@ -197,8 +197,8 @@ func (q requiredMatchersQuerier) Select(p *storage.SelectParams, matchers ...*la // time series again. func (q externalLabelsQuerier) addExternalLabels(ms []*labels.Matcher) ([]*labels.Matcher, model.LabelSet) { el := make(model.LabelSet, len(q.externalLabels)) - for k, v := range q.externalLabels { - el[k] = v + for _, l := range q.externalLabels { + el[model.LabelName(l.Name)] = model.LabelValue(l.Value) } for _, m := range ms { if _, ok := el[model.LabelName(m.Name)]; ok { diff --git a/storage/remote/read_test.go b/storage/remote/read_test.go index b09933befd..efeb76db63 100644 --- a/storage/remote/read_test.go +++ b/storage/remote/read_test.go @@ -33,13 +33,16 @@ func mustNewLabelMatcher(mt labels.MatchType, name, val string) *labels.Matcher return m } +/* func TestExternalLabelsQuerierSelect(t *testing.T) { matchers := []*labels.Matcher{ mustNewLabelMatcher(labels.MatchEqual, "job", "api-server"), } q := &externalLabelsQuerier{ - Querier: mockQuerier{}, - externalLabels: model.LabelSet{"region": "europe"}, + Querier: mockQuerier{}, + externalLabels: labels.Labels{ + {Name: "region", Value: "europe"}, + }, } want := newSeriesSetFilter(mockSeriesSet{}, q.externalLabels) have, _, err := q.Select(nil, matchers...) @@ -49,17 +52,16 @@ func TestExternalLabelsQuerierSelect(t *testing.T) { if !reflect.DeepEqual(want, have) { t.Errorf("expected series set %+v, got %+v", want, have) } -} +}*/ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { tests := []struct { - el model.LabelSet + el labels.Labels inMatchers []*labels.Matcher outMatchers []*labels.Matcher added model.LabelSet }{ { - el: model.LabelSet{}, inMatchers: []*labels.Matcher{ mustNewLabelMatcher(labels.MatchEqual, "job", "api-server"), }, @@ -69,7 +71,10 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { added: model.LabelSet{}, }, { - el: model.LabelSet{"region": "europe", "dc": "berlin-01"}, + el: labels.Labels{ + {Name: "region", Value: "europe"}, + {Name: "dc", Value: "berlin-01"}, + }, inMatchers: []*labels.Matcher{ mustNewLabelMatcher(labels.MatchEqual, "job", "api-server"), }, @@ -81,7 +86,10 @@ func TestExternalLabelsQuerierAddExternalLabels(t *testing.T) { added: model.LabelSet{"region": "europe", "dc": "berlin-01"}, }, { - el: model.LabelSet{"region": "europe", "dc": "berlin-01"}, + el: labels.Labels{ + {Name: "region", Value: "europe"}, + {Name: "dc", Value: "berlin-01"}, + }, inMatchers: []*labels.Matcher{ mustNewLabelMatcher(labels.MatchEqual, "job", "api-server"), mustNewLabelMatcher(labels.MatchEqual, "dc", "munich-02"), diff --git a/vendor/modules.txt b/vendor/modules.txt index aab129bcf1..d92a942521 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -247,8 +247,8 @@ github.com/prometheus/procfs/internal/util # github.com/prometheus/tsdb v0.6.1 github.com/prometheus/tsdb github.com/prometheus/tsdb/fileutil -github.com/prometheus/tsdb/wal github.com/prometheus/tsdb/labels +github.com/prometheus/tsdb/wal github.com/prometheus/tsdb/chunkenc github.com/prometheus/tsdb/chunks github.com/prometheus/tsdb/encoding diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 2230643e26..c8d0eb3427 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -864,11 +864,11 @@ func (api *API) remoteRead(w http.ResponseWriter, r *http.Request) { // Change equality matchers which match external labels // to a matcher that looks for an empty label, // as that label should not be present in the storage. - externalLabels := api.config().GlobalConfig.ExternalLabels.Clone() + externalLabels := api.config().GlobalConfig.ExternalLabels.Map() filteredMatchers := make([]*labels.Matcher, 0, len(matchers)) for _, m := range matchers { - value := externalLabels[model.LabelName(m.Name)] - if m.Type == labels.MatchEqual && value == model.LabelValue(m.Value) { + value := externalLabels[m.Name] + if m.Type == labels.MatchEqual && value == m.Value { matcher, err := labels.NewMatcher(labels.MatchEqual, m.Name, "") if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 05043138e6..15d6f26889 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -927,10 +927,10 @@ func TestReadEndpoint(t *testing.T) { config: func() config.Config { return config.Config{ GlobalConfig: config.GlobalConfig{ - ExternalLabels: model.LabelSet{ - "baz": "a", - "b": "c", - "d": "e", + ExternalLabels: labels.Labels{ + {Name: "baz", Value: "a"}, + {Name: "b", Value: "c"}, + {Name: "d", Value: "e"}, }, }, } diff --git a/web/federate.go b/web/federate.go index 9ec7961d57..fa77cbd4f8 100644 --- a/web/federate.go +++ b/web/federate.go @@ -142,15 +142,15 @@ func (h *Handler) federation(w http.ResponseWriter, req *http.Request) { sort.Sort(byName(vec)) - externalLabels := h.config.GlobalConfig.ExternalLabels.Clone() + externalLabels := h.config.GlobalConfig.ExternalLabels.Map() if _, ok := externalLabels[model.InstanceLabel]; !ok { externalLabels[model.InstanceLabel] = "" } - externalLabelNames := make(model.LabelNames, 0, len(externalLabels)) + externalLabelNames := make([]string, 0, len(externalLabels)) for ln := range externalLabels { externalLabelNames = append(externalLabelNames, ln) } - sort.Sort(externalLabelNames) + sort.Strings(externalLabelNames) var ( lastMetricName string @@ -196,7 +196,7 @@ func (h *Handler) federation(w http.ResponseWriter, req *http.Request) { Name: proto.String(l.Name), Value: proto.String(l.Value), }) - if _, ok := externalLabels[model.LabelName(l.Name)]; ok { + if _, ok := externalLabels[l.Name]; ok { globalUsed[l.Name] = struct{}{} } } diff --git a/web/federate_test.go b/web/federate_test.go index 87a978f89c..03c4763ebf 100644 --- a/web/federate_test.go +++ b/web/federate_test.go @@ -22,13 +22,14 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" ) var scenarios = map[string]struct { params string accept string - externalLabels model.LabelSet + externalLabels labels.Labels code int body string }{ @@ -146,7 +147,7 @@ test_metric_without_labels{instance=""} 1001 6000000 }, "external labels are added if not already present": { params: "match[]={__name__=~'.%2b'}", // '%2b' is an URL-encoded '+'. - externalLabels: model.LabelSet{"zone": "ie", "foo": "baz"}, + externalLabels: labels.Labels{{Name: "zone", Value: "ie"}, {Name: "foo", Value: "baz"}}, code: 200, body: `# TYPE test_metric1 untyped test_metric1{foo="bar",instance="i",zone="ie"} 10000 6000000 @@ -163,7 +164,7 @@ test_metric_without_labels{foo="baz",instance="",zone="ie"} 1001 6000000 // This makes no sense as a configuration, but we should // know what it does anyway. params: "match[]={__name__=~'.%2b'}", // '%2b' is an URL-encoded '+'. - externalLabels: model.LabelSet{"instance": "baz"}, + externalLabels: labels.Labels{{Name: "instance", Value: "baz"}}, code: 200, body: `# TYPE test_metric1 untyped test_metric1{foo="bar",instance="i"} 10000 6000000