Added native histogram support for histogram metrics

Signed-off-by: Sebastian Rabenhorst <[email protected]> Fixed flag Added cli docs Fxi cli args doc Fxi cli args doc Fxi cli args doc Fxi cli args doc Revert "Fxi cli args doc" This reverts commit b0dd2de6e032cefbdb06cead85b0b9be16abcd22. Revert "Fxi cli args doc" This reverts commit 6afc0f7a1c9bfba4ee75dbb99be1b9fbba4df156. Revert "Fxi cli args doc" This reverts commit 1f6c408e1220745bd1279b1aeaf854c748adf754. Revert "Fxi cli args doc" This reverts commit 68beccbd50be56b4f8aa461b6c3685f927e02413. Fix cli args docs Fix cli args docs Fix cli args docs
kubernetes · Aug 23, 2024 · fcbcc7b · fcbcc7b
1 parent 7b4e4e2
commit fcbcc7b
Show file tree

Hide file tree

Showing 9 changed files with 128 additions and 38 deletions.
diff --git a/cmd/dataplane/main.go b/cmd/dataplane/main.go
@@ -66,7 +66,7 @@ func main() {
 	mc := metric.NewDummyCollector()
 	if conf.EnableMetrics {
 		// TODO: Ingress class is not a part of dataplane anymore
-		mc, err = metric.NewCollector(conf.MetricsPerHost, conf.ReportStatusClasses, reg, conf.IngressClassConfiguration.Controller, *conf.MetricsBuckets, conf.ExcludeSocketMetrics)
+		mc, err = metric.NewCollector(conf.MetricsPerHost, conf.ReportStatusClasses, reg, conf.IngressClassConfiguration.Controller, *conf.MetricsBuckets, conf.MetricsBucketFactor, conf.MetricsMaxBuckets, conf.ExcludeSocketMetrics)
 		if err != nil {
 			klog.Fatalf("Error creating prometheus collector:  %v", err)
 		}

diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go
@@ -130,7 +130,7 @@ func main() {
 
 	mc := metric.NewDummyCollector()
 	if conf.EnableMetrics {
-		mc, err = metric.NewCollector(conf.MetricsPerHost, conf.ReportStatusClasses, reg, conf.IngressClassConfiguration.Controller, *conf.MetricsBuckets, conf.ExcludeSocketMetrics)
+		mc, err = metric.NewCollector(conf.MetricsPerHost, conf.ReportStatusClasses, reg, conf.IngressClassConfiguration.Controller, *conf.MetricsBuckets, conf.MetricsBucketFactor, conf.MetricsMaxBuckets, conf.ExcludeSocketMetrics)
 		if err != nil {
 			klog.Fatalf("Error creating prometheus collector:  %v", err)
 		}

diff --git a/docs/user-guide/cli-arguments.md b/docs/user-guide/cli-arguments.md
@@ -8,6 +8,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment
 |----------|-------------|
 | `--annotations-prefix`             | Prefix of the Ingress annotations specific to the NGINX controller. (default "nginx.ingress.kubernetes.io") |
 | `--apiserver-host`                 | Address of the Kubernetes API server. Takes the form "protocol://address:port". If not specified, it is assumed the program runs inside a Kubernetes cluster and local discovery is attempted. |
+| `--bucket-factor`                    | Bucket factor for native histograms. Value must be > 1 for enabling native histograms. (default 0) |
 | `--certificate-authority`          | Path to a cert file for the certificate authority. This certificate is used only when the flag --apiserver-host is specified. |
 | `--configmap`                      | Name of the ConfigMap containing custom global configurations for the controller. |
 | `--controller-class`                      | Ingress Class Controller value this Ingress satisfies. The class of an Ingress object is set using the field IngressClassName in Kubernetes clusters version v1.19.0 or higher. The .spec.controller value of the IngressClass referenced in an Ingress Object should be the same value specified here to make this object be watched. |
@@ -40,6 +41,7 @@ They are set in the container spec of the `ingress-nginx-controller` Deployment
 | `--internal-logger-address`        | Address to be used when binding internal syslogger. (default 127.0.0.1:11514) |
 | `--kubeconfig`                     | Path to a kubeconfig file containing authorization and API server information. |
 | `--length-buckets`                     | Set of buckets which will be used for prometheus histogram metrics such as RequestLength, ResponseLength. (default `[10, 20, 30, 40, 50, 60, 70, 80, 90, 100]`) |
+| `--max-buckets`                      | Maximum number of buckets for native histograms. (default 100) |
 | `--maxmind-edition-ids`            | Maxmind edition ids to download GeoLite2 Databases. (default "GeoLite2-City,GeoLite2-ASN") |
 | `--maxmind-retries-timeout`        | Maxmind downloading delay between 1st and 2nd attempt, 0s - do not retry to download if something went wrong. (default 0s) |
 | `--maxmind-retries-count`          | Number of attempts to download the GeoIP DB. (default 1) |

diff --git a/go.work.sum b/go.work.sum
diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go
@@ -108,6 +108,8 @@ type Configuration struct {
 	EnableMetrics        bool
 	MetricsPerHost       bool
 	MetricsBuckets       *collectors.HistogramBuckets
+	MetricsBucketFactor  float64
+	MetricsMaxBuckets    uint32
 	ReportStatusClasses  bool
 	ExcludeSocketMetrics []string
 

diff --git a/internal/ingress/metric/collectors/socket.go b/internal/ingress/metric/collectors/socket.go
@@ -99,7 +99,7 @@ var requestTags = []string{
 
 // NewSocketCollector creates a new SocketCollector instance using
 // the ingress watch namespace and class used by the controller
-func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStatusClasses bool, buckets HistogramBuckets, excludeMetrics []string) (*SocketCollector, error) {
+func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStatusClasses bool, buckets HistogramBuckets, bucketFactor float64, maxBuckets uint32, excludeMetrics []string) (*SocketCollector, error) {
 	socket := "/tmp/nginx/prometheus-nginx.socket"
 	// unix sockets must be unlink()ed before being used
 	//nolint:errcheck // Ignore unlink error
@@ -144,11 +144,13 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStat
 
 		connectTime: histogramMetric(
 			&prometheus.HistogramOpts{
-				Name:        "connect_duration_seconds",
-				Help:        "The time spent on establishing a connection with the upstream server",
-				Namespace:   PrometheusNamespace,
-				ConstLabels: constLabels,
-				Buckets:     buckets.TimeBuckets,
+				Name:                           "connect_duration_seconds",
+				Help:                           "The time spent on establishing a connection with the upstream server",
+				Namespace:                      PrometheusNamespace,
+				ConstLabels:                    constLabels,
+				Buckets:                        buckets.TimeBuckets,
+				NativeHistogramBucketFactor:    bucketFactor,
+				NativeHistogramMaxBucketNumber: maxBuckets,
 			},
 			requestTags,
 			em,
@@ -157,23 +159,27 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStat
 
 		headerTime: histogramMetric(
 			&prometheus.HistogramOpts{
-				Name:        "header_duration_seconds",
-				Help:        "The time spent on receiving first header from the upstream server",
-				Namespace:   PrometheusNamespace,
-				ConstLabels: constLabels,
-				Buckets:     buckets.TimeBuckets,
+				Name:                           "header_duration_seconds",
+				Help:                           "The time spent on receiving first header from the upstream server",
+				Namespace:                      PrometheusNamespace,
+				ConstLabels:                    constLabels,
+				Buckets:                        buckets.TimeBuckets,
+				NativeHistogramBucketFactor:    bucketFactor,
+				NativeHistogramMaxBucketNumber: maxBuckets,
 			},
 			requestTags,
 			em,
 			mm,
 		),
 		responseTime: histogramMetric(
 			&prometheus.HistogramOpts{
-				Name:        "response_duration_seconds",
-				Help:        "The time spent on receiving the response from the upstream server",
-				Namespace:   PrometheusNamespace,
-				ConstLabels: constLabels,
-				Buckets:     buckets.TimeBuckets,
+				Name:                           "response_duration_seconds",
+				Help:                           "The time spent on receiving the response from the upstream server",
+				Namespace:                      PrometheusNamespace,
+				ConstLabels:                    constLabels,
+				Buckets:                        buckets.TimeBuckets,
+				NativeHistogramBucketFactor:    bucketFactor,
+				NativeHistogramMaxBucketNumber: maxBuckets,
 			},
 			requestTags,
 			em,
@@ -182,11 +188,13 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStat
 
 		requestTime: histogramMetric(
 			&prometheus.HistogramOpts{
-				Name:        "request_duration_seconds",
-				Help:        "The request processing time in milliseconds",
-				Namespace:   PrometheusNamespace,
-				ConstLabels: constLabels,
-				Buckets:     buckets.TimeBuckets,
+				Name:                           "request_duration_seconds",
+				Help:                           "The request processing time in milliseconds",
+				Namespace:                      PrometheusNamespace,
+				ConstLabels:                    constLabels,
+				Buckets:                        buckets.TimeBuckets,
+				NativeHistogramBucketFactor:    bucketFactor,
+				NativeHistogramMaxBucketNumber: maxBuckets,
 			},
 			requestTags,
 			em,
@@ -195,11 +203,13 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStat
 
 		responseLength: histogramMetric(
 			&prometheus.HistogramOpts{
-				Name:        "response_size",
-				Help:        "The response length (including request line, header, and request body)",
-				Namespace:   PrometheusNamespace,
-				ConstLabels: constLabels,
-				Buckets:     buckets.LengthBuckets,
+				Name:                           "response_size",
+				Help:                           "The response length (including request line, header, and request body)",
+				Namespace:                      PrometheusNamespace,
+				ConstLabels:                    constLabels,
+				Buckets:                        buckets.LengthBuckets,
+				NativeHistogramBucketFactor:    bucketFactor,
+				NativeHistogramMaxBucketNumber: maxBuckets,
 			},
 			requestTags,
 			em,
@@ -208,11 +218,13 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost, reportStat
 
 		requestLength: histogramMetric(
 			&prometheus.HistogramOpts{
-				Name:        "request_size",
-				Help:        "The request length (including request line, header, and request body)",
-				Namespace:   PrometheusNamespace,
-				ConstLabels: constLabels,
-				Buckets:     buckets.LengthBuckets,
+				Name:                           "request_size",
+				Help:                           "The request length (including request line, header, and request body)",
+				Namespace:                      PrometheusNamespace,
+				ConstLabels:                    constLabels,
+				Buckets:                        buckets.LengthBuckets,
+				NativeHistogramBucketFactor:    bucketFactor,
+				NativeHistogramMaxBucketNumber: maxBuckets,
 			},
 			requestTags,
 			em,

diff --git a/internal/ingress/metric/collectors/socket_test.go b/internal/ingress/metric/collectors/socket_test.go
@@ -83,6 +83,9 @@ func TestCollector(t *testing.T) {
 		prometheus.ExponentialBuckets(10, 10, 7),
 	}
 
+	bucketFactor := 1.1
+	maxBuckets := uint32(100)
+
 	cases := []struct {
 		name             string
 		data             []string
@@ -594,7 +597,7 @@ func TestCollector(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			registry := prometheus.NewPedanticRegistry()
 
-			sc, err := NewSocketCollector("pod", "default", "ingress", true, c.useStatusClasses, buckets, c.excludeMetrics)
+			sc, err := NewSocketCollector("pod", "default", "ingress", true, c.useStatusClasses, buckets, bucketFactor, maxBuckets, c.excludeMetrics)
 			if err != nil {
 				t.Errorf("%v: unexpected error creating new SocketCollector: %v", c.name, err)
 			}

diff --git a/internal/ingress/metric/main.go b/internal/ingress/metric/main.go
@@ -71,7 +71,7 @@ type collector struct {
 }
 
 // NewCollector creates a new metric collector the for ingress controller
-func NewCollector(metricsPerHost, reportStatusClasses bool, registry *prometheus.Registry, ingressclass string, buckets collectors.HistogramBuckets, excludedSocketMetrics []string) (Collector, error) {
+func NewCollector(metricsPerHost, reportStatusClasses bool, registry *prometheus.Registry, ingressclass string, buckets collectors.HistogramBuckets, bucketFactor float64, maxBuckets uint32, excludedSocketMetrics []string) (Collector, error) {
 	podNamespace := os.Getenv("POD_NAMESPACE")
 	if podNamespace == "" {
 		podNamespace = "default"
@@ -89,7 +89,7 @@ func NewCollector(metricsPerHost, reportStatusClasses bool, registry *prometheus
 		return nil, err
 	}
 
-	s, err := collectors.NewSocketCollector(podName, podNamespace, ingressclass, metricsPerHost, reportStatusClasses, buckets, excludedSocketMetrics)
+	s, err := collectors.NewSocketCollector(podName, podNamespace, ingressclass, metricsPerHost, reportStatusClasses, buckets, bucketFactor, maxBuckets, excludedSocketMetrics)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go
@@ -183,6 +183,8 @@ Requires the update-status parameter.`)
 		timeBuckets          = flags.Float64Slice("time-buckets", prometheus.DefBuckets, "Set of buckets which will be used for prometheus histogram metrics such as RequestTime, ResponseTime.")
 		lengthBuckets        = flags.Float64Slice("length-buckets", prometheus.LinearBuckets(10, 10, 10), "Set of buckets which will be used for prometheus histogram metrics such as RequestLength, ResponseLength.")
 		sizeBuckets          = flags.Float64Slice("size-buckets", prometheus.ExponentialBuckets(10, 10, 7), "Set of buckets which will be used for prometheus histogram metrics such as BytesSent.")
+		bucketFactor         = flags.Float64("bucket-factor", 0, "Bucket factor for native histograms. Value must be > 1 for enabling native histograms.")
+		maxBuckets           = flags.Uint32("max-buckets", 100, "Maximum number of buckets for native histograms.")
 		excludeSocketMetrics = flags.StringSlice("exclude-socket-metrics", []string{}, "et of socket request metrics to exclude which won't be exported nor being calculated. E.g. 'nginx_ingress_controller_success,nginx_ingress_controller_header_duration_seconds'.")
 		monitorMaxBatchSize  = flags.Int("monitor-max-batch-size", 10000, "Max batch size of NGINX metrics.")
 
@@ -339,6 +341,8 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g
 		EnableMetrics:               *enableMetrics,
 		MetricsPerHost:              *metricsPerHost,
 		MetricsBuckets:              histogramBuckets,
+		MetricsBucketFactor:         *bucketFactor,
+		MetricsMaxBuckets:           *maxBuckets,
 		ReportStatusClasses:         *reportStatusClasses,
 		ExcludeSocketMetrics:        *excludeSocketMetrics,
 		MonitorMaxBatchSize:         *monitorMaxBatchSize,