From 5164275e062226423a1f8d8a706f4cc926fbfc7a Mon Sep 17 00:00:00 2001 From: Sergei Razukov Date: Thu, 27 May 2021 17:40:54 +1000 Subject: [PATCH] Add ability to use custom prometheus buckets --- cmd/nginx/flags.go | 12 ++++++++++++ cmd/nginx/main.go | 2 +- internal/ingress/controller/controller.go | 2 ++ internal/ingress/metric/collectors/socket.go | 18 +++++++++++++++--- .../ingress/metric/collectors/socket_test.go | 13 ++++++++++++- internal/ingress/metric/main.go | 4 ++-- 6 files changed, 44 insertions(+), 7 deletions(-) diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index 72a2bfb8d3..f9d6702fe7 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -22,6 +22,7 @@ import ( "os" "time" + "github.com/prometheus/client_golang/prometheus" "github.com/spf13/pflag" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" @@ -29,6 +30,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/controller" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" + "k8s.io/ingress-nginx/internal/ingress/metric/collectors" "k8s.io/ingress-nginx/internal/ingress/status" ing_net "k8s.io/ingress-nginx/internal/net" "k8s.io/ingress-nginx/internal/nginx" @@ -161,6 +163,9 @@ Requires the update-status parameter.`) `Enables the collection of NGINX metrics`) metricsPerHost = flags.Bool("metrics-per-host", true, `Export metrics per-host`) + 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") monitorMaxBatchSize = flags.Int("monitor-max-batch-size", 10000, "Max batch size of NGINX metrics") httpPort = flags.Int("http-port", 80, `Port to use for servicing HTTP traffic.`) @@ -283,6 +288,12 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g } } + var histogramBuckets = &collectors.HistogramBuckets{ + TimeBuckets: *timeBuckets, + LengthBuckets: *lengthBuckets, + SizeBuckets: *sizeBuckets, + } + ngx_config.EnableSSLChainCompletion = *enableSSLChainCompletion config := &controller.Configuration{ @@ -293,6 +304,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g EnableProfiling: *profiling, EnableMetrics: *enableMetrics, MetricsPerHost: *metricsPerHost, + MetricsBuckets: histogramBuckets, MonitorMaxBatchSize: *monitorMaxBatchSize, DisableServiceExternalName: *disableServiceExternalName, EnableSSLPassthrough: *enableSSLPassthrough, diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index fb87ad84a1..cbfca547ce 100644 --- a/cmd/nginx/main.go +++ b/cmd/nginx/main.go @@ -133,7 +133,7 @@ func main() { mc := metric.NewDummyCollector() if conf.EnableMetrics { - mc, err = metric.NewCollector(conf.MetricsPerHost, reg, conf.IngressClassConfiguration.Controller) + mc, err = metric.NewCollector(conf.MetricsPerHost, reg, conf.IngressClassConfiguration.Controller, *conf.MetricsBuckets) if err != nil { klog.Fatalf("Error creating prometheus collector: %v", err) } diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 7c200bd8f9..a4ae4217c0 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -41,6 +41,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/internal/ingress/controller/store" "k8s.io/ingress-nginx/internal/ingress/errors" + "k8s.io/ingress-nginx/internal/ingress/metric/collectors" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/nginx" "k8s.io/klog/v2" @@ -97,6 +98,7 @@ type Configuration struct { EnableMetrics bool MetricsPerHost bool + MetricsBuckets *collectors.HistogramBuckets FakeCertificate *ingress.SSLCert diff --git a/internal/ingress/metric/collectors/socket.go b/internal/ingress/metric/collectors/socket.go index 8fec7ddedc..a220d14566 100644 --- a/internal/ingress/metric/collectors/socket.go +++ b/internal/ingress/metric/collectors/socket.go @@ -56,6 +56,13 @@ type socketData struct { Path string `json:"path"` } +// HistogramBuckets allow customizing prometheus histogram buckets values +type HistogramBuckets struct { + TimeBuckets []float64 + LengthBuckets []float64 + SizeBuckets []float64 +} + // SocketCollector stores prometheus metrics and ingress meta-data type SocketCollector struct { prometheus.Collector @@ -79,6 +86,8 @@ type SocketCollector struct { hosts sets.String metricsPerHost bool + + buckets HistogramBuckets } var ( @@ -101,7 +110,7 @@ var defObjectives = map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001} // NewSocketCollector creates a new SocketCollector instance using // the ingress watch namespace and class used by the controller -func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*SocketCollector, error) { +func NewSocketCollector(pod, namespace, class string, metricsPerHost bool, buckets HistogramBuckets) (*SocketCollector, error) { socket := "/tmp/prometheus-nginx.socket" // unix sockets must be unlink()ed before being used _ = syscall.Unlink(socket) @@ -138,6 +147,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*Soc Help: "The time spent on receiving the response from the upstream server", Namespace: PrometheusNamespace, ConstLabels: constLabels, + Buckets: buckets.TimeBuckets, }, requestTags, ), @@ -147,6 +157,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*Soc Help: "The response length (including request line, header, and request body)", Namespace: PrometheusNamespace, ConstLabels: constLabels, + Buckets: buckets.LengthBuckets, }, requestTags, ), @@ -157,6 +168,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*Soc Help: "The request processing time in milliseconds", Namespace: PrometheusNamespace, ConstLabels: constLabels, + Buckets: buckets.TimeBuckets, }, requestTags, ), @@ -165,7 +177,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*Soc Name: "request_size", Help: "The request length (including request line, header, and request body)", Namespace: PrometheusNamespace, - Buckets: prometheus.LinearBuckets(10, 10, 10), // 10 buckets, each 10 bytes wide. + Buckets: buckets.LengthBuckets, ConstLabels: constLabels, }, requestTags, @@ -186,7 +198,7 @@ func NewSocketCollector(pod, namespace, class string, metricsPerHost bool) (*Soc Name: "bytes_sent", Help: "The number of bytes sent to a client", Namespace: PrometheusNamespace, - Buckets: prometheus.ExponentialBuckets(10, 10, 7), // 7 buckets, exponential factor of 10. + Buckets: buckets.SizeBuckets, ConstLabels: constLabels, }, requestTags, diff --git a/internal/ingress/metric/collectors/socket_test.go b/internal/ingress/metric/collectors/socket_test.go index 6ab972f586..3b124efa09 100644 --- a/internal/ingress/metric/collectors/socket_test.go +++ b/internal/ingress/metric/collectors/socket_test.go @@ -68,6 +68,17 @@ func TestNewUDPLogListener(t *testing.T) { } func TestCollector(t *testing.T) { + + buckets := struct { + TimeBuckets []float64 + LengthBuckets []float64 + SizeBuckets []float64 + }{ + prometheus.DefBuckets, + prometheus.LinearBuckets(10, 10, 10), + prometheus.ExponentialBuckets(10, 10, 7), + } + cases := []struct { name string data []string @@ -338,7 +349,7 @@ func TestCollector(t *testing.T) { t.Run(c.name, func(t *testing.T) { registry := prometheus.NewPedanticRegistry() - sc, err := NewSocketCollector("pod", "default", "ingress", true) + sc, err := NewSocketCollector("pod", "default", "ingress", true, buckets) 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 index 0cc07fe288..644a9b690e 100644 --- a/internal/ingress/metric/main.go +++ b/internal/ingress/metric/main.go @@ -68,7 +68,7 @@ type collector struct { } // NewCollector creates a new metric collector the for ingress controller -func NewCollector(metricsPerHost bool, registry *prometheus.Registry, ingressclass string) (Collector, error) { +func NewCollector(metricsPerHost bool, registry *prometheus.Registry, ingressclass string, buckets collectors.HistogramBuckets) (Collector, error) { podNamespace := os.Getenv("POD_NAMESPACE") if podNamespace == "" { podNamespace = "default" @@ -86,7 +86,7 @@ func NewCollector(metricsPerHost bool, registry *prometheus.Registry, ingresscla return nil, err } - s, err := collectors.NewSocketCollector(podName, podNamespace, ingressclass, metricsPerHost) + s, err := collectors.NewSocketCollector(podName, podNamespace, ingressclass, metricsPerHost, buckets) if err != nil { return nil, err }