From f9d57a8d5db3e58f79a1b1958d80c049c63d6cde Mon Sep 17 00:00:00 2001 From: Adhityaa Chandrasekar Date: Wed, 4 Nov 2020 22:19:52 +0000 Subject: [PATCH 1/2] APF: use snake_case in metric labels Signed-off-by: Adhityaa Chandrasekar --- .../util/flowcontrol/fairqueuing/queueset/queueset_test.go | 6 +++--- .../apiserver/pkg/util/flowcontrol/metrics/metrics.go | 4 ++-- .../src/k8s.io/component-base/metrics/testutil/promlint.go | 5 ----- test/integration/apiserver/flowcontrol/concurrency_test.go | 2 +- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go index 7b26569924a..5ba0ddb81e7 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go @@ -167,7 +167,7 @@ func (uss *uniformScenarioState) exercise() { for i, uc := range uss.clients { uss.integrators[i] = fq.NewIntegrator(uss.clk) fsName := fmt.Sprintf("client%d", i) - uss.expectedInqueue = uss.expectedInqueue + fmt.Sprintf(` apiserver_flowcontrol_current_inqueue_requests{flowSchema=%q,priorityLevel=%q} 0%s`, fsName, uss.name, "\n") + uss.expectedInqueue = uss.expectedInqueue + fmt.Sprintf(` apiserver_flowcontrol_current_inqueue_requests{flow_schema=%q,priority_level=%q} 0%s`, fsName, uss.name, "\n") for j := 0; j < uc.nThreads; j++ { ust := uniformScenarioThread{ uss: uss, @@ -309,10 +309,10 @@ func (uss *uniformScenarioState) finalReview() { for i := range uss.clients { fsName := fmt.Sprintf("client%d", i) if atomic.AddInt32(&uss.executions[i], 0) > 0 { - uss.expectedExecuting = uss.expectedExecuting + fmt.Sprintf(` apiserver_flowcontrol_current_executing_requests{flowSchema=%q,priorityLevel=%q} 0%s`, fsName, uss.name, "\n") + uss.expectedExecuting = uss.expectedExecuting + fmt.Sprintf(` apiserver_flowcontrol_current_executing_requests{flow_schema=%q,priority_level=%q} 0%s`, fsName, uss.name, "\n") } if atomic.AddInt32(&uss.rejects[i], 0) > 0 { - expectedRejects = expectedRejects + fmt.Sprintf(` apiserver_flowcontrol_rejected_requests_total{flowSchema=%q,priorityLevel=%q,reason=%q} %d%s`, fsName, uss.name, uss.rejectReason, uss.rejects[i], "\n") + expectedRejects = expectedRejects + fmt.Sprintf(` apiserver_flowcontrol_rejected_requests_total{flow_schema=%q,priority_level=%q,reason=%q} %d%s`, fsName, uss.name, uss.rejectReason, uss.rejects[i], "\n") } } if uss.evalExecutingMetrics && len(uss.expectedExecuting) > 0 { diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go index 940dac48104..fc286fbc032 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go @@ -34,8 +34,8 @@ const ( const ( requestKind = "request_kind" - priorityLevel = "priorityLevel" - flowSchema = "flowSchema" + priorityLevel = "priority_level" + flowSchema = "flow_schema" phase = "phase" mark = "mark" ) diff --git a/staging/src/k8s.io/component-base/metrics/testutil/promlint.go b/staging/src/k8s.io/component-base/metrics/testutil/promlint.go index 4e270c4089a..33b83f05c5a 100644 --- a/staging/src/k8s.io/component-base/metrics/testutil/promlint.go +++ b/staging/src/k8s.io/component-base/metrics/testutil/promlint.go @@ -33,11 +33,6 @@ var exceptionMetrics = []string{ // k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/egressselector "apiserver_egress_dialer_dial_failure_count", // counter metrics should have "_total" suffix - // k8s.io/apiserver/pkg/util/flowcontrol/fairqueuing/queueset - "apiserver_flowcontrol_current_inqueue_requests", // label names should be written in 'snake_case' not 'camelCase', - "apiserver_flowcontrol_current_executing_requests", // label names should be written in 'snake_case' not 'camelCase' - "apiserver_flowcontrol_rejected_requests_total", // label names should be written in 'snake_case' not 'camelCase' - // k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/server/healthz "apiserver_request_total", // label names should be written in 'snake_case' not 'camelCase' diff --git a/test/integration/apiserver/flowcontrol/concurrency_test.go b/test/integration/apiserver/flowcontrol/concurrency_test.go index 0e1d440ea3e..8ba921cb932 100644 --- a/test/integration/apiserver/flowcontrol/concurrency_test.go +++ b/test/integration/apiserver/flowcontrol/concurrency_test.go @@ -45,7 +45,7 @@ const ( sharedConcurrencyMetricsName = "apiserver_flowcontrol_request_concurrency_limit" dispatchedRequestCountMetricsName = "apiserver_flowcontrol_dispatched_requests_total" rejectedRequestCountMetricsName = "apiserver_flowcontrol_rejected_requests_total" - labelPriorityLevel = "priorityLevel" + labelPriorityLevel = "priority_level" timeout = time.Second * 10 ) From b16f36b251ddbfef5f12fed58640de53512631f0 Mon Sep 17 00:00:00 2001 From: Adhityaa Chandrasekar Date: Thu, 5 Nov 2020 15:35:39 +0000 Subject: [PATCH 2/2] APF metrics: set StabilityLevel to ALPHA Signed-off-by: Adhityaa Chandrasekar --- .../pkg/util/flowcontrol/metrics/metrics.go | 78 ++++++++++--------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go index fc286fbc032..bdbaa94601f 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/util/flowcontrol/metrics/metrics.go @@ -84,19 +84,21 @@ func (rs Registerables) Append(more ...compbasemetrics.Registerable) Registerabl var ( apiserverRejectedRequestsTotal = compbasemetrics.NewCounterVec( &compbasemetrics.CounterOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "rejected_requests_total", - Help: "Number of requests rejected by API Priority and Fairness system", + Namespace: namespace, + Subsystem: subsystem, + Name: "rejected_requests_total", + Help: "Number of requests rejected by API Priority and Fairness system", + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema, "reason"}, ) apiserverDispatchedRequestsTotal = compbasemetrics.NewCounterVec( &compbasemetrics.CounterOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "dispatched_requests_total", - Help: "Number of requests released by API Priority and Fairness system for service", + Namespace: namespace, + Subsystem: subsystem, + Name: "dispatched_requests_total", + Help: "Number of requests released by API Priority and Fairness system for service", + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema}, ) @@ -143,58 +145,64 @@ var ( apiserverCurrentInqueueRequests = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "current_inqueue_requests", - Help: "Number of requests currently pending in queues of the API Priority and Fairness system", + Namespace: namespace, + Subsystem: subsystem, + Name: "current_inqueue_requests", + Help: "Number of requests currently pending in queues of the API Priority and Fairness system", + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema}, ) apiserverRequestQueueLength = compbasemetrics.NewHistogramVec( &compbasemetrics.HistogramOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "request_queue_length_after_enqueue", - Help: "Length of queue in the API Priority and Fairness system, as seen by each request after it is enqueued", - Buckets: queueLengthBuckets, + Namespace: namespace, + Subsystem: subsystem, + Name: "request_queue_length_after_enqueue", + Help: "Length of queue in the API Priority and Fairness system, as seen by each request after it is enqueued", + Buckets: queueLengthBuckets, + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema}, ) apiserverRequestConcurrencyLimit = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "request_concurrency_limit", - Help: "Shared concurrency limit in the API Priority and Fairness system", + Namespace: namespace, + Subsystem: subsystem, + Name: "request_concurrency_limit", + Help: "Shared concurrency limit in the API Priority and Fairness system", + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel}, ) apiserverCurrentExecutingRequests = compbasemetrics.NewGaugeVec( &compbasemetrics.GaugeOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "current_executing_requests", - Help: "Number of requests currently executing in the API Priority and Fairness system", + Namespace: namespace, + Subsystem: subsystem, + Name: "current_executing_requests", + Help: "Number of requests currently executing in the API Priority and Fairness system", + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema}, ) apiserverRequestWaitingSeconds = compbasemetrics.NewHistogramVec( &compbasemetrics.HistogramOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "request_wait_duration_seconds", - Help: "Length of time a request spent waiting in its queue", - Buckets: requestDurationSecondsBuckets, + Namespace: namespace, + Subsystem: subsystem, + Name: "request_wait_duration_seconds", + Help: "Length of time a request spent waiting in its queue", + Buckets: requestDurationSecondsBuckets, + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema, "execute"}, ) apiserverRequestExecutionSeconds = compbasemetrics.NewHistogramVec( &compbasemetrics.HistogramOpts{ - Namespace: namespace, - Subsystem: subsystem, - Name: "request_execution_seconds", - Help: "Duration of request execution in the API Priority and Fairness system", - Buckets: requestDurationSecondsBuckets, + Namespace: namespace, + Subsystem: subsystem, + Name: "request_execution_seconds", + Help: "Duration of request execution in the API Priority and Fairness system", + Buckets: requestDurationSecondsBuckets, + StabilityLevel: compbasemetrics.ALPHA, }, []string{priorityLevel, flowSchema}, )