support sharding for probe #5735
Conversation
pkg/prometheus/promcfg.go
Outdated
| @@ -957,6 +957,7 @@ func (cg *ConfigGenerator) generateProbeConfig( | |||
| // Add configured relabelings. | |||
| xc := labeler.GetRelabelingConfigs(m.TypeMeta, m.ObjectMeta, m.Spec.Targets.StaticConfig.RelabelConfigs) | |||
| relabelings = append(relabelings, generateRelabelConfig(xc)...) | |||
| relabelings = generateAddressShardingRelabelingRulesForProbes(relabelings, shards) | |||
There was a problem hiding this comment.
Do we want to configure sharing on every occasion? Or only when sharding is enabled in Prometheus?
There was a problem hiding this comment.
For simplicity, we add the sharding relabel config in all cases (e.g. whether the number of shards is greater than 1 or not).
Can we move this line and the next one outside of the switch case since it applies to all cases (static an ingress targets)?
There was a problem hiding this comment.
thanks for the suggestion. this is the change e4ded8d
pkg/prometheus/promcfg.go
Outdated
| @@ -909,6 +909,7 @@ func (cg *ConfigGenerator) generateProbeConfig( | |||
| }, | |||
| }...) | |||
| } | |||
| relabelings = generateAddressShardingRelabelingRulesForProbes(relabelings, shards) | |||
There was a problem hiding this comment.
I'd keep it as the last relabeling config.
There was a problem hiding this comment.
@steven05jiang yes I think that it makes sense to do so.
There was a problem hiding this comment.
please review the change a634807. Thanks.
pkg/prometheus/promcfg.go
Outdated
|
|
||
| if m.Spec.Targets.StaticConfig != nil || m.Spec.Targets.Ingress != nil { |
There was a problem hiding this comment.
I don't think that the if statement is required
| if m.Spec.Targets.StaticConfig != nil || m.Spec.Targets.Ingress != nil { |
There was a problem hiding this comment.
just wanted to confirm before removing it - after removing the condition, it will add relabeling configs (job name and sharding) to empty_probe, prober_spec, and module_config in this test. while I'm not familiar with all use cases of probe, would it raise any compatibility issue in this case? Thanks.
There was a problem hiding this comment.
This is a good remark. The operator already takes care of filtering out probes without static/ingress targets here. But even if the check doesn't work, it shouldn't be a problem to include the relabeling configs.
There was a problem hiding this comment.
tl;dr: my recommendation should be ok and updating the tests is fine ;)
There was a problem hiding this comment.
Sounds good. This is the update ffb2a35
There was a problem hiding this comment.
good job @steven05jiang and @simonpasquier!
sorry for not being very useful here 😅
|
Thanks all for the review! |
Description
fix #5176.
By adding a sharding relabeling rule, prometheus operator will shard targets in the probe across the prometheus cluster.
Type of change
What type of changes does your code introduce to the Prometheus operator? Put an
xin the box that apply.CHANGE(fix or feature that would cause existing functionality to not work as expected)FEATURE(non-breaking change which adds functionality)BUGFIX(non-breaking change which fixes an issue)ENHANCEMENT(non-breaking change which improves existing functionality)NONE(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Changelog entry
Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.