From 2fde2fb37d257f085d7abd08c1de6eda82383409 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 7 Mar 2023 17:10:15 +0000 Subject: [PATCH 1/3] scrape: add Target.LabelsRange This allows users of a Target to iterate labels without allocating heap memory. Signed-off-by: Bryan Boreham --- scrape/target.go | 9 +++++++++ scrape/target_test.go | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/scrape/target.go b/scrape/target.go index ae952b420..59f6e2873 100644 --- a/scrape/target.go +++ b/scrape/target.go @@ -181,6 +181,15 @@ func (t *Target) Labels() labels.Labels { return b.Labels() } +// LabelsRange calls f on each public label of the target. +func (t *Target) LabelsRange(f func(l labels.Label)) { + t.labels.Range(func(l labels.Label) { + if !strings.HasPrefix(l.Name, model.ReservedLabelPrefix) { + f(l) + } + }) +} + // DiscoveredLabels returns a copy of the target's labels before any processing. func (t *Target) DiscoveredLabels() labels.Labels { t.mtx.Lock() diff --git a/scrape/target_test.go b/scrape/target_test.go index 991195f5b..4937359ed 100644 --- a/scrape/target_test.go +++ b/scrape/target_test.go @@ -43,6 +43,17 @@ func TestTargetLabels(t *testing.T) { want := labels.FromStrings(model.JobLabel, "some_job", "foo", "bar") got := target.Labels() require.Equal(t, want, got) + i := 0 + target.LabelsRange(func(l labels.Label) { + switch i { + case 0: + require.Equal(t, labels.Label{Name: "foo", Value: "bar"}, l) + case 1: + require.Equal(t, labels.Label{Name: model.JobLabel, Value: "some_job"}, l) + } + i++ + }) + require.Equal(t, 2, i) } func TestTargetOffset(t *testing.T) { From 0dfa1e73f8c248fa19282b56b5c8fdf58b44caf4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 7 Mar 2023 17:11:24 +0000 Subject: [PATCH 2/3] scrape: use LabelsRange instead of Labels, for performance Includes a rewrite of `resolveConflictingExposedLabels` to use `labels.Builder.Get`, which simplifies it considerably. Signed-off-by: Bryan Boreham --- scrape/scrape.go | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index 3fce6f9dd..5c71a0110 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -500,7 +500,10 @@ func (sp *scrapePool) Sync(tgs []*targetgroup.Group) { } targetSyncFailed.WithLabelValues(sp.config.JobName).Add(float64(len(failures))) for _, t := range targets { - if !t.Labels().IsEmpty() { + // Replicate .Labels().IsEmpty() with a loop here to avoid generating garbage. + nonEmpty := false + t.LabelsRange(func(l labels.Label) { nonEmpty = true }) + if nonEmpty { all = append(all, t) } else if !t.DiscoveredLabels().IsEmpty() { sp.droppedTargets = append(sp.droppedTargets, t) @@ -666,17 +669,16 @@ func verifyLabelLimits(lset labels.Labels, limits *labelLimits) error { func mutateSampleLabels(lset labels.Labels, target *Target, honor bool, rc []*relabel.Config) labels.Labels { lb := labels.NewBuilder(lset) - targetLabels := target.Labels() if honor { - targetLabels.Range(func(l labels.Label) { + target.LabelsRange(func(l labels.Label) { if !lset.Has(l.Name) { lb.Set(l.Name, l.Value) } }) } else { var conflictingExposedLabels []labels.Label - targetLabels.Range(func(l labels.Label) { + target.LabelsRange(func(l labels.Label) { existingValue := lset.Get(l.Name) if existingValue != "" { conflictingExposedLabels = append(conflictingExposedLabels, labels.Label{Name: l.Name, Value: existingValue}) @@ -686,7 +688,7 @@ func mutateSampleLabels(lset labels.Labels, target *Target, honor bool, rc []*re }) if len(conflictingExposedLabels) > 0 { - resolveConflictingExposedLabels(lb, lset, targetLabels, conflictingExposedLabels) + resolveConflictingExposedLabels(lb, conflictingExposedLabels) } } @@ -699,42 +701,27 @@ func mutateSampleLabels(lset labels.Labels, target *Target, honor bool, rc []*re return res } -func resolveConflictingExposedLabels(lb *labels.Builder, exposedLabels, targetLabels labels.Labels, conflictingExposedLabels []labels.Label) { +func resolveConflictingExposedLabels(lb *labels.Builder, conflictingExposedLabels []labels.Label) { sort.SliceStable(conflictingExposedLabels, func(i, j int) bool { return len(conflictingExposedLabels[i].Name) < len(conflictingExposedLabels[j].Name) }) - for i, l := range conflictingExposedLabels { + for _, l := range conflictingExposedLabels { newName := l.Name for { newName = model.ExportedLabelPrefix + newName - if !exposedLabels.Has(newName) && - !targetLabels.Has(newName) && - !labelSliceHas(conflictingExposedLabels[:i], newName) { - conflictingExposedLabels[i].Name = newName + if lb.Get(newName) == "" { + lb.Set(newName, l.Value) break } } } - - for _, l := range conflictingExposedLabels { - lb.Set(l.Name, l.Value) - } -} - -func labelSliceHas(lbls []labels.Label, name string) bool { - for _, l := range lbls { - if l.Name == name { - return true - } - } - return false } func mutateReportSampleLabels(lset labels.Labels, target *Target) labels.Labels { lb := labels.NewBuilder(lset) - target.Labels().Range(func(l labels.Label) { + target.LabelsRange(func(l labels.Label) { lb.Set(model.ExportedLabelPrefix+l.Name, lset.Get(l.Name)) lb.Set(l.Name, l.Value) }) From 0c09c3feb021a45a87bd99dfc44ccadfa14deb21 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 7 Mar 2023 17:17:49 +0000 Subject: [PATCH 3/3] scrape sync: avoid copy of labels for dropped targets Since the Target object was just created in this function, nobody else has a reference to it and there are no concerns about it being modified concurrently so we don't need to copy the value. Signed-off-by: Bryan Boreham --- scrape/scrape.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrape/scrape.go b/scrape/scrape.go index 5c71a0110..01c66ca81 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -505,7 +505,7 @@ func (sp *scrapePool) Sync(tgs []*targetgroup.Group) { t.LabelsRange(func(l labels.Label) { nonEmpty = true }) if nonEmpty { all = append(all, t) - } else if !t.DiscoveredLabels().IsEmpty() { + } else if !t.discoveredLabels.IsEmpty() { sp.droppedTargets = append(sp.droppedTargets, t) } }