Copilot commented on code in PR #889: URL: https://github.com/apache/solr-operator/pull/889#discussion_r4508107596
########## helm/solr-operator/templates/podmonitor.yaml: ########## @@ -0,0 +1,43 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +{{- if and .Values.metrics.enable .Values.prometheus.podMonitor.enable }} +apiVersion: monitoring.coreos.com/v1 +kind: PodMonitor +metadata: + name: {{ include "solr-operator.fullname" . }} + namespace: {{ .Release.Namespace }} + labels: + control-plane: solr-operator + {{- with .Values.prometheus.podMonitor.labels }} + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + selector: + matchLabels: + control-plane: solr-operator + namespaceSelector: + matchNames: + - {{ .Release.Namespace }} + podMetricsEndpoints: + - port: metrics + path: /metrics Review Comment: The PodMonitor defaults to `honorLabels: false`, and Prometheus Operator adds a target `namespace` label containing the operator pod's namespace. This conflicts with the resource `namespace` labels emitted by these metrics: watched namespaces are renamed to `exported_namespace`, so the documented namespace filters and aggregations use the operator's namespace instead and can combine unrelated resources. Set `honorLabels: true` on this endpoint to preserve the resource labels. ########## controllers/resource_metrics.go: ########## @@ -0,0 +1,263 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package controllers + +import ( + "context" + "time" + + solrv1beta1 "github.com/apache/solr-operator/api/v1beta1" + "github.com/apache/solr-operator/controllers/metrics" + "github.com/go-logr/logr" + "github.com/prometheus/client_golang/prometheus" + appsv1 "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +// How long a scrape may spend reading the cache. Reads from a synced cache take microseconds, this only matters +// while the cache is still syncing after startup. +const resourceMetricsTimeout = 10 * time.Second + +// Label values of solr_operator_solrbackup_state +const ( + backupStateRunning = "running" + backupStateSucceeded = "succeeded" + backupStateFailed = "failed" +) + +// Label values of solr_operator_solrcloud_cluster_operations +const ( + clusterOpStateRunning = "running" + clusterOpStateQueued = "queued" +) + +var ( + managedResourcesDesc = prometheus.NewDesc(metrics.Namespace+"_managed_resources", + "Number of Solr resources managed by the operator, by kind (SolrCloud, SolrBackup, SolrPrometheusExporter) and namespace.", + []string{"kind", "namespace"}, nil) + + solrCloudInfoDesc = prometheus.NewDesc(metrics.Namespace+"_solrcloud_info", + "Information about a SolrCloud, always 1. The version label is the version of Solr the SolrCloud is running.", + []string{"namespace", "name", "version"}, nil) + solrCloudDesiredReplicasDesc = prometheus.NewDesc(metrics.Namespace+"_solrcloud_desired_replicas", + "Number of Solr pods the SolrCloud asks for (spec.replicas).", + []string{"namespace", "name"}, nil) + solrCloudReadyReplicasDesc = prometheus.NewDesc(metrics.Namespace+"_solrcloud_ready_replicas", + "Number of Solr pods of the SolrCloud that are ready.", + []string{"namespace", "name"}, nil) + solrCloudUpToDateReplicasDesc = prometheus.NewDesc(metrics.Namespace+"_solrcloud_up_to_date_replicas", + "Number of Solr pods of the SolrCloud that run the latest pod spec.", + []string{"namespace", "name"}, nil) + solrCloudReadyDesc = prometheus.NewDesc(metrics.Namespace+"_solrcloud_ready", + "Whether all the Solr pods the SolrCloud asks for are ready (1) or not (0).", + []string{"namespace", "name"}, nil) + solrCloudClusterOperationsDesc = prometheus.NewDesc(metrics.Namespace+"_solrcloud_cluster_operations", + "Number of cluster operations on the SolrCloud, by operation (RollingUpdate, ScalingUp, ScalingDown, BalanceReplicas, PVCExpansion) "+ + "and state: \"running\" for the operation holding the cluster operation lock, \"queued\" for operations waiting to be retried. "+ + "Only present while such operations exist.", + []string{"namespace", "name", "operation", "state"}, nil) + + solrBackupStateDesc = prometheus.NewDesc(metrics.Namespace+"_solrbackup_state", + "State of the latest run of the SolrBackup: 1 for its current state out of \"running\", \"succeeded\" and \"failed\", 0 for the other states.", + []string{"namespace", "name", "solrcloud", "state"}, nil) + + solrPrometheusExporterReadyDesc = prometheus.NewDesc(metrics.Namespace+"_solrprometheusexporter_ready", + "Whether the SolrPrometheusExporter is ready (1) or not (0).", + []string{"namespace", "name", "solrcloud"}, nil) +) + +// ResourceMetricsCollector exposes gauges describing the Solr resources that the operator manages. They are computed +// at scrape time from the operator's cache, so they reflect what the operator sees and never go stale when a +// resource is deleted. With several operator replicas every replica reports them, not just the leader. +type ResourceMetricsCollector struct { + reader client.Reader +} + +// NewResourceMetricsCollector creates a collector that reads the managed resources through the given reader, +// which should be the operator's cache. +func NewResourceMetricsCollector(reader client.Reader) *ResourceMetricsCollector { + return &ResourceMetricsCollector{reader: reader} +} + +// RegisterResourceMetrics registers the gauges describing the managed Solr resources with the registry behind the +// operator's metrics endpoint, reading the resources through the given reader (the operator's cache). +func RegisterResourceMetrics(reader client.Reader) error { + return ctrlmetrics.Registry.Register(NewResourceMetricsCollector(reader)) +} + +func (c *ResourceMetricsCollector) Describe(ch chan<- *prometheus.Desc) { + ch <- managedResourcesDesc + ch <- solrCloudInfoDesc + ch <- solrCloudDesiredReplicasDesc + ch <- solrCloudReadyReplicasDesc + ch <- solrCloudUpToDateReplicasDesc + ch <- solrCloudReadyDesc + ch <- solrCloudClusterOperationsDesc + ch <- solrBackupStateDesc + ch <- solrPrometheusExporterReadyDesc +} + +func (c *ResourceMetricsCollector) Collect(ch chan<- prometheus.Metric) { + ctx, cancel := context.WithTimeout(context.Background(), resourceMetricsTimeout) + defer cancel() + + // kind -> namespace -> number of resources + counts := map[string]map[string]int{} + count := func(kind string, namespace string) { + if counts[kind] == nil { + counts[kind] = map[string]int{} + } + counts[kind][namespace]++ + } + + c.collectSolrClouds(ctx, ch, count) + c.collectSolrBackups(ctx, ch, count) + c.collectSolrPrometheusExporters(ctx, ch, count) + + for kind, namespaces := range counts { + for namespace, n := range namespaces { + ch <- prometheus.MustNewConstMetric(managedResourcesDesc, prometheus.GaugeValue, float64(n), kind, namespace) + } + } +} + +func (c *ResourceMetricsCollector) collectSolrClouds(ctx context.Context, ch chan<- prometheus.Metric, count func(kind string, namespace string)) { + solrClouds := &solrv1beta1.SolrCloudList{} + if err := c.reader.List(ctx, solrClouds); err != nil { + resourceMetricsLog().Error(err, "Could not list SolrClouds for the operator metrics") + return + } + for i := range solrClouds.Items { + solrCloud := &solrClouds.Items[i] + count("SolrCloud", solrCloud.Namespace) + labels := []string{solrCloud.Namespace, solrCloud.Name} + + desiredReplicas := int32(0) Review Comment: An omitted `spec.replicas` means three replicas (`DefaultSolrReplicas` in `api/v1beta1/solrcloud_types.go`), not zero. Until the controller persists its defaults—and for longer if that update fails—this reports desired replicas as zero and marks a cloud with no ready pods as ready. Initialize this value with the API default while continuing to honor an explicitly configured zero. ########## controllers/solrbackup_controller.go: ########## @@ -142,7 +143,10 @@ func (r *SolrBackupReconciler) Reconcile(ctx context.Context, req ctrl.Request) now := metav1.Now() backup.Status.IndividualSolrBackupStatus.FinishTime = &now - if successful := backup.Status.IndividualSolrBackupStatus.Successful; successful != nil && *successful { + successful := backup.Status.IndividualSolrBackupStatus.Successful + backupSucceeded := successful != nil && *successful + metrics.RecordBackupRun(backup.Namespace, backup.Name, backup.Spec.SolrCloud, backupSucceeded) Review Comment: This increments the finished-run counter before the status patch at the end of `Reconcile` succeeds. If that patch fails, the stored backup remains unfinished; the retry detects the same completed collection backups and increments the counter again for the same run. Track the newly completed run and increment the counter only after its status patch succeeds. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
