diff --git a/.chloggen/fix-4074-initialize-scrape-target-labels.yaml b/.chloggen/fix-4074-initialize-scrape-target-labels.yaml new file mode 100644 index 0000000000..eaf6ad3ad7 --- /dev/null +++ b/.chloggen/fix-4074-initialize-scrape-target-labels.yaml @@ -0,0 +1,20 @@ +# 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. collector, target allocator, auto-instrumentation, opamp, 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: Initialize scrape target labels the same way Prometheus does to prevent hash inconsistencies. + +# One or more tracking issues related to the change +issues: [4074] + +# (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: | + Target labels are now populated with scrape config defaults (job, __metrics_path__, __scheme__, + __scrape_interval__, __scrape_timeout__) before hashing, matching Prometheus's PopulateDiscoveredLabels + behavior. This eliminates hash collisions for targets with the same address but different scrape + configurations. diff --git a/cmd/otel-allocator/internal/prehook/relabel.go b/cmd/otel-allocator/internal/prehook/relabel.go index 41a3f9ea7f..8901acadfd 100644 --- a/cmd/otel-allocator/internal/prehook/relabel.go +++ b/cmd/otel-allocator/internal/prehook/relabel.go @@ -48,7 +48,7 @@ func (tf *relabelConfigTargetFilter) Apply(targets []*target.Item) []*target.Ite if keepTarget { // Compute hash immediately while we have the builder, skipping meta labels. // This avoids materializing the filtered labels. - hash := target.HashFromBuilder(builder, tItem.JobName) + hash := target.HashFromBuilder(builder) targets[writeIndex] = target.NewItem( tItem.JobName, tItem.TargetURL, diff --git a/cmd/otel-allocator/internal/prehook/relabel_test.go b/cmd/otel-allocator/internal/prehook/relabel_test.go index e5eac4f25b..928d548d48 100644 --- a/cmd/otel-allocator/internal/prehook/relabel_test.go +++ b/cmd/otel-allocator/internal/prehook/relabel_test.go @@ -382,7 +382,7 @@ func MakeTargetFromProm(rCfgs []*relabel.Config, rawTarget *target.Item) (*targe } // Compute the hash from the builder, skipping meta labels - hash := target.HashFromBuilder(lb, rawTarget.JobName) + hash := target.HashFromBuilder(lb) newTarget := target.NewItem( rawTarget.JobName, rawTarget.TargetURL, diff --git a/cmd/otel-allocator/internal/server/testdata/collector_multiple.html b/cmd/otel-allocator/internal/server/testdata/collector_multiple.html index a63ff645e1..9bf715c37a 100644 --- a/cmd/otel-allocator/internal/server/testdata/collector_multiple.html +++ b/cmd/otel-allocator/internal/server/testdata/collector_multiple.html @@ -31,7 +31,7 @@

Collector: test-collector2

test-job - test-url + test-url diff --git a/cmd/otel-allocator/internal/server/testdata/collector_single.html b/cmd/otel-allocator/internal/server/testdata/collector_single.html index a63ff645e1..9bf715c37a 100644 --- a/cmd/otel-allocator/internal/server/testdata/collector_single.html +++ b/cmd/otel-allocator/internal/server/testdata/collector_single.html @@ -31,7 +31,7 @@

Collector: test-collector2

test-job - test-url + test-url diff --git a/cmd/otel-allocator/internal/server/testdata/index_multiple.html b/cmd/otel-allocator/internal/server/testdata/index_multiple.html index add01979d0..429f5ba034 100644 --- a/cmd/otel-allocator/internal/server/testdata/index_multiple.html +++ b/cmd/otel-allocator/internal/server/testdata/index_multiple.html @@ -44,7 +44,7 @@

OpenTelemetry Target Allocator

Targets - 3 + 2 @@ -76,10 +76,10 @@

OpenTelemetry Target Allocator

