From b5d3de5a9df414a39964ccf86caf1966a00130b8 Mon Sep 17 00:00:00 2001 From: Swetha Repakula Date: Thu, 25 May 2023 18:11:51 -0700 Subject: [PATCH 1/3] Remove unnecessary consts in metrics definitions * removal of constants makes reading the metric definition easier --- pkg/neg/metrics/label_propagation_metrics.go | 28 ++----- pkg/neg/metrics/metrics.go | 87 ++++++++------------ pkg/neg/metrics/syncer_metrics.go | 7 +- 3 files changed, 44 insertions(+), 78 deletions(-) diff --git a/pkg/neg/metrics/label_propagation_metrics.go b/pkg/neg/metrics/label_propagation_metrics.go index 29794a7fd6..aaf8b15905 100644 --- a/pkg/neg/metrics/label_propagation_metrics.go +++ b/pkg/neg/metrics/label_propagation_metrics.go @@ -21,36 +21,24 @@ import ( ) const ( - labelNumber = "label_number_per_endpoint" - annotationSize = "annotation_size_per_endpoint" - labelErrorNumber = "label_propagation_error_count" - numberOfEndpoints = "number_of_endpoints" - epWithAnnotation = "with_annotation" - totalEndpoints = "total" + epWithAnnotation = "with_annotation" + totalEndpoints = "total" ) var ( - labelPropagationErrorLabels = []string{ - "error_type", - } - - endpointAnnotationLabels = []string{ - "feature", - } - NumberOfEndpoints = prometheus.NewGaugeVec( prometheus.GaugeOpts{ Subsystem: negControllerSubsystem, - Name: numberOfEndpoints, + Name: "number_of_endpoints", Help: "The total number of endpoints", }, - endpointAnnotationLabels, + []string{"feature"}, ) LabelNumber = prometheus.NewHistogram( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: labelNumber, + Name: "label_number_per_endpoint", Help: "The number of labels per endpoint", // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), @@ -60,7 +48,7 @@ var ( AnnotationSize = prometheus.NewHistogram( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: annotationSize, + Name: "annotation_size_per_endpoint", Help: "The size in byte of endpoint annotations per endpoint", // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), @@ -70,10 +58,10 @@ var ( LabelPropagationError = prometheus.NewCounterVec( prometheus.CounterOpts{ Subsystem: negControllerSubsystem, - Name: labelErrorNumber, + Name: "label_propagation_error_count", Help: "the number of errors occurred for label propagation", }, - labelPropagationErrorLabels, + []string{"error_type"}, ) ) diff --git a/pkg/neg/metrics/metrics.go b/pkg/neg/metrics/metrics.go index 6f7c2fef79..9600c2379d 100644 --- a/pkg/neg/metrics/metrics.go +++ b/pkg/neg/metrics/metrics.go @@ -25,16 +25,7 @@ import ( ) const ( - negControllerSubsystem = "neg_controller" - syncerLatencyKey = "syncer_sync_duration_seconds" - managerProcessLatencyKey = "manager_process_duration_seconds" - initLatencyKey = "neg_initialization_duration_seconds" - negOpLatencyKey = "neg_operation_duration_seconds" - negOpEndpointsKey = "neg_operation_endpoints" - lastSyncTimestampKey = "sync_timestamp" - syncerStalenessKey = "syncer_staleness" - epsStalenessKey = "endpointslice_staleness" - degradedModeCorrectnessKey = "degraded_mode_correctness" + negControllerSubsystem = "neg_controller" resultSuccess = "success" resultError = "error" @@ -61,83 +52,70 @@ const ( type syncType string var ( - negOpLatencyMetricsLabels = []string{ - "operation", // endpoint operation - "neg_type", // type of neg - "api_version", // GCE API version - "result", // result of the sync - } - - negOpEndpointsMetricsLabels = []string{ - "operation", // endpoint operation - "neg_type", // type of neg - "result", // result of the sync - } - - negProcessMetricsLabels = []string{ - "process", // type of manager process loop - "result", // result of the process - } - - syncerMetricsLabels = []string{ - "neg_type", //type of neg - "endpoint_calculator_mode", // type of endpoint calculator used - "result", // result of the sync - } - - degradedModeCorrectnessLabels = []string{ - "neg_type", // type of neg - "endpoint_type", // type of endpoint - } - NegOperationLatency = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: negOpLatencyKey, + Name: "neg_operation_duration_seconds", Help: "Latency of a NEG Operation", // custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), }, - negOpLatencyMetricsLabels, + []string{ + "operation", // endpoint operation + "neg_type", // type of neg + "api_version", // GCE API version + "result", // result of the sync + }, ) NegOperationEndpoints = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: negOpEndpointsKey, + Name: "neg_operation_endpoints", Help: "Number of Endpoints during an NEG Operation", // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), }, - negOpEndpointsMetricsLabels, + []string{ + "operation", // endpoint operation + "neg_type", // type of neg + "result", // result of the sync + }, ) SyncerSyncLatency = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: syncerLatencyKey, + Name: "syncer_sync_duration_seconds", Help: "Sync latency for NEG Syncer", // custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), }, - syncerMetricsLabels, + []string{ + "neg_type", //type of neg + "endpoint_calculator_mode", // type of endpoint calculator used + "result", // result of the sync + }, ) ManagerProcessLatency = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: managerProcessLatencyKey, + Name: "manager_process_duration_seconds", Help: "Process latency for NEG Manager", // custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), }, - negProcessMetricsLabels, + []string{ + "process", // type of manager process loop + "result", // result of the process + }, ) InitializationLatency = prometheus.NewHistogram( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: initLatencyKey, + Name: "neg_initialization_duration_seconds", Help: "Initialization latency of a NEG", // custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 13), @@ -147,7 +125,7 @@ var ( LastSyncTimestamp = prometheus.NewGauge( prometheus.GaugeOpts{ Subsystem: negControllerSubsystem, - Name: lastSyncTimestampKey, + Name: "sync_timestamp", Help: "The timestamp of the last execution of NEG controller sync loop.", }, ) @@ -156,7 +134,7 @@ var ( SyncerStaleness = prometheus.NewHistogram( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: syncerStalenessKey, + Name: "syncer_staleness", Help: "The duration of a syncer since it last syncs", // custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), 8192(~136min), +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 14), @@ -167,7 +145,7 @@ var ( EPSStaleness = prometheus.NewHistogram( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: epsStalenessKey, + Name: "endpointslice_staleness", Help: "The duration for an endpoint slice since it was last processed by syncer", // custom buckets - [1s, 2s, 4s, 8s, 16s, 32s, 64s, 128s, 256s(~4min), 512s(~8min), 1024s(~17min), 2048 (~34min), 4096(~68min), 8192(~136min), +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 14), @@ -177,12 +155,15 @@ var ( DegradeModeCorrectness = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: degradedModeCorrectnessKey, + Name: "degraded_mode_correctness", Help: "Number of endpoints differed between current endpoint calculation and degraded mode calculation", // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072, 262144, 524288, +Inf] Buckets: prometheus.ExponentialBuckets(1, 2, 20), }, - degradedModeCorrectnessLabels, + []string{ + "neg_type", // type of neg + "endpoint_type", // type of endpoint + }, ) DualStackMigrationFinishedDurations = prometheus.NewHistogram( diff --git a/pkg/neg/metrics/syncer_metrics.go b/pkg/neg/metrics/syncer_metrics.go index e11a7ba345..f50def038c 100644 --- a/pkg/neg/metrics/syncer_metrics.go +++ b/pkg/neg/metrics/syncer_metrics.go @@ -22,9 +22,6 @@ import ( ) const ( - syncResultLabel = "result" - syncResultKey = "sync_result" - EPCountsDiffer = "EndpointCountsDiffer" EPNodeMissing = "EndpointNodeMissing" EPNodeNotFound = "EndpointNodeNotFound" @@ -49,10 +46,10 @@ var ( syncerSyncResult = prometheus.NewCounterVec( prometheus.CounterOpts{ Subsystem: negControllerSubsystem, - Name: syncResultKey, + Name: "sync_result", Help: "Current count for each sync result", }, - []string{syncResultLabel}, + []string{"result"}, ) // syncerState tracks the count of syncer in different states From 8693a51a4808d8ba534348b1169c1cdd5827a735 Mon Sep 17 00:00:00 2001 From: Swetha Repakula Date: Thu, 25 May 2023 18:32:24 -0700 Subject: [PATCH 2/3] Consolidate metrics by whether emitted by metrics collector * all gauge metrics that require state are moved to syncer_metrics.go are published by the metrics collector * all metrics that do not require state and not emitted by the metrics collector --- pkg/neg/metrics/endpoint_metrics.go | 43 ------ pkg/neg/metrics/label_propagation_metrics.go | 89 ------------ pkg/neg/metrics/metrics.go | 73 +++++----- pkg/neg/metrics/metrics_test.go | 96 ------------- pkg/neg/metrics/neg_metrics_collector.go | 27 +++- pkg/neg/metrics/neg_metrics_collector_test.go | 72 ++++++++++ pkg/neg/metrics/syncer_metrics.go | 127 +++++++++++++----- 7 files changed, 228 insertions(+), 299 deletions(-) delete mode 100644 pkg/neg/metrics/endpoint_metrics.go delete mode 100644 pkg/neg/metrics/label_propagation_metrics.go delete mode 100644 pkg/neg/metrics/metrics_test.go diff --git a/pkg/neg/metrics/endpoint_metrics.go b/pkg/neg/metrics/endpoint_metrics.go deleted file mode 100644 index e1788fb13e..0000000000 --- a/pkg/neg/metrics/endpoint_metrics.go +++ /dev/null @@ -1,43 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed 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 metrics - -import ( - "github.com/prometheus/client_golang/prometheus" -) - -var ( - // syncerEndpointState tracks the count of endpoints in different states - syncerEndpointState = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Subsystem: negControllerSubsystem, - Name: "syncer_endpoint_state", - Help: "Current count of endpoints in each state", - }, - []string{"state"}, - ) - - // syncerEndpointSliceState tracks the count of endpoint slices in different states - syncerEndpointSliceState = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Subsystem: negControllerSubsystem, - Name: "syncer_endpoint_slice_state", - Help: "Current count of endpoint slices in each state", - }, - []string{"state"}, - ) -) diff --git a/pkg/neg/metrics/label_propagation_metrics.go b/pkg/neg/metrics/label_propagation_metrics.go deleted file mode 100644 index aaf8b15905..0000000000 --- a/pkg/neg/metrics/label_propagation_metrics.go +++ /dev/null @@ -1,89 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed 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 metrics - -import ( - "github.com/prometheus/client_golang/prometheus" -) - -const ( - epWithAnnotation = "with_annotation" - totalEndpoints = "total" -) - -var ( - NumberOfEndpoints = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ - Subsystem: negControllerSubsystem, - Name: "number_of_endpoints", - Help: "The total number of endpoints", - }, - []string{"feature"}, - ) - - LabelNumber = prometheus.NewHistogram( - prometheus.HistogramOpts{ - Subsystem: negControllerSubsystem, - Name: "label_number_per_endpoint", - Help: "The number of labels per endpoint", - // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] - Buckets: prometheus.ExponentialBuckets(1, 2, 13), - }, - ) - - AnnotationSize = prometheus.NewHistogram( - prometheus.HistogramOpts{ - Subsystem: negControllerSubsystem, - Name: "annotation_size_per_endpoint", - Help: "The size in byte of endpoint annotations per endpoint", - // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] - Buckets: prometheus.ExponentialBuckets(1, 2, 13), - }, - ) - - LabelPropagationError = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Subsystem: negControllerSubsystem, - Name: "label_propagation_error_count", - Help: "the number of errors occurred for label propagation", - }, - []string{"error_type"}, - ) -) - -// LabelPropagationStat contains stats related to label propagation. -type LabelPropagationStats struct { - EndpointsWithAnnotation int - NumberOfEndpoints int -} - -// LabelPropagationMetrics contains aggregated label propagation related metrics. -type LabelPropagationMetrics struct { - EndpointsWithAnnotation int - NumberOfEndpoints int -} - -// PublishLabelPropagationError publishes error occured during label propagation. -func PublishLabelPropagationError(errType string) { - LabelPropagationError.WithLabelValues(errType).Inc() -} - -// PublishAnnotationMetrics publishes collected metrics for endpoint annotations. -func PublishAnnotationMetrics(annotationSize int, labelNumber int) { - AnnotationSize.Observe(float64(annotationSize)) - LabelNumber.Observe(float64(labelNumber)) -} diff --git a/pkg/neg/metrics/metrics.go b/pkg/neg/metrics/metrics.go index 9600c2379d..e84ec7cbd5 100644 --- a/pkg/neg/metrics/metrics.go +++ b/pkg/neg/metrics/metrics.go @@ -36,12 +36,6 @@ const ( NotInDegradedEndpoints = "not_in_degraded_endpoints" OnlyInDegradedEndpoints = "only_in_degraded_endpoints" - // Classification of endpoints within a NEG. - ipv4EndpointType = "IPv4" - ipv6EndpointType = "IPv6" - dualStackEndpointType = "DualStack" - migrationEndpointType = "Migration" - gceServerError = "GCE_server_error" k8sServerError = "K8s_server_error" ignoredError = "ignored_error" @@ -166,49 +160,42 @@ var ( }, ) - DualStackMigrationFinishedDurations = prometheus.NewHistogram( - prometheus.HistogramOpts{ - Subsystem: negControllerSubsystem, - Name: "dual_stack_migration_finished_durations_seconds", - Help: "Time taken to migrate all endpoints within all NEGs for a service port", - // Buckets ~= [1s, 1.85s, 3.42s, 6s, 11s, 21s, 40s, 1m14s, 2m17s, 4m13s, 7m49s, 14m28s, 26m47s, 49m33s, 1h31m40s, 2h49m35s, 5h13m45s, 9h40m27s, +Inf] - Buckets: prometheus.ExponentialBuckets(1, 1.85, 18), - }, - ) - - // A zero value for this metric means that there are no ongoing migrations. - DualStackMigrationLongestUnfinishedDuration = prometheus.NewGauge( - prometheus.GaugeOpts{ + // NegControllerErrorCount tracks the count of server errors(GCE/K8s) and + // all errors from NEG controller. + NegControllerErrorCount = prometheus.NewCounterVec( + prometheus.CounterOpts{ Subsystem: negControllerSubsystem, - Name: "dual_stack_migration_longest_unfinished_duration_seconds", - Help: "Longest time elapsed since a migration was started which hasn't yet completed", + Name: "error_count", + Help: "Counts of server errors and NEG controller errors.", }, + []string{"error_type"}, ) - DualStackMigrationServiceCount = prometheus.NewGauge( - prometheus.GaugeOpts{ + LabelNumber = prometheus.NewHistogram( + prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: "dual_stack_migration_service_count", - Help: "Number of Services which have migration endpoints", + Name: "label_number_per_endpoint", + Help: "The number of labels per endpoint", + // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] + Buckets: prometheus.ExponentialBuckets(1, 2, 13), }, ) - SyncerCountByEndpointType = prometheus.NewGaugeVec( - prometheus.GaugeOpts{ + AnnotationSize = prometheus.NewHistogram( + prometheus.HistogramOpts{ Subsystem: negControllerSubsystem, - Name: "syncer_count_by_endpoint_type", - Help: "Number of Syncers managing NEGs containing endpoint of a particular kind", + Name: "annotation_size_per_endpoint", + Help: "The size in byte of endpoint annotations per endpoint", + // custom buckets - [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, +Inf] + Buckets: prometheus.ExponentialBuckets(1, 2, 13), }, - []string{"endpoint_type"}, ) - // NegControllerErrorCount tracks the count of server errors(GCE/K8s) and - // all errors from NEG controller. - NegControllerErrorCount = prometheus.NewCounterVec( + LabelPropagationError = prometheus.NewCounterVec( prometheus.CounterOpts{ Subsystem: negControllerSubsystem, - Name: "error_count", - Help: "Counts of server errors and NEG controller errors.", + Name: "label_propagation_error_count", + Help: "the number of errors occurred for label propagation", }, []string{"error_type"}, ) @@ -226,15 +213,10 @@ func RegisterMetrics() { prometheus.MustRegister(InitializationLatency) prometheus.MustRegister(SyncerStaleness) prometheus.MustRegister(EPSStaleness) - prometheus.MustRegister(NumberOfEndpoints) prometheus.MustRegister(LabelPropagationError) prometheus.MustRegister(LabelNumber) prometheus.MustRegister(AnnotationSize) prometheus.MustRegister(DegradeModeCorrectness) - prometheus.MustRegister(DualStackMigrationFinishedDurations) - prometheus.MustRegister(DualStackMigrationLongestUnfinishedDuration) - prometheus.MustRegister(DualStackMigrationServiceCount) - prometheus.MustRegister(SyncerCountByEndpointType) prometheus.MustRegister(NegControllerErrorCount) RegisterSyncerMetrics() @@ -291,6 +273,17 @@ func PublishNegControllerErrorCountMetrics(err error, isIgnored bool) { NegControllerErrorCount.WithLabelValues(getErrorLabel(err, isIgnored)).Inc() } +// PublishLabelPropagationError publishes error occured during label propagation. +func PublishLabelPropagationError(errType string) { + LabelPropagationError.WithLabelValues(errType).Inc() +} + +// PublishAnnotationMetrics publishes collected metrics for endpoint annotations. +func PublishAnnotationMetrics(annotationSize int, labelNumber int) { + AnnotationSize.Observe(float64(annotationSize)) + LabelNumber.Observe(float64(labelNumber)) +} + func getResult(err error) string { if err != nil { return resultError diff --git a/pkg/neg/metrics/metrics_test.go b/pkg/neg/metrics/metrics_test.go deleted file mode 100644 index 10914f667f..0000000000 --- a/pkg/neg/metrics/metrics_test.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed 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 metrics - -import ( - "testing" - "time" - - "github.com/google/go-cmp/cmp" - negtypes "k8s.io/ingress-gce/pkg/neg/types" - "k8s.io/klog/v2" -) - -func TestComputeLabelMetrics(t *testing.T) { - collector := NewNegMetricsCollector(10*time.Second, klog.TODO()) - syncer1 := negtypes.NegSyncerKey{ - Namespace: "ns1", - Name: "svc-1", - NegName: "neg-1", - NegType: negtypes.VmIpPortEndpointType, - EpCalculatorMode: negtypes.L7Mode, - } - syncer2 := negtypes.NegSyncerKey{ - Namespace: "ns1", - Name: "svc-2", - NegName: "neg-2", - NegType: negtypes.VmIpPortEndpointType, - EpCalculatorMode: negtypes.L7Mode, - } - for _, tc := range []struct { - desc string - syncerLabelProagationStats map[negtypes.NegSyncerKey]LabelPropagationStats - expect LabelPropagationMetrics - }{ - { - desc: "Empty Data", - syncerLabelProagationStats: map[negtypes.NegSyncerKey]LabelPropagationStats{ - syncer1: {}, - }, - expect: LabelPropagationMetrics{ - EndpointsWithAnnotation: 0, - NumberOfEndpoints: 0, - }, - }, - { - desc: "All endpoints have annotations", - syncerLabelProagationStats: map[negtypes.NegSyncerKey]LabelPropagationStats{ - syncer1: { - EndpointsWithAnnotation: 10, - NumberOfEndpoints: 10, - }, - }, - expect: LabelPropagationMetrics{ - EndpointsWithAnnotation: 10, - NumberOfEndpoints: 10, - }, - }, - { - desc: "Test with 2 syncers", - syncerLabelProagationStats: map[negtypes.NegSyncerKey]LabelPropagationStats{ - syncer1: { - EndpointsWithAnnotation: 10, - NumberOfEndpoints: 10, - }, - syncer2: { - EndpointsWithAnnotation: 5, - NumberOfEndpoints: 10, - }, - }, - expect: LabelPropagationMetrics{ - EndpointsWithAnnotation: 15, - NumberOfEndpoints: 20, - }, - }, - } { - collector.syncerLabelProagationStats = tc.syncerLabelProagationStats - out := collector.computeLabelMetrics() - if diff := cmp.Diff(out, tc.expect); diff != "" { - t.Errorf("For test case %s, (-want +got):\n%s", tc.desc, diff) - } - } -} diff --git a/pkg/neg/metrics/neg_metrics_collector.go b/pkg/neg/metrics/neg_metrics_collector.go index b9840d8e76..c6b56985bc 100644 --- a/pkg/neg/metrics/neg_metrics_collector.go +++ b/pkg/neg/metrics/neg_metrics_collector.go @@ -86,10 +86,15 @@ func FakeSyncerMetrics() *SyncerMetrics { // RegisterSyncerMetrics registers syncer related metrics func RegisterSyncerMetrics() { - prometheus.MustRegister(syncerSyncResult) prometheus.MustRegister(syncerState) prometheus.MustRegister(syncerEndpointState) prometheus.MustRegister(syncerEndpointSliceState) + prometheus.MustRegister(NumberOfEndpoints) + prometheus.MustRegister(DualStackMigrationFinishedDurations) + prometheus.MustRegister(DualStackMigrationLongestUnfinishedDuration) + prometheus.MustRegister(DualStackMigrationServiceCount) + prometheus.MustRegister(SyncerCountByEndpointType) + prometheus.MustRegister(syncerSyncResult) } func (sm *SyncerMetrics) Run(stopCh <-chan struct{}) { @@ -373,3 +378,23 @@ func (sm *SyncerMetrics) computeDualStackMigrationCounts() (map[string]int, int, } return syncerCountByEndpointType, migrationEndpointCount, migrationServices.Len() } + +func PublishSyncerStateMetrics(stateCount *syncerStateCount) { + syncerState.WithLabelValues(EPCountsDiffer).Set(float64(stateCount.epCountsDiffer)) + syncerState.WithLabelValues(EPNodeMissing).Set(float64(stateCount.epNodeMissing)) + syncerState.WithLabelValues(EPNodeNotFound).Set(float64(stateCount.epNodeNotFound)) + syncerState.WithLabelValues(EPPodMissing).Set(float64(stateCount.epPodMissing)) + syncerState.WithLabelValues(EPPodNotFound).Set(float64(stateCount.epPodNotFound)) + syncerState.WithLabelValues(EPPodTypeAssertionFailed).Set(float64(stateCount.epPodTypeAssertionFailed)) + syncerState.WithLabelValues(EPZoneMissing).Set(float64(stateCount.epZoneMissing)) + syncerState.WithLabelValues(EPSEndpointCountZero).Set(float64(stateCount.epsEndpointCountZero)) + syncerState.WithLabelValues(EPCalculationCountZero).Set(float64(stateCount.epCalculationCountZero)) + syncerState.WithLabelValues(InvalidAPIResponse).Set(float64(stateCount.invalidAPIResponse)) + syncerState.WithLabelValues(InvalidEPAttach).Set(float64(stateCount.invalidEPAttach)) + syncerState.WithLabelValues(InvalidEPDetach).Set(float64(stateCount.invalidEPDetach)) + syncerState.WithLabelValues(NegNotFound).Set(float64(stateCount.negNotFound)) + syncerState.WithLabelValues(CurrentNegEPNotFound).Set(float64(stateCount.currentNegEPNotFound)) + syncerState.WithLabelValues(EPSNotFound).Set(float64(stateCount.epsNotFound)) + syncerState.WithLabelValues(OtherError).Set(float64(stateCount.otherError)) + syncerState.WithLabelValues(Success).Set(float64(stateCount.success)) +} diff --git a/pkg/neg/metrics/neg_metrics_collector_test.go b/pkg/neg/metrics/neg_metrics_collector_test.go index 25a2322cc2..c38bd0b67b 100644 --- a/pkg/neg/metrics/neg_metrics_collector_test.go +++ b/pkg/neg/metrics/neg_metrics_collector_test.go @@ -8,6 +8,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "k8s.io/ingress-gce/pkg/neg/types" + negtypes "k8s.io/ingress-gce/pkg/neg/types" + "k8s.io/klog/v2" "k8s.io/utils/clock" ) @@ -219,6 +221,76 @@ func TestComputeDualStackMigrationCounts(t *testing.T) { } } +func TestComputeLabelMetrics(t *testing.T) { + collector := NewNegMetricsCollector(10*time.Second, klog.TODO()) + syncer1 := negtypes.NegSyncerKey{ + Namespace: "ns1", + Name: "svc-1", + NegName: "neg-1", + NegType: negtypes.VmIpPortEndpointType, + EpCalculatorMode: negtypes.L7Mode, + } + syncer2 := negtypes.NegSyncerKey{ + Namespace: "ns1", + Name: "svc-2", + NegName: "neg-2", + NegType: negtypes.VmIpPortEndpointType, + EpCalculatorMode: negtypes.L7Mode, + } + for _, tc := range []struct { + desc string + syncerLabelProagationStats map[negtypes.NegSyncerKey]LabelPropagationStats + expect LabelPropagationMetrics + }{ + { + desc: "Empty Data", + syncerLabelProagationStats: map[negtypes.NegSyncerKey]LabelPropagationStats{ + syncer1: {}, + }, + expect: LabelPropagationMetrics{ + EndpointsWithAnnotation: 0, + NumberOfEndpoints: 0, + }, + }, + { + desc: "All endpoints have annotations", + syncerLabelProagationStats: map[negtypes.NegSyncerKey]LabelPropagationStats{ + syncer1: { + EndpointsWithAnnotation: 10, + NumberOfEndpoints: 10, + }, + }, + expect: LabelPropagationMetrics{ + EndpointsWithAnnotation: 10, + NumberOfEndpoints: 10, + }, + }, + { + desc: "Test with 2 syncers", + syncerLabelProagationStats: map[negtypes.NegSyncerKey]LabelPropagationStats{ + syncer1: { + EndpointsWithAnnotation: 10, + NumberOfEndpoints: 10, + }, + syncer2: { + EndpointsWithAnnotation: 5, + NumberOfEndpoints: 10, + }, + }, + expect: LabelPropagationMetrics{ + EndpointsWithAnnotation: 15, + NumberOfEndpoints: 20, + }, + }, + } { + collector.syncerLabelProagationStats = tc.syncerLabelProagationStats + out := collector.computeLabelMetrics() + if diff := cmp.Diff(out, tc.expect); diff != "" { + t.Errorf("For test case %s, (-want +got):\n%s", tc.desc, diff) + } + } +} + type fakeClock struct { clock.Clock curTime time.Time diff --git a/pkg/neg/metrics/syncer_metrics.go b/pkg/neg/metrics/syncer_metrics.go index f50def038c..b111a5acec 100644 --- a/pkg/neg/metrics/syncer_metrics.go +++ b/pkg/neg/metrics/syncer_metrics.go @@ -39,19 +39,19 @@ const ( EPSNotFound = "EndpointSliceNotFound" OtherError = "OtherError" Success = "Success" + + // Label values for Label Propagation Metrics + epWithAnnotation = "with_annotation" + totalEndpoints = "total" + + // Classification of endpoints within a NEG. + ipv4EndpointType = "IPv4" + ipv6EndpointType = "IPv6" + dualStackEndpointType = "DualStack" + migrationEndpointType = "Migration" ) var ( - // syncerSyncResult tracks the count for each sync result - syncerSyncResult = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Subsystem: negControllerSubsystem, - Name: "sync_result", - Help: "Current count for each sync result", - }, - []string{"result"}, - ) - // syncerState tracks the count of syncer in different states syncerState = prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -61,6 +61,81 @@ var ( }, []string{"state"}, ) + + // syncerEndpointState tracks the count of endpoints in different states + syncerEndpointState = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: negControllerSubsystem, + Name: "syncer_endpoint_state", + Help: "Current count of endpoints in each state", + }, + []string{"state"}, + ) + + // syncerEndpointSliceState tracks the count of endpoint slices in different states + syncerEndpointSliceState = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: negControllerSubsystem, + Name: "syncer_endpoint_slice_state", + Help: "Current count of endpoint slices in each state", + }, + []string{"state"}, + ) + + NumberOfEndpoints = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: negControllerSubsystem, + Name: "number_of_endpoints", + Help: "The total number of endpoints", + }, + []string{"feature"}, + ) + + DualStackMigrationFinishedDurations = prometheus.NewHistogram( + prometheus.HistogramOpts{ + Subsystem: negControllerSubsystem, + Name: "dual_stack_migration_finished_durations_seconds", + Help: "Time taken to migrate all endpoints within all NEGs for a service port", + // Buckets ~= [1s, 1.85s, 3.42s, 6s, 11s, 21s, 40s, 1m14s, 2m17s, 4m13s, 7m49s, 14m28s, 26m47s, 49m33s, 1h31m40s, 2h49m35s, 5h13m45s, 9h40m27s, +Inf] + Buckets: prometheus.ExponentialBuckets(1, 1.85, 18), + }, + ) + + // A zero value for this metric means that there are no ongoing migrations. + DualStackMigrationLongestUnfinishedDuration = prometheus.NewGauge( + prometheus.GaugeOpts{ + Subsystem: negControllerSubsystem, + Name: "dual_stack_migration_longest_unfinished_duration_seconds", + Help: "Longest time elapsed since a migration was started which hasn't yet completed", + }, + ) + + SyncerCountByEndpointType = prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Subsystem: negControllerSubsystem, + Name: "syncer_count_by_endpoint_type", + Help: "Number of Syncers managing NEGs containing endpoint of a particular kind", + }, + []string{"endpoint_type"}, + ) + + DualStackMigrationServiceCount = prometheus.NewGauge( + prometheus.GaugeOpts{ + Subsystem: negControllerSubsystem, + Name: "dual_stack_migration_service_count", + Help: "Number of Services which have migration endpoints", + }, + ) + + // syncerSyncResult tracks the count for each sync result + syncerSyncResult = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Subsystem: negControllerSubsystem, + Name: "sync_result", + Help: "Current count for each sync result", + }, + []string{"result"}, + ) ) type syncerStateCount struct { @@ -83,6 +158,18 @@ type syncerStateCount struct { success int } +// LabelPropagationStat contains stats related to label propagation. +type LabelPropagationStats struct { + EndpointsWithAnnotation int + NumberOfEndpoints int +} + +// LabelPropagationMetrics contains aggregated label propagation related metrics. +type LabelPropagationMetrics struct { + EndpointsWithAnnotation int + NumberOfEndpoints int +} + func (sc *syncerStateCount) inc(reason negtypes.Reason) { switch reason { case negtypes.ReasonEPCountsDiffer: @@ -119,23 +206,3 @@ func (sc *syncerStateCount) inc(reason negtypes.Reason) { sc.success++ } } - -func PublishSyncerStateMetrics(stateCount *syncerStateCount) { - syncerState.WithLabelValues(EPCountsDiffer).Set(float64(stateCount.epCountsDiffer)) - syncerState.WithLabelValues(EPNodeMissing).Set(float64(stateCount.epNodeMissing)) - syncerState.WithLabelValues(EPNodeNotFound).Set(float64(stateCount.epNodeNotFound)) - syncerState.WithLabelValues(EPPodMissing).Set(float64(stateCount.epPodMissing)) - syncerState.WithLabelValues(EPPodNotFound).Set(float64(stateCount.epPodNotFound)) - syncerState.WithLabelValues(EPPodTypeAssertionFailed).Set(float64(stateCount.epPodTypeAssertionFailed)) - syncerState.WithLabelValues(EPZoneMissing).Set(float64(stateCount.epZoneMissing)) - syncerState.WithLabelValues(EPSEndpointCountZero).Set(float64(stateCount.epsEndpointCountZero)) - syncerState.WithLabelValues(EPCalculationCountZero).Set(float64(stateCount.epCalculationCountZero)) - syncerState.WithLabelValues(InvalidAPIResponse).Set(float64(stateCount.invalidAPIResponse)) - syncerState.WithLabelValues(InvalidEPAttach).Set(float64(stateCount.invalidEPAttach)) - syncerState.WithLabelValues(InvalidEPDetach).Set(float64(stateCount.invalidEPDetach)) - syncerState.WithLabelValues(NegNotFound).Set(float64(stateCount.negNotFound)) - syncerState.WithLabelValues(CurrentNegEPNotFound).Set(float64(stateCount.currentNegEPNotFound)) - syncerState.WithLabelValues(EPSNotFound).Set(float64(stateCount.epsNotFound)) - syncerState.WithLabelValues(OtherError).Set(float64(stateCount.otherError)) - syncerState.WithLabelValues(Success).Set(float64(stateCount.success)) -} From ec8a4af1b1fc8ae2392b1890dba46431a6ada4dc Mon Sep 17 00:00:00 2001 From: Swetha Repakula Date: Fri, 26 May 2023 13:54:46 -0700 Subject: [PATCH 3/3] Separate metricscollector and related metrics into package * metricscollector depends on negtypes therefore pull this into its own package * this separation will help when adding metrics that need to go into the negtypes package * register the metrics for each package separately to ensure isolation between the packages --- pkg/neg/controller.go | 5 +-- pkg/neg/manager.go | 5 +-- pkg/neg/manager_test.go | 4 +-- pkg/neg/metrics/metrics.go | 2 -- .../metrics.go} | 5 ++- .../metrics_collector.go} | 32 +++++++++++-------- .../metrics_collector_test.go} | 18 ++++++++++- pkg/neg/syncers/dualstack/migrator_test.go | 4 +-- pkg/neg/syncers/endpoints_calculator.go | 5 +-- pkg/neg/syncers/endpoints_calculator_test.go | 4 +-- pkg/neg/syncers/transaction.go | 11 ++++--- pkg/neg/syncers/transaction_test.go | 16 +++++----- 12 files changed, 68 insertions(+), 43 deletions(-) rename pkg/neg/metrics/{syncer_metrics.go => metricscollector/metrics.go} (98%) rename pkg/neg/metrics/{neg_metrics_collector.go => metricscollector/metrics_collector.go} (96%) rename pkg/neg/metrics/{neg_metrics_collector_test.go => metricscollector/metrics_collector_test.go} (95%) diff --git a/pkg/neg/controller.go b/pkg/neg/controller.go index 707c0e34d8..ed753a41be 100644 --- a/pkg/neg/controller.go +++ b/pkg/neg/controller.go @@ -40,7 +40,7 @@ import ( "k8s.io/ingress-gce/pkg/flags" usageMetrics "k8s.io/ingress-gce/pkg/metrics" "k8s.io/ingress-gce/pkg/neg/metrics" - syncMetrics "k8s.io/ingress-gce/pkg/neg/metrics" + syncMetrics "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/readiness" "k8s.io/ingress-gce/pkg/neg/syncers/labels" negtypes "k8s.io/ingress-gce/pkg/neg/types" @@ -56,6 +56,7 @@ import ( func init() { // register prometheus metrics metrics.RegisterMetrics() + syncMetrics.RegisterMetrics() } // Controller is network endpoint group controller. @@ -163,7 +164,7 @@ func NewController( recorder := eventBroadcaster.NewRecorder(negScheme, apiv1.EventSource{Component: "neg-controller"}) - syncerMetrics := metrics.NewNegMetricsCollector(flags.F.NegMetricsExportInterval, logger) + syncerMetrics := syncMetrics.NewNegMetricsCollector(flags.F.NegMetricsExportInterval, logger) manager := newSyncerManager( namer, recorder, diff --git a/pkg/neg/manager.go b/pkg/neg/manager.go index 8802276bcd..af34bc6c3c 100644 --- a/pkg/neg/manager.go +++ b/pkg/neg/manager.go @@ -37,6 +37,7 @@ import ( "k8s.io/client-go/tools/record" negv1beta1 "k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1" "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/readiness" negsyncer "k8s.io/ingress-gce/pkg/neg/syncers" podlabels "k8s.io/ingress-gce/pkg/neg/syncers/labels" @@ -81,7 +82,7 @@ type syncerManager struct { // key consists of service namespace, name and targetPort. Value is the corresponding syncer. syncerMap map[negtypes.NegSyncerKey]negtypes.NegSyncer // syncCollector collect sync related metrics - syncerMetrics *metrics.SyncerMetrics + syncerMetrics *metricscollector.SyncerMetrics // reflector handles NEG readiness gate and conditions for pods in NEG. reflector readiness.Reflector @@ -126,7 +127,7 @@ func newSyncerManager(namer negtypes.NetworkEndpointGroupNamer, endpointSliceLister cache.Indexer, nodeLister cache.Indexer, svcNegLister cache.Indexer, - syncerMetrics *metrics.SyncerMetrics, + syncerMetrics *metricscollector.SyncerMetrics, enableNonGcpMode bool, enableDualStackNEG bool, numGCWorkers int, diff --git a/pkg/neg/manager_test.go b/pkg/neg/manager_test.go index ca9959a9a8..c13ce6fd19 100644 --- a/pkg/neg/manager_test.go +++ b/pkg/neg/manager_test.go @@ -42,7 +42,7 @@ import ( "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" negv1beta1 "k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1" - "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/syncers/labels" "k8s.io/ingress-gce/pkg/neg/types" negtypes "k8s.io/ingress-gce/pkg/neg/types" @@ -92,7 +92,7 @@ func NewTestSyncerManager(kubeClient kubernetes.Interface) (*syncerManager, *gce testContext.EndpointSliceInformer.GetIndexer(), testContext.NodeInformer.GetIndexer(), testContext.SvcNegInformer.GetIndexer(), - metrics.FakeSyncerMetrics(), + metricscollector.FakeSyncerMetrics(), false, //enableNonGcpMode testContext.EnableDualStackNEG, testContext.NumGCWorkers, diff --git a/pkg/neg/metrics/metrics.go b/pkg/neg/metrics/metrics.go index e84ec7cbd5..b6c11da790 100644 --- a/pkg/neg/metrics/metrics.go +++ b/pkg/neg/metrics/metrics.go @@ -218,8 +218,6 @@ func RegisterMetrics() { prometheus.MustRegister(AnnotationSize) prometheus.MustRegister(DegradeModeCorrectness) prometheus.MustRegister(NegControllerErrorCount) - - RegisterSyncerMetrics() }) } diff --git a/pkg/neg/metrics/syncer_metrics.go b/pkg/neg/metrics/metricscollector/metrics.go similarity index 98% rename from pkg/neg/metrics/syncer_metrics.go rename to pkg/neg/metrics/metricscollector/metrics.go index b111a5acec..c5555adbd9 100644 --- a/pkg/neg/metrics/syncer_metrics.go +++ b/pkg/neg/metrics/metricscollector/metrics.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package metrics +package metricscollector import ( "github.com/prometheus/client_golang/prometheus" @@ -22,6 +22,9 @@ import ( ) const ( + negControllerSubsystem = "neg_controller" + + // Label Values for Syncer Sync Result Metrics EPCountsDiffer = "EndpointCountsDiffer" EPNodeMissing = "EndpointNodeMissing" EPNodeNotFound = "EndpointNodeNotFound" diff --git a/pkg/neg/metrics/neg_metrics_collector.go b/pkg/neg/metrics/metricscollector/metrics_collector.go similarity index 96% rename from pkg/neg/metrics/neg_metrics_collector.go rename to pkg/neg/metrics/metricscollector/metrics_collector.go index c6b56985bc..d720a99697 100644 --- a/pkg/neg/metrics/neg_metrics_collector.go +++ b/pkg/neg/metrics/metricscollector/metrics_collector.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package metrics +package metricscollector import ( "fmt" @@ -29,6 +29,23 @@ import ( "k8s.io/utils/clock" ) +var register sync.Once + +// RegisterSyncerMetrics registers syncer related metrics +func RegisterMetrics() { + register.Do(func() { + prometheus.MustRegister(syncerState) + prometheus.MustRegister(syncerEndpointState) + prometheus.MustRegister(syncerEndpointSliceState) + prometheus.MustRegister(NumberOfEndpoints) + prometheus.MustRegister(DualStackMigrationFinishedDurations) + prometheus.MustRegister(DualStackMigrationLongestUnfinishedDuration) + prometheus.MustRegister(DualStackMigrationServiceCount) + prometheus.MustRegister(SyncerCountByEndpointType) + prometheus.MustRegister(syncerSyncResult) + }) +} + type SyncerMetricsCollector interface { // UpdateSyncerStatusInMetrics update the status of corresponding syncer based on the sync error UpdateSyncerStatusInMetrics(key negtypes.NegSyncerKey, err error) @@ -84,19 +101,6 @@ func FakeSyncerMetrics() *SyncerMetrics { return NewNegMetricsCollector(5*time.Second, klog.TODO()) } -// RegisterSyncerMetrics registers syncer related metrics -func RegisterSyncerMetrics() { - prometheus.MustRegister(syncerState) - prometheus.MustRegister(syncerEndpointState) - prometheus.MustRegister(syncerEndpointSliceState) - prometheus.MustRegister(NumberOfEndpoints) - prometheus.MustRegister(DualStackMigrationFinishedDurations) - prometheus.MustRegister(DualStackMigrationLongestUnfinishedDuration) - prometheus.MustRegister(DualStackMigrationServiceCount) - prometheus.MustRegister(SyncerCountByEndpointType) - prometheus.MustRegister(syncerSyncResult) -} - func (sm *SyncerMetrics) Run(stopCh <-chan struct{}) { sm.logger.V(3).Info("Syncer Metrics initialized.", "exportInterval", sm.metricsInterval) // Compute and export metrics periodically. diff --git a/pkg/neg/metrics/neg_metrics_collector_test.go b/pkg/neg/metrics/metricscollector/metrics_collector_test.go similarity index 95% rename from pkg/neg/metrics/neg_metrics_collector_test.go rename to pkg/neg/metrics/metricscollector/metrics_collector_test.go index c38bd0b67b..986bf83882 100644 --- a/pkg/neg/metrics/neg_metrics_collector_test.go +++ b/pkg/neg/metrics/metricscollector/metrics_collector_test.go @@ -1,4 +1,20 @@ -package metrics +/* +Copyright 2023 The Kubernetes Authors. + +Licensed 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 metricscollector import ( "fmt" diff --git a/pkg/neg/syncers/dualstack/migrator_test.go b/pkg/neg/syncers/dualstack/migrator_test.go index 5f8c5990c8..b4044667d4 100644 --- a/pkg/neg/syncers/dualstack/migrator_test.go +++ b/pkg/neg/syncers/dualstack/migrator_test.go @@ -8,7 +8,7 @@ import ( "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/types" "k8s.io/klog/v2/ktesting" "k8s.io/utils/clock" @@ -992,7 +992,7 @@ func cloneZoneNetworkEndpointsMap(m map[string]types.NetworkEndpointSet) map[str func newMigratorForTest(t *testing.T, enableDualStackNEG bool) *Migrator { logger, _ := ktesting.NewTestContext(t) - m := NewMigrator(enableDualStackNEG, &fakeSyncable{}, types.NegSyncerKey{}, metrics.FakeSyncerMetrics(), &fakeErrorStateChecker{}, logger) + m := NewMigrator(enableDualStackNEG, &fakeSyncable{}, types.NegSyncerKey{}, metricscollector.FakeSyncerMetrics(), &fakeErrorStateChecker{}, logger) m.clock = clocktesting.NewFakeClock(time.Now()) return m } diff --git a/pkg/neg/syncers/endpoints_calculator.go b/pkg/neg/syncers/endpoints_calculator.go index 1bd3f363a5..cae8883ea9 100644 --- a/pkg/neg/syncers/endpoints_calculator.go +++ b/pkg/neg/syncers/endpoints_calculator.go @@ -25,6 +25,7 @@ import ( listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/types" negtypes "k8s.io/ingress-gce/pkg/neg/types" "k8s.io/ingress-gce/pkg/network" @@ -213,10 +214,10 @@ type L7EndpointsCalculator struct { networkEndpointType types.NetworkEndpointType enableDualStackNEG bool logger klog.Logger - syncMetricsCollector *metrics.SyncerMetrics + syncMetricsCollector *metricscollector.SyncerMetrics } -func NewL7EndpointsCalculator(zoneGetter types.ZoneGetter, podLister, nodeLister, serviceLister cache.Indexer, syncerKey types.NegSyncerKey, logger klog.Logger, enableDualStackNEG bool, syncMetricsCollector *metrics.SyncerMetrics) *L7EndpointsCalculator { +func NewL7EndpointsCalculator(zoneGetter types.ZoneGetter, podLister, nodeLister, serviceLister cache.Indexer, syncerKey types.NegSyncerKey, logger klog.Logger, enableDualStackNEG bool, syncMetricsCollector *metricscollector.SyncerMetrics) *L7EndpointsCalculator { return &L7EndpointsCalculator{ zoneGetter: zoneGetter, servicePortName: syncerKey.PortTuple.Name, diff --git a/pkg/neg/syncers/endpoints_calculator_test.go b/pkg/neg/syncers/endpoints_calculator_test.go index 808fafa4a1..10fd44e27c 100644 --- a/pkg/neg/syncers/endpoints_calculator_test.go +++ b/pkg/neg/syncers/endpoints_calculator_test.go @@ -30,7 +30,7 @@ import ( "k8s.io/client-go/tools/cache" networkv1 "k8s.io/cloud-provider-gcp/crd/apis/network/v1" "k8s.io/cloud-provider-gcp/providers/gce" - "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" negtypes "k8s.io/ingress-gce/pkg/neg/types" "k8s.io/ingress-gce/pkg/network" "k8s.io/ingress-gce/pkg/utils" @@ -314,7 +314,7 @@ func TestValidateEndpoints(t *testing.T) { podLister := testContext.PodInformer.GetIndexer() nodeLister := testContext.NodeInformer.GetIndexer() serviceLister := testContext.ServiceInformer.GetIndexer() - L7EndpointsCalculator := NewL7EndpointsCalculator(zoneGetter, podLister, nodeLister, serviceLister, svcPort, klog.TODO(), testContext.EnableDualStackNEG, metrics.FakeSyncerMetrics()) + L7EndpointsCalculator := NewL7EndpointsCalculator(zoneGetter, podLister, nodeLister, serviceLister, svcPort, klog.TODO(), testContext.EnableDualStackNEG, metricscollector.FakeSyncerMetrics()) L4LocalEndpointCalculator := NewLocalL4ILBEndpointsCalculator(listers.NewNodeLister(nodeLister), zoneGetter, svcKey, klog.TODO(), &network.NetworkInfo{}) L4ClusterEndpointCalculator := NewClusterL4ILBEndpointsCalculator(listers.NewNodeLister(nodeLister), zoneGetter, svcKey, klog.TODO(), &network.NetworkInfo{}) diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go index 85bb983c05..76c073da0d 100644 --- a/pkg/neg/syncers/transaction.go +++ b/pkg/neg/syncers/transaction.go @@ -44,6 +44,7 @@ import ( negv1beta1 "k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1" "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/readiness" "k8s.io/ingress-gce/pkg/neg/syncers/dualstack" "k8s.io/ingress-gce/pkg/neg/syncers/labels" @@ -107,7 +108,7 @@ type transactionSyncer struct { errorState bool // syncMetricsCollector collect sync related metrics - syncMetricsCollector metrics.SyncerMetricsCollector + syncMetricsCollector metricscollector.SyncerMetricsCollector // enableDegradedMode indicates whether we do endpoint calculation using degraded mode procedures enableDegradedMode bool @@ -138,7 +139,7 @@ func NewTransactionSyncer( epc negtypes.NetworkEndpointsCalculator, kubeSystemUID string, svcNegClient svcnegclient.Interface, - syncerMetrics *metrics.SyncerMetrics, + syncerMetrics *metricscollector.SyncerMetrics, customName bool, log klog.Logger, lpConfig labels.PodLabelPropagationConfig, @@ -183,7 +184,7 @@ func NewTransactionSyncer( return syncer } -func GetEndpointsCalculator(podLister, nodeLister, serviceLister cache.Indexer, zoneGetter negtypes.ZoneGetter, syncerKey negtypes.NegSyncerKey, mode negtypes.EndpointsCalculatorMode, logger klog.Logger, enableDualStackNEG bool, syncMetricsCollector *metrics.SyncerMetrics, networkInfo *network.NetworkInfo) negtypes.NetworkEndpointsCalculator { +func GetEndpointsCalculator(podLister, nodeLister, serviceLister cache.Indexer, zoneGetter negtypes.ZoneGetter, syncerKey negtypes.NegSyncerKey, mode negtypes.EndpointsCalculatorMode, logger klog.Logger, enableDualStackNEG bool, syncMetricsCollector *metricscollector.SyncerMetrics, networkInfo *network.NetworkInfo) negtypes.NetworkEndpointsCalculator { serviceKey := strings.Join([]string{syncerKey.Name, syncerKey.Namespace}, "/") if syncerKey.NegType == negtypes.VmIpEndpointType { nodeLister := listers.NewNodeLister(nodeLister) @@ -953,8 +954,8 @@ func publishAnnotationSizeMetrics(endpoints map[string]negtypes.NetworkEndpointS } // collectLabelStats calculate the number of endpoints and the number of endpoints with annotations. -func collectLabelStats(currentPodLabelMap, addPodLabelMap labels.EndpointPodLabelMap, targetEndpointMap map[string]negtypes.NetworkEndpointSet) metrics.LabelPropagationStats { - labelPropagationStats := metrics.LabelPropagationStats{} +func collectLabelStats(currentPodLabelMap, addPodLabelMap labels.EndpointPodLabelMap, targetEndpointMap map[string]negtypes.NetworkEndpointSet) metricscollector.LabelPropagationStats { + labelPropagationStats := metricscollector.LabelPropagationStats{} for _, endpointSet := range targetEndpointMap { for endpoint := range endpointSet { labelPropagationStats.NumberOfEndpoints += 1 diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go index ac3cf68c96..9878fcb3f6 100644 --- a/pkg/neg/syncers/transaction_test.go +++ b/pkg/neg/syncers/transaction_test.go @@ -46,7 +46,7 @@ import ( negv1beta1 "k8s.io/ingress-gce/pkg/apis/svcneg/v1beta1" "k8s.io/ingress-gce/pkg/composite" "k8s.io/ingress-gce/pkg/flags" - "k8s.io/ingress-gce/pkg/neg/metrics" + "k8s.io/ingress-gce/pkg/neg/metrics/metricscollector" "k8s.io/ingress-gce/pkg/neg/readiness" "k8s.io/ingress-gce/pkg/neg/syncers/labels" negtypes "k8s.io/ingress-gce/pkg/neg/types" @@ -2005,14 +2005,14 @@ func TestCollectLabelStats(t *testing.T) { curLabelMap labels.EndpointPodLabelMap addLabelMap labels.EndpointPodLabelMap targetEndpointMap map[string]negtypes.NetworkEndpointSet - expect metrics.LabelPropagationStats + expect metricscollector.LabelPropagationStats }{ { desc: "Empty inputs", curLabelMap: labels.EndpointPodLabelMap{}, addLabelMap: labels.EndpointPodLabelMap{}, targetEndpointMap: map[string]negtypes.NetworkEndpointSet{}, - expect: metrics.LabelPropagationStats{ + expect: metricscollector.LabelPropagationStats{ EndpointsWithAnnotation: 0, NumberOfEndpoints: 0, }, @@ -2031,7 +2031,7 @@ func TestCollectLabelStats(t *testing.T) { endpoint2, ), }, - expect: metrics.LabelPropagationStats{ + expect: metricscollector.LabelPropagationStats{ EndpointsWithAnnotation: 1, NumberOfEndpoints: 2, }, @@ -2058,7 +2058,7 @@ func TestCollectLabelStats(t *testing.T) { endpoint4, ), }, - expect: metrics.LabelPropagationStats{ + expect: metricscollector.LabelPropagationStats{ EndpointsWithAnnotation: 2, NumberOfEndpoints: 4, }, @@ -2077,7 +2077,7 @@ func TestCollectLabelStats(t *testing.T) { endpoint4, ), }, - expect: metrics.LabelPropagationStats{ + expect: metricscollector.LabelPropagationStats{ EndpointsWithAnnotation: 1, NumberOfEndpoints: 2, }, @@ -2134,10 +2134,10 @@ func newTestTransactionSyncer(fakeGCE negtypes.NetworkEndpointGroupCloud, negTyp testContext.SvcNegInformer.GetIndexer(), reflector, GetEndpointsCalculator(testContext.PodInformer.GetIndexer(), testContext.NodeInformer.GetIndexer(), testContext.ServiceInformer.GetIndexer(), - fakeZoneGetter, svcPort, mode, klog.TODO(), testContext.EnableDualStackNEG, metrics.FakeSyncerMetrics(), &network.NetworkInfo{IsDefault: true}), + fakeZoneGetter, svcPort, mode, klog.TODO(), testContext.EnableDualStackNEG, metricscollector.FakeSyncerMetrics(), &network.NetworkInfo{IsDefault: true}), string(kubeSystemUID), testContext.SvcNegClient, - metrics.FakeSyncerMetrics(), + metricscollector.FakeSyncerMetrics(), customName, klog.TODO(), labels.PodLabelPropagationConfig{},