From 557c67f5a9f7ec14f0b5ef7e11f90b1033f6cf5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= Date: Wed, 27 Sep 2023 13:39:30 +0200 Subject: [PATCH] Rebuild targets on scrape config regex-only changes --- .chloggen/fix_ta-regex-hashing.yaml | 16 ++++++ cmd/otel-allocator/target/discovery.go | 29 ++++++++-- cmd/otel-allocator/target/discovery_test.go | 59 ++++++++++----------- 3 files changed, 70 insertions(+), 34 deletions(-) create mode 100755 .chloggen/fix_ta-regex-hashing.yaml diff --git a/.chloggen/fix_ta-regex-hashing.yaml b/.chloggen/fix_ta-regex-hashing.yaml new file mode 100755 index 0000000000..1e9d3ced16 --- /dev/null +++ b/.chloggen/fix_ta-regex-hashing.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action) +component: target allocator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Rebuild targets on scrape config regex-only changes + +# One or more tracking issues related to the change +issues: [1358,1926] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/cmd/otel-allocator/target/discovery.go b/cmd/otel-allocator/target/discovery.go index 70cf82a374..838832a764 100644 --- a/cmd/otel-allocator/target/discovery.go +++ b/cmd/otel-allocator/target/discovery.go @@ -15,14 +15,17 @@ package target import ( + "hash" + "hash/fnv" + "github.com/go-logr/logr" - "github.com/mitchellh/hashstructure" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/discovery" "github.com/prometheus/prometheus/model/relabel" + "gopkg.in/yaml.v3" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) @@ -40,7 +43,7 @@ type Discoverer struct { close chan struct{} configsMap map[allocatorWatcher.EventSource]*config.Config hook discoveryHook - scrapeConfigsHash uint64 + scrapeConfigsHash hash.Hash scrapeConfigsUpdater scrapeConfigsUpdater } @@ -82,7 +85,7 @@ func (m *Discoverer) ApplyConfig(source allocatorWatcher.EventSource, cfg *confi } } - hash, err := hashstructure.Hash(jobToScrapeConfig, nil) + hash, err := getScrapeConfigHash(jobToScrapeConfig) if err != nil { return err } @@ -131,3 +134,23 @@ func (m *Discoverer) Watch(fn func(targets map[string]*Item)) error { func (m *Discoverer) Close() { close(m.close) } + +// Calculate a hash for a scrape config map. +// This is done by marshalling to YAML because it's the most straightforward and doesn't run into problems with unexported fields. +func getScrapeConfigHash(jobToScrapeConfig map[string]*config.ScrapeConfig) (hash.Hash64, error) { + var err error + hash := fnv.New64() + yamlEncoder := yaml.NewEncoder(hash) + for jobName, scrapeConfig := range jobToScrapeConfig { + _, err = hash.Write([]byte(jobName)) + if err != nil { + return nil, err + } + err = yamlEncoder.Encode(scrapeConfig) + if err != nil { + return nil, err + } + } + yamlEncoder.Close() + return hash, err +} diff --git a/cmd/otel-allocator/target/discovery_test.go b/cmd/otel-allocator/target/discovery_test.go index 25a44b6411..1391595d0d 100644 --- a/cmd/otel-allocator/target/discovery_test.go +++ b/cmd/otel-allocator/target/discovery_test.go @@ -17,6 +17,7 @@ package target import ( "context" "errors" + "hash" "sort" "testing" "time" @@ -263,39 +264,35 @@ func TestDiscovery_ScrapeConfigHashing(t *testing.T) { }, expectErr: true, }, - // { - // TODO: fix handler logic so this test passes. - // This test currently fails due to the regexp struct not having any - // exported fields for the hashing algorithm to hash on, causing the - // hashes to be the same even though the data is different. - // description: "different regex", - // cfg: &promconfig.Config{ - // ScrapeConfigs: []*promconfig.ScrapeConfig{ - // { - // JobName: "serviceMonitor/testapp/testapp/1", - // HonorTimestamps: false, - // ScrapeTimeout: model.Duration(30 * time.Second), - // MetricsPath: "/metrics", - // HTTPClientConfig: commonconfig.HTTPClientConfig{ - // FollowRedirects: true, - // }, - // RelabelConfigs: []*relabel.Config{ - // { - // SourceLabels: model.LabelNames{model.LabelName("job")}, - // Separator: ";", - // Regex: relabel.MustNewRegexp("something else"), - // TargetLabel: "__tmp_prometheus_job_name", - // Replacement: "$$1", - // Action: relabel.Replace, - // }, - // }, - // }, - // }, - // }, - // }, + { + description: "different regex", + cfg: &promconfig.Config{ + ScrapeConfigs: []*promconfig.ScrapeConfig{ + { + JobName: "serviceMonitor/testapp/testapp/1", + HonorTimestamps: false, + ScrapeTimeout: model.Duration(30 * time.Second), + MetricsPath: "/metrics", + HTTPClientConfig: commonconfig.HTTPClientConfig{ + FollowRedirects: true, + }, + RelabelConfigs: []*relabel.Config{ + { + SourceLabels: model.LabelNames{model.LabelName("job")}, + Separator: ";", + Regex: relabel.MustNewRegexp("something else"), + TargetLabel: "__tmp_prometheus_job_name", + Replacement: "$$1", + Action: relabel.Replace, + }, + }, + }, + }, + }, + }, } var ( - lastValidHash uint64 + lastValidHash hash.Hash expectedConfig map[string]*promconfig.ScrapeConfig lastValidConfig map[string]*promconfig.ScrapeConfig )