PO to GMP Migration Tool: Podmonitor Endpoint, Target Labels, and Metric Relabelings#1976
PO to GMP Migration Tool: Podmonitor Endpoint, Target Labels, and Metric Relabelings#1976karthunni wants to merge 5 commits into
Conversation
feat: map basic 1:1 endpoint settings
There was a problem hiding this comment.
Code Review
This pull request implements the migration logic for converting Prometheus Operator PodMonitor resources to Google Cloud Managed Service for Prometheus (GMP) PodMonitoring and ClusterPodMonitoring resources, including endpoint mapping, protected label handling, and metric relabeling rules. The review feedback suggests unconditionally parsing and validating the scrape interval to catch invalid values early, even when the scrape timeout is not specified.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements the conversion of endpoints, target labels, and metric relabeling configurations from Prometheus Operator's PodMonitor to Google Cloud Managed Service for Prometheus (GMP) PodMonitoring and ClusterPodMonitoring resources. It also introduces warnings for unsupported fields and adds comprehensive unit tests. A critical issue was identified in the relabeling conversion logic where the 'keepequal' and 'dropequal' actions are incorrectly subjected to protected label renaming, which would break their comparison logic.
| } | ||
|
|
||
| targetLabel := config.TargetLabel | ||
| if action == "replace" || action == "hashmod" || action == "lowercase" || action == "uppercase" || action == "keepequal" || action == "dropequal" || action == "" { |
There was a problem hiding this comment.
The keepequal and dropequal relabeling actions only perform equality checks (reading the label value) rather than writing to or overwriting the target label. Renaming target_label to exported_<label> for these actions will break the comparison logic because it will compare against a non-existent or differently-valued exported_ label instead of the actual protected label on the target. These actions should be excluded from the protected label renaming logic.
| if action == "replace" || action == "hashmod" || action == "lowercase" || action == "uppercase" || action == "keepequal" || action == "dropequal" || action == "" { | |
| if action == "replace" || action == "hashmod" || action == "lowercase" || action == "uppercase" || action == "" { |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements the conversion of Prometheus Operator PodMonitor resources to Google Cloud Managed Service for Prometheus (GMP) PodMonitoring and ClusterPodMonitoring resources. This includes mapping endpoints, scrape intervals, timeouts, target labels, and metric relabeling rules, while warning about unsupported fields and handling protected labels. Feedback on the changes suggests deduplicating the generated target label mappings to prevent potential Kubernetes validation failures when duplicate labels are present in the input.
| func (c *PodMonitorConverter) convertFromPodLabels(logger *slog.Logger, pm *pomonitoringv1.PodMonitor) []monitoringv1.LabelMapping { | ||
| var fromPod []monitoringv1.LabelMapping | ||
|
|
||
| for _, l := range pm.Spec.PodTargetLabels { | ||
| mapping := monitoringv1.LabelMapping{From: l} | ||
| if protectedLabels[l] { | ||
| mapping.To = "exported_" + l | ||
| logger.Warn(fmt.Sprintf("Pod target label %q is protected in GMP. Renamed target to %q.", l, mapping.To)) | ||
| } | ||
| fromPod = append(fromPod, mapping) | ||
| } | ||
|
|
||
| if pm.Spec.JobLabel != "" { | ||
| logger.Warn(fmt.Sprintf("GMP does not support overriding the protected 'job' label. Value on label %q has been copied into the target label 'exported_job'.", pm.Spec.JobLabel)) | ||
| fromPod = append(fromPod, monitoringv1.LabelMapping{ | ||
| From: pm.Spec.JobLabel, | ||
| To: "exported_job", | ||
| }) | ||
| } | ||
|
|
||
| return fromPod | ||
| } |
There was a problem hiding this comment.
If pm.Spec.PodTargetLabels contains duplicate labels, or if pm.Spec.JobLabel matches a label already present in pm.Spec.PodTargetLabels (or maps to the same target label), duplicate target label mappings will be generated in the output TargetLabels spec. This can cause validation failures or unexpected behavior when applying the generated resource to Kubernetes.
We should deduplicate the generated mappings based on the target label name.
func (c *PodMonitorConverter) convertFromPodLabels(logger *slog.Logger, pm *pomonitoringv1.PodMonitor) []monitoringv1.LabelMapping {
var fromPod []monitoringv1.LabelMapping
seenTargets := make(map[string]bool)
for _, l := range pm.Spec.PodTargetLabels {
mapping := monitoringv1.LabelMapping{From: l}
target := l
if protectedLabels[l] {
mapping.To = "exported_" + l
target = mapping.To
logger.Warn(fmt.Sprintf("Pod target label %q is protected in GMP. Renamed target to %q.", l, mapping.To))
}
if seenTargets[target] {
continue
}
seenTargets[target] = true
fromPod = append(fromPod, mapping)
}
if pm.Spec.JobLabel != "" {
target := "exported_job"
if !seenTargets[target] {
logger.Warn(fmt.Sprintf("GMP does not support overriding the protected 'job' label. Value on label %q has been copied into the target label 'exported_job'.", pm.Spec.JobLabel))
fromPod = append(fromPod, monitoringv1.LabelMapping{
From: pm.Spec.JobLabel,
To: target,
})
seenTargets[target] = true
}
}
return fromPod
}
Features added:
instancetoexported_instanceandjobtoexported_job).labelmap, and preventing writes to protected target labels.