fix(scrape): use HonorLabels instead of HonorTimestamps in newScrapeLoop (#17731)
Some checks are pending
buf.build / lint and publish (push) Waiting to run
CI / Go tests (push) Waiting to run
CI / More Go tests (push) Waiting to run
CI / Go tests with previous Go version (push) Waiting to run
CI / UI tests (push) Waiting to run
CI / Go tests on Windows (push) Waiting to run
CI / Mixins tests (push) Waiting to run
CI / Build Prometheus for common architectures (push) Waiting to run
CI / Build Prometheus for all architectures (push) Waiting to run
CI / Report status of build Prometheus for all architectures (push) Blocked by required conditions
CI / Check generated parser (push) Waiting to run
CI / golangci-lint (push) Waiting to run
CI / fuzzing (push) Waiting to run
CI / codeql (push) Waiting to run
CI / Publish main branch artifacts (push) Blocked by required conditions
CI / Publish release artefacts (push) Blocked by required conditions
CI / Publish UI on npm Registry (push) Blocked by required conditions
Scorecards supply-chain security / Scorecards analysis (push) Waiting to run

* fix(scrape): use HonorLabels instead of HonorTimestamps in newScrapeLoop

The sampleMutator closure in newScrapeLoop was incorrectly passing
HonorTimestamps to mutateSampleLabels instead of HonorLabels. This
caused honor_labels configuration to be ignored, with the behavior
incorrectly controlled by honor_timestamps instead.

Adding TestNewScrapeLoopHonorLabelsWiring integration test that exercises
the real newScrapeLoop constructor with HonorLabels and HonorTimestamps
set to opposite values to catch this class of wiring bug.

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Update scrape/scrape_test.go

Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Add honor_labels=false test case

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

---------

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
This commit is contained in:
Arve Knudsen 2025-12-22 16:28:08 +01:00 committed by GitHub
parent 17e06dbab5
commit f0dfb9f802
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 80 additions and 1 deletions

View file

@ -1182,7 +1182,7 @@ func newScrapeLoop(opts scrapeLoopOptions) *scrapeLoop {
interval: opts.interval,
timeout: opts.timeout,
sampleMutator: func(l labels.Labels) labels.Labels {
return mutateSampleLabels(l, opts.target, opts.sp.config.HonorTimestamps, opts.sp.config.MetricRelabelConfigs)
return mutateSampleLabels(l, opts.target, opts.sp.config.HonorLabels, opts.sp.config.MetricRelabelConfigs)
},
reportSampleMutator: func(l labels.Labels) labels.Labels { return mutateReportSampleLabels(l, opts.target) },
scraper: opts.scraper,

View file

@ -5853,3 +5853,82 @@ func BenchmarkScrapePoolRestartLoops(b *testing.B) {
sp.restartLoops(true)
}
}
// TestNewScrapeLoopHonorLabelsWiring verifies that newScrapeLoop correctly wires
// HonorLabels (not HonorTimestamps) to the sampleMutator.
func TestNewScrapeLoopHonorLabelsWiring(t *testing.T) {
// Scraped metric has label "lbl" with value "scraped".
// Discovery target has label "lbl" with value "discovery".
// With honor_labels=true, the scraped value should win.
// With honor_labels=false, the discovery value should win and scraped moves to exported_lbl.
testCases := []struct {
name string
honorLabels bool
expectedLbl string
expectedExpLbl string // exported_lbl value, empty if not expected
}{
{
name: "honor_labels=true",
honorLabels: true,
expectedLbl: "scraped",
},
{
name: "honor_labels=false",
honorLabels: false,
expectedLbl: "discovery",
expectedExpLbl: "scraped",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ts, scrapedTwice := newScrapableServer(`metric{lbl="scraped"} 1`)
defer ts.Close()
testURL, err := url.Parse(ts.URL)
require.NoError(t, err)
s := teststorage.New(t)
defer s.Close()
cfg := &config.ScrapeConfig{
JobName: "test",
Scheme: "http",
HonorLabels: tc.honorLabels,
HonorTimestamps: !tc.honorLabels, // Opposite of HonorLabels to catch wiring bugs
ScrapeInterval: model.Duration(1 * time.Second),
ScrapeTimeout: model.Duration(100 * time.Millisecond),
MetricNameValidationScheme: model.UTF8Validation,
}
sp, err := newScrapePool(cfg, s, 0, nil, nil, &Options{skipOffsetting: true}, newTestScrapeMetrics(t))
require.NoError(t, err)
defer sp.stop()
// Sync with a target that has a conflicting label.
sp.Sync([]*targetgroup.Group{{
Targets: []model.LabelSet{{
model.AddressLabel: model.LabelValue(testURL.Host),
"lbl": "discovery",
}},
}})
require.Len(t, sp.ActiveTargets(), 1)
// Wait for scrape to complete.
select {
case <-time.After(5 * time.Second):
t.Fatal("scrape did not complete in time")
case <-scrapedTwice:
}
// Query the storage to verify label values.
q, err := s.Querier(time.Time{}.UnixNano(), time.Now().UnixNano())
require.NoError(t, err)
defer q.Close()
series := q.Select(t.Context(), false, nil, labels.MustNewMatcher(labels.MatchEqual, "__name__", "metric"))
require.True(t, series.Next(), "metric series not found")
require.Equal(t, tc.expectedLbl, series.At().Labels().Get("lbl"))
require.Equal(t, tc.expectedExpLbl, series.At().Labels().Get("exported_lbl"))
})
}
}