diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 2d79dbe75..b1d434881 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -285,20 +285,26 @@ type Builder struct { add []Label } -// NewBuilder returns a new LabelsBuilder +// NewBuilder returns a new LabelsBuilder. func NewBuilder(base Labels) *Builder { - return &Builder{ - base: base, - del: make([]string, 0, 5), - add: make([]Label, 0, 5), + b := &Builder{ + del: make([]string, 0, 5), + add: make([]Label, 0, 5), } + b.Reset(base) + return b } -// Reset clears all current state for the builder +// Reset clears all current state for the builder. func (b *Builder) Reset(base Labels) { b.base = base b.del = b.del[:0] b.add = b.add[:0] + for _, l := range b.base { + if l.Value == "" { + b.del = append(b.del, l.Name) + } + } } // Del deletes the label of the given name. @@ -316,6 +322,10 @@ func (b *Builder) Del(ns ...string) *Builder { // Set the name/value pair as a label. func (b *Builder) Set(n, v string) *Builder { + if v == "" { + // Empty labels are the same as missing labels. + return b.Del(n) + } for i, a := range b.add { if a.Name == n { b.add[i].Value = v diff --git a/rules/alerting_test.go b/rules/alerting_test.go index 8d611c3e4..ee95dd47e 100644 --- a/rules/alerting_test.go +++ b/rules/alerting_test.go @@ -49,8 +49,7 @@ func TestAlertingRuleLabelsUpdate(t *testing.T) { testutil.Ok(t, err) defer suite.Close() - err = suite.Run() - testutil.Ok(t, err) + testutil.Ok(t, suite.Run()) expr, err := promql.ParseExpr(`http_requests < 100`) testutil.Ok(t, err) @@ -152,8 +151,7 @@ func TestAlertingRuleExternalLabelsInTemplate(t *testing.T) { testutil.Ok(t, err) defer suite.Close() - err = suite.Run() - testutil.Ok(t, err) + testutil.Ok(t, suite.Run()) expr, err := promql.ParseExpr(`http_requests < 100`) testutil.Ok(t, err) @@ -236,3 +234,58 @@ func TestAlertingRuleExternalLabelsInTemplate(t *testing.T) { testutil.Equals(t, result, filteredRes) } + +func TestAlertingRuleEmptyLabelFromTemplate(t *testing.T) { + suite, err := promql.NewTest(t, ` + load 1m + http_requests{job="app-server", instance="0"} 75 85 70 70 + `) + testutil.Ok(t, err) + defer suite.Close() + + testutil.Ok(t, suite.Run()) + + expr, err := promql.ParseExpr(`http_requests < 100`) + testutil.Ok(t, err) + + rule := NewAlertingRule( + "EmptyLabel", + expr, + time.Minute, + labels.FromStrings("empty_label", ""), + nil, + nil, + true, log.NewNopLogger(), + ) + result := promql.Vector{ + { + Metric: labels.FromStrings( + "__name__", "ALERTS", + "alertname", "EmptyLabel", + "alertstate", "pending", + "instance", "0", + "job", "app-server", + ), + Point: promql.Point{V: 1}, + }, + } + + evalTime := time.Unix(0, 0) + result[0].Point.T = timestamp.FromTime(evalTime) + + var filteredRes promql.Vector // After removing 'ALERTS_FOR_STATE' samples. + res, err := rule.Eval( + suite.Context(), evalTime, EngineQueryFunc(suite.QueryEngine(), suite.Storage()), nil, + ) + testutil.Ok(t, err) + for _, smpl := range res { + smplName := smpl.Metric.Get("__name__") + if smplName == "ALERTS" { + filteredRes = append(filteredRes, smpl) + } else { + // If not 'ALERTS', it has to be 'ALERTS_FOR_STATE'. + testutil.Equals(t, smplName, "ALERTS_FOR_STATE") + } + } + testutil.Equals(t, result, filteredRes) +} diff --git a/rules/recording.go b/rules/recording.go index 36af49586..4e3ec635b 100644 --- a/rules/recording.go +++ b/rules/recording.go @@ -88,11 +88,7 @@ func (rule *RecordingRule) Eval(ctx context.Context, ts time.Time, query QueryFu lb.Set(labels.MetricName, rule.name) for _, l := range rule.labels { - if l.Value == "" { - lb.Del(l.Name) - } else { - lb.Set(l.Name, l.Value) - } + lb.Set(l.Name, l.Value) } sample.Metric = lb.Labels() diff --git a/scrape/scrape.go b/scrape/scrape.go index 0b2b1441d..80571ac14 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -449,20 +449,16 @@ func mutateSampleLabels(lset labels.Labels, target *Target, honor bool, rc []*re } } else { for _, l := range target.Labels() { - lv := lset.Get(l.Name) - if lv != "" { - lb.Set(model.ExportedLabelPrefix+l.Name, lv) - } + // existingValue will be empty if l.Name doesn't exist. + existingValue := lset.Get(l.Name) + // Because setting a label with an empty value is a no-op, + // this will only create the prefixed label if necessary. + lb.Set(model.ExportedLabelPrefix+l.Name, existingValue) + // It is now safe to set the target label. lb.Set(l.Name, l.Value) } } - for _, l := range lb.Labels() { - if l.Value == "" { - lb.Del(l.Name) - } - } - res := lb.Labels() if len(rc) > 0 { @@ -476,10 +472,7 @@ func mutateReportSampleLabels(lset labels.Labels, target *Target) labels.Labels lb := labels.NewBuilder(lset) for _, l := range target.Labels() { - lv := lset.Get(l.Name) - if lv != "" { - lb.Set(model.ExportedLabelPrefix+l.Name, lv) - } + lb.Set(model.ExportedLabelPrefix+l.Name, lset.Get(l.Name)) lb.Set(l.Name, l.Value) } diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 826d45732..2e2141761 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -85,7 +85,7 @@ func TestDroppedTargetsList(t *testing.T) { }, } sp, _ = newScrapePool(cfg, app, 0, nil) - expectedLabelSetString = "{__address__=\"127.0.0.1:9090\", __metrics_path__=\"\", __scheme__=\"\", job=\"dropMe\"}" + expectedLabelSetString = "{__address__=\"127.0.0.1:9090\", job=\"dropMe\"}" expectedLength = 1 ) sp.Sync(tgs)