test-collector2 - 1 + 0 - 1 + 0 diff --git a/cmd/otel-allocator/internal/server/testdata/targets_multiple.html b/cmd/otel-allocator/internal/server/testdata/targets_multiple.html index 0aeb927043..4065d5ace6 100644 --- a/cmd/otel-allocator/internal/server/testdata/targets_multiple.html +++ b/cmd/otel-allocator/internal/server/testdata/targets_multiple.html @@ -31,10 +31,10 @@

OpenTelemetry Target Allocator - Targets

- test-job2 + test-job - test-url3 + test-url test-collector1 @@ -45,24 +45,10 @@

OpenTelemetry Target Allocator - Targets

- test-job - - - test-url2 - - - test-collector2 - - - - - - - - test-job + test-job2 - test-url + test-url3 test-collector1 diff --git a/cmd/otel-allocator/internal/server/testdata/targets_single.html b/cmd/otel-allocator/internal/server/testdata/targets_single.html index 466de24d48..5b38171198 100644 --- a/cmd/otel-allocator/internal/server/testdata/targets_single.html +++ b/cmd/otel-allocator/internal/server/testdata/targets_single.html @@ -34,7 +34,7 @@

OpenTelemetry Target Allocator - Targets

test-job - test-url + test-url test-collector1 diff --git a/cmd/otel-allocator/internal/target/discovery.go b/cmd/otel-allocator/internal/target/discovery.go index 6c86b1f38e..b35fc9ba54 100644 --- a/cmd/otel-allocator/internal/target/discovery.go +++ b/cmd/otel-allocator/internal/target/discovery.go @@ -7,8 +7,6 @@ import ( "context" "hash" "hash/fnv" - "maps" - "slices" "sync" "time" @@ -36,6 +34,7 @@ type Discoverer struct { close chan struct{} mtxScrape sync.Mutex // Guards the fields below. configsMap map[allocatorWatcher.EventSource][]*promconfig.ScrapeConfig + jobToScrapeConfig map[string]*promconfig.ScrapeConfig hook discoveryHook scrapeConfigsHash hash.Hash scrapeConfigsUpdater scrapeConfigsUpdater @@ -109,7 +108,7 @@ func (m *Discoverer) ApplyConfig(source allocatorWatcher.EventSource, scrapeConf // If the hash has changed, updated stored hash and send the new config. // Otherwise, skip updating scrape configs. if m.scrapeConfigsUpdater != nil && m.scrapeConfigsHash != hash { - err := m.scrapeConfigsUpdater.UpdateScrapeConfigResponse(jobToScrapeConfig) + err = m.scrapeConfigsUpdater.UpdateScrapeConfigResponse(jobToScrapeConfig) if err != nil { return err } @@ -120,7 +119,20 @@ func (m *Discoverer) ApplyConfig(source allocatorWatcher.EventSource, scrapeConf if m.hook != nil { m.hook.SetConfig(relabelCfg) } - return m.manager.ApplyConfig(discoveryCfg) + + err = m.manager.ApplyConfig(discoveryCfg) + if err != nil { + return err + } + + // Store scrape configs only after all operations succeed to avoid + // inconsistent state on partial failure. Guard with mtxScrape since + // Reload() reads this field concurrently under the same lock. + m.mtxScrape.Lock() + m.jobToScrapeConfig = jobToScrapeConfig + m.mtxScrape.Unlock() + + return nil } func (m *Discoverer) Run() error { @@ -184,14 +196,16 @@ func (m *Discoverer) Reload() { } targets := make([]*Item, targetCount) + jobToScrapeConfig := m.jobToScrapeConfig targetsAssigned := 0 for jobName, groups := range m.targetSets { wg.Add(1) + scrapeConfig := jobToScrapeConfig[jobName] // Run the sync in parallel as these take a while and at high load can't catch up. - go func(jobName string, groups []*targetgroup.Group, intoTargets []*Item) { - m.processTargetGroups(jobName, groups, intoTargets) + go func(jobName string, groups []*targetgroup.Group, intoTargets []*Item, cfg *promconfig.ScrapeConfig) { + m.processTargetGroups(jobName, groups, intoTargets, cfg) wg.Done() - }(jobName, groups, targets[targetsAssigned:]) + }(jobName, groups, targets[targetsAssigned:], scrapeConfig) for _, group := range groups { targetsAssigned += len(group.Targets) } @@ -201,13 +215,15 @@ func (m *Discoverer) Reload() { m.processTargetsCallBack(targets) } -// processTargetGroups processes the target groups and returns a map of targets. -func (m *Discoverer) processTargetGroups(jobName string, groups []*targetgroup.Group, intoTargets []*Item) { - // the builder for group labels +// processTargetGroups processes the target groups and populates labels the same way +// Prometheus does (via scrape.PopulateDiscoveredLabels) before creating target items. +// This ensures the label set includes scrape config defaults (job, metrics_path, +// scheme, scrape_interval, scrape_timeout) so that target hashes are consistent +// with what Prometheus computes. +func (m *Discoverer) processTargetGroups(jobName string, groups []*targetgroup.Group, intoTargets []*Item, cfg *promconfig.ScrapeConfig) { + lb := labels.NewBuilder(labels.EmptyLabels()) groupBuilder := labels.NewScratchBuilder(labelBuilderPreallocSize) - - // a slice for sorting target label names, we allocate it here to avoid doing it in the hot loop - targetLabelNames := make([]string, 0, labelBuilderPreallocSize) + defaults := newDiscoveredLabelDefaults(cfg) begin := time.Now() defer func() { @@ -216,32 +232,11 @@ func (m *Discoverer) processTargetGroups(jobName string, groups []*targetgroup.G var count float64 index := 0 for _, tg := range groups { - groupBuilder.Reset() - for ln, lv := range tg.Labels { - groupBuilder.Add(string(ln), string(lv)) - } - groupBuilder.Sort() + groupLabels := populateGroupLabels(&groupBuilder, tg.Labels) for _, t := range tg.Targets { count++ - // ScratchBuilder is a struct containing a slice of labels. By assigning to a new variable, we get a copy - // of the struct, with a new slice pointing to the same underlying array. As long as we don't mutate the - // original slice and only append to it, we can avoid copying the group labels. - targetBuilder := groupBuilder - targetLabelNames = targetLabelNames[:0] - - // We can't sort the whole builder slice, because that would modify the underlying groupBuilder. Instead, - // we sort the labels in a separate slice. As a result, the group labels and the target labels are sorted - // subslices of the builder slice, which is in itself not sorted. This is fine, as we don't care what the - // order of labels is - just that it's consistent, so the hash is always the same. - for ln := range maps.Keys(t) { - targetLabelNames = append(targetLabelNames, string(ln)) - } - slices.Sort(targetLabelNames) - for _, ln := range targetLabelNames { - lv := t[model.LabelName(ln)] - targetBuilder.Add(ln, string(lv)) - } - item := NewItem(jobName, string(t[model.AddressLabel]), targetBuilder.Labels(), "") + lset := populateDiscoveredLabels(lb, defaults, t, groupLabels) + item := NewItem(jobName, string(t[model.AddressLabel]), lset, "") intoTargets[index] = item index++ } @@ -249,6 +244,79 @@ func (m *Discoverer) processTargetGroups(jobName string, groups []*targetgroup.G m.targetsDiscovered.Record(context.Background(), count, metric.WithAttributes(attribute.String("job.name", jobName))) } +func populateGroupLabels(groupBuilder *labels.ScratchBuilder, tgLabels model.LabelSet) labels.Labels { + groupBuilder.Reset() + for ln, lv := range tgLabels { + if lv != "" { + groupBuilder.Add(string(ln), string(lv)) + } + } + groupBuilder.Sort() + return groupBuilder.Labels() +} + +type discoveredLabelDefaults struct { + jobName string + scrapeInterval string + scrapeTimeout string + metricsPath string + scheme string + params []labels.Label +} + +func newDiscoveredLabelDefaults(cfg *promconfig.ScrapeConfig) discoveredLabelDefaults { + defaults := discoveredLabelDefaults{ + jobName: cfg.JobName, + scrapeInterval: cfg.ScrapeInterval.String(), + scrapeTimeout: cfg.ScrapeTimeout.String(), + metricsPath: cfg.MetricsPath, + scheme: cfg.Scheme, + } + + if len(cfg.Params) > 0 { + defaults.params = make([]labels.Label, 0, len(cfg.Params)) + for k, v := range cfg.Params { + if len(v) > 0 { + defaults.params = append(defaults.params, labels.Label{ + Name: model.ParamLabelPrefix + k, + Value: v[0], + }) + } + } + } + + return defaults +} + +// populateDiscoveredLabels matches scrape.PopulateDiscoveredLabels. Keep this +// local hot-path helper in sync with Prometheus and covered by the comparison +// test, because calling the Prometheus helper directly repeats group-label and +// scrape-default work for every target. +func populateDiscoveredLabels(lb *labels.Builder, defaults discoveredLabelDefaults, tLabels model.LabelSet, groupLabels labels.Labels) labels.Labels { + lb.Reset(groupLabels) + for ln, lv := range tLabels { + lb.Set(string(ln), string(lv)) + } + + setDefaultLabel(lb, model.JobLabel, defaults.jobName) + setDefaultLabel(lb, model.ScrapeIntervalLabel, defaults.scrapeInterval) + setDefaultLabel(lb, model.ScrapeTimeoutLabel, defaults.scrapeTimeout) + setDefaultLabel(lb, model.MetricsPathLabel, defaults.metricsPath) + setDefaultLabel(lb, model.SchemeLabel, defaults.scheme) + + for _, param := range defaults.params { + setDefaultLabel(lb, param.Name, param.Value) + } + + return lb.Labels() +} + +func setDefaultLabel(lb *labels.Builder, name, value string) { + if lb.Get(name) == "" { + lb.Set(name, value) + } +} + // Run receives and saves target set updates and triggers the scraping loops reloading. // Reloading happens in the background so that it doesn't block receiving targets updates. func (m *Discoverer) run(tsets <-chan map[string][]*targetgroup.Group) error { diff --git a/cmd/otel-allocator/internal/target/discovery_test.go b/cmd/otel-allocator/internal/target/discovery_test.go index f0001e14bd..929c71b6da 100644 --- a/cmd/otel-allocator/internal/target/discovery_test.go +++ b/cmd/otel-allocator/internal/target/discovery_test.go @@ -6,7 +6,9 @@ package target import ( "context" "errors" + "fmt" "hash" + "net/url" "slices" "testing" "time" @@ -19,6 +21,7 @@ import ( "github.com/prometheus/prometheus/discovery/targetgroup" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/relabel" + "github.com/prometheus/prometheus/scrape" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ctrl "sigs.k8s.io/controller-runtime" @@ -497,15 +500,194 @@ func TestProcessTargetGroups_StableLabelIterationOrder(t *testing.T) { manager := discovery.NewManager(ctx, config.NopLogger, registry, sdMetrics) d, err := NewDiscoverer(ctrl.Log.WithName("test"), manager, nil, scu, nil) require.NoError(t, err) - d.processTargetGroups("test", groups, results) + d.processTargetGroups("test", groups, results, &promconfig.ScrapeConfig{JobName: "test"}) - i := 0 + // Verify all labels are in sorted order (the core invariant this test validates). + // scrape.PopulateDiscoveredLabels adds scrape config defaults (__scheme__, etc.) + // which interleave with the alphabetic test labels. + var prevName string results[0].Labels.Range(func(l labels.Label) { - expected := string(rune('a' + i)) - assert.Equal(t, expected, l.Name, "unexpected label key at index %d", i) - assert.Equal(t, expected, l.Value, "unexpected label value at index %d", i) - i++ + if prevName != "" { + assert.Less(t, prevName, l.Name, "labels should be sorted, but %q came after %q", l.Name, prevName) + } + prevName = l.Name }) + + // Verify that all original group and target labels are present. + for i := 'a'; i <= 'z'; i++ { + name := string(rune(i)) + assert.Equal(t, name, results[0].Labels.Get(name), "expected label %q to be present", name) + } +} + +func TestPopulateDiscoveredLabels(t *testing.T) { + tests := []struct { + description string + cfg *promconfig.ScrapeConfig + tLabels model.LabelSet + tgLabels model.LabelSet + wantLabels map[string]string + }{ + { + description: "scrape config defaults and params are applied", + cfg: &promconfig.ScrapeConfig{ + JobName: "my-job", + ScrapeInterval: model.Duration(30 * time.Second), + ScrapeTimeout: model.Duration(10 * time.Second), + MetricsPath: "/metrics", + Scheme: "http", + Params: url.Values{ + "module": []string{"http_2xx"}, + }, + }, + tLabels: model.LabelSet{"__address__": "localhost:9090"}, + tgLabels: model.LabelSet{}, + wantLabels: map[string]string{ + "__address__": "localhost:9090", + "job": "my-job", + "__scrape_interval__": "30s", + "__scrape_timeout__": "10s", + "__metrics_path__": "/metrics", + "__scheme__": "http", + "__param_module": "http_2xx", + }, + }, + { + description: "target labels override group labels", + cfg: &promconfig.ScrapeConfig{ + JobName: "job1", + MetricsPath: "/metrics", + Scheme: "http", + }, + tLabels: model.LabelSet{"__address__": "target-addr", "env": "target-env"}, + tgLabels: model.LabelSet{"env": "group-env", "region": "us-west"}, + wantLabels: map[string]string{ + "__address__": "target-addr", + "env": "target-env", + "region": "us-west", + "job": "job1", + }, + }, + { + description: "existing labels are not overridden by scrape config", + cfg: &promconfig.ScrapeConfig{ + JobName: "default-job", + MetricsPath: "/default-metrics", + Scheme: "https", + }, + tLabels: model.LabelSet{ + "__address__": "addr", + "job": "custom-job", + "__metrics_path__": "/custom-path", + }, + tgLabels: model.LabelSet{}, + wantLabels: map[string]string{ + "__address__": "addr", + "job": "custom-job", + "__metrics_path__": "/custom-path", + "__scheme__": "https", + }, + }, + } + + groupBuilder := labels.NewScratchBuilder(labelBuilderPreallocSize) + lb := labels.NewBuilder(labels.EmptyLabels()) + prometheusLabelsBuilder := labels.NewBuilder(labels.EmptyLabels()) + for _, tc := range tests { + t.Run(tc.description, func(t *testing.T) { + groupLabels := populateGroupLabels(&groupBuilder, tc.tgLabels) + lset := populateDiscoveredLabels(lb, newDiscoveredLabelDefaults(tc.cfg), tc.tLabels, groupLabels) + + scrape.PopulateDiscoveredLabels(prometheusLabelsBuilder, tc.cfg, tc.tLabels, tc.tgLabels) + prometheusLabels := prometheusLabelsBuilder.Labels() + assert.Equal(t, prometheusLabels, lset) + for k, v := range tc.wantLabels { + assert.Equal(t, v, lset.Get(k), "label %q mismatch", k) + } + }) + } +} + +func BenchmarkProcessTargetGroups(b *testing.B) { + tests := []struct { + name string + groupCount int + targetsPerGroup int + groupLabelCount int + targetLabelCount int + }{ + { + name: "1k_targets_10_labels", + groupCount: 10, + targetsPerGroup: 100, + groupLabelCount: 5, + targetLabelCount: 5, + }, + { + name: "10k_targets_20_labels", + groupCount: 100, + targetsPerGroup: 100, + groupLabelCount: 10, + targetLabelCount: 10, + }, + } + + scu := &mockScrapeConfigUpdater{} + ctx := context.Background() + registry := prometheus.NewRegistry() + sdMetrics, err := discovery.CreateAndRegisterSDMetrics(registry) + require.NoError(b, err) + manager := discovery.NewManager(ctx, config.NopLogger, registry, sdMetrics) + d, err := NewDiscoverer(ctrl.Log.WithName("benchmark"), manager, nil, scu, nil) + require.NoError(b, err) + + cfg := &promconfig.ScrapeConfig{ + JobName: "benchmark", + ScrapeInterval: model.Duration(30 * time.Second), + ScrapeTimeout: model.Duration(10 * time.Second), + MetricsPath: "/metrics", + Scheme: "http", + } + + for _, tt := range tests { + b.Run(tt.name, func(b *testing.B) { + groups := benchmarkTargetGroups(tt.groupCount, tt.targetsPerGroup, tt.groupLabelCount, tt.targetLabelCount) + results := make([]*Item, tt.groupCount*tt.targetsPerGroup) + + b.ReportAllocs() + b.ResetTimer() + for range b.N { + d.processTargetGroups("benchmark", groups, results, cfg) + } + }) + } +} + +func benchmarkTargetGroups(groupCount, targetsPerGroup, groupLabelCount, targetLabelCount int) []*targetgroup.Group { + groups := make([]*targetgroup.Group, groupCount) + for groupIndex := range groupCount { + groupLabels := make(model.LabelSet, groupLabelCount) + for labelIndex := range groupLabelCount { + groupLabels[model.LabelName(fmt.Sprintf("__meta_kubernetes_group_label_%02d", labelIndex))] = model.LabelValue(fmt.Sprintf("group-%d-%d", groupIndex, labelIndex)) + } + + targets := make([]model.LabelSet, targetsPerGroup) + for targetIndex := range targetsPerGroup { + targetLabels := make(model.LabelSet, targetLabelCount+1) + targetLabels[model.AddressLabel] = model.LabelValue(fmt.Sprintf("10.%d.%d.%d:9100", groupIndex%255, targetIndex/255, targetIndex%255)) + for labelIndex := range targetLabelCount { + targetLabels[model.LabelName(fmt.Sprintf("__meta_kubernetes_target_label_%02d", labelIndex))] = model.LabelValue(fmt.Sprintf("target-%d-%d-%d", groupIndex, targetIndex, labelIndex)) + } + targets[targetIndex] = targetLabels + } + + groups[groupIndex] = &targetgroup.Group{ + Targets: targets, + Labels: groupLabels, + Source: fmt.Sprintf("benchmark/%d", groupIndex), + } + } + return groups } func BenchmarkApplyScrapeConfig(b *testing.B) { diff --git a/cmd/otel-allocator/internal/target/target.go b/cmd/otel-allocator/internal/target/target.go index ae90dc35e3..7c6a46795e 100644 --- a/cmd/otel-allocator/internal/target/target.go +++ b/cmd/otel-allocator/internal/target/target.go @@ -4,7 +4,6 @@ package target import ( - "encoding/binary" "strconv" "strings" "sync" @@ -67,15 +66,18 @@ func WithHash(hash ItemHash) ItemOption { func (t *Item) Hash() ItemHash { if t.hash == 0 { - t.hash = ItemHash(LabelsHashWithJobName(t.Labels, t.JobName)) + // Labels already include job name and other scrape config defaults + // populated by scrape.PopulateDiscoveredLabels, so labels.Hash() is sufficient. + t.hash = ItemHash(t.Labels.Hash()) } return t.hash } // HashFromBuilder computes a hash from a labels.Builder, skipping meta labels. // This is used during relabeling to compute the hash efficiently without materializing -// the filtered labels. -func HashFromBuilder(builder *labels.Builder, jobName string) ItemHash { +// the filtered labels. Labels are expected to already include scrape config defaults +// (job, metrics_path, scheme, etc.) from scrape.PopulateDiscoveredLabels. +func HashFromBuilder(builder *labels.Builder) ItemHash { hash := hasherPool.Get().(*xxhash.Digest) hash.Reset() builder.Range(func(l labels.Label) { @@ -89,7 +91,6 @@ func HashFromBuilder(builder *labels.Builder, jobName string) ItemHash { _, _ = hash.WriteString(l.Value) _, _ = hash.Write(seps) }) - _, _ = hash.WriteString(jobName) result := hash.Sum64() hasherPool.Put(hash) return ItemHash(result) @@ -131,20 +132,3 @@ func NewItem(jobName, targetURL string, itemLabels labels.Labels, collectorName } return item } - -// LabelsHashWithJobName computes a hash of the labels and the job name. -// Same logic as Prometheus labels.Hash: https://github.com/prometheus/prometheus/blob/8fd46f74aa0155e4d5aa30654f9c02e564e03743/model/labels/labels.go#L72 -// but adds in the job name since this is not in the labelset from the discovery manager. -// The scrape manager adds it later. Address is already included in the labels, so it is not needed here. -func LabelsHashWithJobName(ls labels.Labels, jobName string) uint64 { - labelsHash := ls.Hash() - var labelsHashBytes [8]byte - binary.LittleEndian.PutUint64(labelsHashBytes[:], labelsHash) - hash := hasherPool.Get().(*xxhash.Digest) - hash.Reset() - _, _ = hash.Write(labelsHashBytes[:]) - _, _ = hash.WriteString(jobName) - result := hash.Sum64() - hasherPool.Put(hash) - return result -} diff --git a/cmd/otel-allocator/internal/target/target_test.go b/cmd/otel-allocator/internal/target/target_test.go index f5f9665432..5c93c2474c 100644 --- a/cmd/otel-allocator/internal/target/target_test.go +++ b/cmd/otel-allocator/internal/target/target_test.go @@ -73,11 +73,18 @@ func TestItemHashStability(t *testing.T) { } func TestItemHashDifferentJobs(t *testing.T) { - ls := labels.New(labels.Label{Name: "app", Value: "test"}) - item1 := NewItem("job-a", "http://10.0.0.1:8080", ls, "") - item2 := NewItem("job-b", "http://10.0.0.1:8080", ls, "") + ls1 := labels.New( + labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "job-a"}, + ) + ls2 := labels.New( + labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "job-b"}, + ) + item1 := NewItem("job-a", "http://10.0.0.1:8080", ls1, "") + item2 := NewItem("job-b", "http://10.0.0.1:8080", ls2, "") - // Different job names should produce different hashes + // Different job names in labels should produce different hashes assert.NotEqual(t, item1.Hash(), item2.Hash()) } @@ -201,48 +208,61 @@ func TestGetEndpointSliceName(t *testing.T) { } } -func TestLabelsHashWithJobName(t *testing.T) { - ls := labels.New( +func TestItemHashDifferentJobLabels(t *testing.T) { + // When labels include the job name (as populated by scrape.PopulateDiscoveredLabels), + // items with different jobs produce different hashes via labels.Hash(). + ls1 := labels.New( labels.Label{Name: "app", Value: "test"}, - labels.Label{Name: "env", Value: "prod"}, + labels.Label{Name: "job", Value: "job-a"}, + ) + ls2 := labels.New( + labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "job-b"}, ) - hash1 := LabelsHashWithJobName(ls, "job-a") - hash2 := LabelsHashWithJobName(ls, "job-a") - hash3 := LabelsHashWithJobName(ls, "job-b") + item1 := NewItem("job-a", "http://10.0.0.1:8080", ls1, "") + item2 := NewItem("job-b", "http://10.0.0.1:8080", ls2, "") - // Same inputs produce the same hash - assert.Equal(t, hash1, hash2) - // Different job names produce different hashes - assert.NotEqual(t, hash1, hash3) - // Hash is non-zero - assert.NotZero(t, hash1) + // Same labels except job name => different hashes + assert.NotEqual(t, item1.Hash(), item2.Hash()) + assert.NotZero(t, item1.Hash()) } func TestHashFromBuilder(t *testing.T) { ls := labels.New( labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "my-job"}, labels.Label{Name: "__meta_kubernetes_namespace", Value: "default"}, ) builder := labels.NewBuilder(ls) - hash := HashFromBuilder(builder, "my-job") + hash := HashFromBuilder(builder) - // Meta labels are skipped, so the hash should only consider "app=test" - lsNoMeta := labels.New(labels.Label{Name: "app", Value: "test"}) + // Meta labels are skipped, so the hash should only consider "app=test" + "job=my-job" + lsNoMeta := labels.New( + labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "my-job"}, + ) builderNoMeta := labels.NewBuilder(lsNoMeta) - hashNoMeta := HashFromBuilder(builderNoMeta, "my-job") + hashNoMeta := HashFromBuilder(builderNoMeta) assert.Equal(t, hash, hashNoMeta, "meta labels should be skipped in hash computation") assert.NotZero(t, hash) } func TestHashFromBuilderDifferentJobs(t *testing.T) { - ls := labels.New(labels.Label{Name: "app", Value: "test"}) - builder1 := labels.NewBuilder(ls) - builder2 := labels.NewBuilder(ls) + ls1 := labels.New( + labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "job-1"}, + ) + ls2 := labels.New( + labels.Label{Name: "app", Value: "test"}, + labels.Label{Name: "job", Value: "job-2"}, + ) + builder1 := labels.NewBuilder(ls1) + builder2 := labels.NewBuilder(ls2) - hash1 := HashFromBuilder(builder1, "job-1") - hash2 := HashFromBuilder(builder2, "job-2") + hash1 := HashFromBuilder(builder1) + hash2 := HashFromBuilder(builder2) assert.NotEqual(t, hash1, hash2) } diff --git a/go.mod b/go.mod index 7817e04890..f0c9f44526 100644 --- a/go.mod +++ b/go.mod @@ -272,6 +272,7 @@ require ( github.com/stackitcloud/stackit-sdk-go/core v0.23.0 // indirect github.com/vultr/govultr/v3 v3.28.1 // indirect go.mongodb.org/mongo-driver/v2 v2.5.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.67.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.19.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.19.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.43.0 // indirect diff --git a/go.sum b/go.sum index 712594e5ed..de768ffc63 100644 --- a/go.sum +++ b/go.sum @@ -624,6 +624,8 @@ go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/collector/featuregate v1.57.0 h1:KPDSUKYn6MHwgyGRSGPPcW/G96HH93pxuvvPwM+R8nY= go.opentelemetry.io/collector/featuregate v1.57.0/go.mod h1:4ga1QBMPEejXXmpyJS8lmaRpknJ3Lb9Bvk6e420bUFU= +go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.67.0 h1:c9r/G1CSw4dPI1jaNNG9RnQP+q4SvZnHciDQJVIvchU= +go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace v0.67.0/go.mod h1:gO9smoZe9KnZcJCqcB0lMmQ4Z5VEifYmjMTpnwtTSuQ= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.68.0 h1:CqXxU8VOmDefoh0+ztfGaymYbhdB/tT3zs79QaZTNGY= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.68.0/go.mod h1:BuhAPThV8PBHBvg8ZzZ/Ok3idOdhWIodywz2xEcRbJo= go.opentelemetry.io/contrib/otelconf v0.23.0 h1:s3C7KdMYiutf4rC8hKFA0WOIDG+gIru8ajjQKS59ir8=