[receiver/prometheus] syncTargetAllocator now detects regex changes in relabel config #32127
Conversation
|
@rashmichandrashekar yeah, in my opinion we should do something very similar to what the target allocator does and just serialize the configs to yaml, and hash the output. They should be sorted, to avoid map ordering inconsistencies. |
| @@ -145,20 +147,47 @@ func (r *pReceiver) startTargetAllocator(allocConf *TargetAllocator, baseCfg *Pr | |||
| return nil | |||
| } | |||
|
|
|||
| // Calculate a hash for a scrape config map. | |||
| // This is done by marshaling 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) { | |||
There was a problem hiding this comment.
Could we add some simple unit tests for this function? There was concern about map iteration order inconsistency, so we should at least check if this function ran on two copies of the same config gives the same result.
There was a problem hiding this comment.
Thanks @swiatekm. Added one. Please take a look.
| @@ -532,6 +644,17 @@ func TestTargetAllocatorJobRetrieval(t *testing.T) { | |||
| // which is identical to the source url | |||
| s.Labels["__meta_url"] = model.LabelValue(sdConfig.URL) | |||
| require.Equal(t, s.Labels, group.Labels) | |||
| if s.MetricRelabelConfig != nil { | |||
| // Adding wait here so that the latest scrape config is applied with the updated regex | |||
| time.Sleep(5 * time.Second) | |||
There was a problem hiding this comment.
If we do need to wait here, could we use assert.Eventually instead?
There was a problem hiding this comment.
@swiatekm - This is not needed. Removed it.
|
Ack. I'm OK with the general approach now. I'll try to review soon |
dashpole
added
the
ready to merge
jpkrohling
changed the title
Description:
Fixing a bug - With the Targetallocator component in prometheus receiver, when prometheus scrape config is updated for metric relabel config with just regex change, the prometheus metrics receiver doesn't update the hash and hence doesn't pick up the metrics relabel config with the new regex.
This is because the regex struct has unexported fields. This fix similar to the fix made in opentelemetry-operator fixes this issue by taking into account the unexported fields as well.
Link to tracking Issue: - #29313
Testing: Tested to make sure that just the regex changes in metric relabeling gets picked up with TargetAllocator enabled.
Reopening this PR since the old one(#30258) got closed due to inactivity.