Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flag to set custom buckets for prometheus histogram #7171

Merged
merged 1 commit into from
Jan 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cmd/nginx/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ import (
"os"
"time"

"github.com/prometheus/client_golang/prometheus"
"github.com/spf13/pflag"
apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"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"
Expand Down Expand Up @@ -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.`)
Expand Down Expand Up @@ -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{
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cmd/nginx/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -97,6 +98,7 @@ type Configuration struct {

EnableMetrics bool
MetricsPerHost bool
MetricsBuckets *collectors.HistogramBuckets

FakeCertificate *ingress.SSLCert

Expand Down
18 changes: 15 additions & 3 deletions internal/ingress/metric/collectors/socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -79,6 +86,8 @@ type SocketCollector struct {
hosts sets.String

metricsPerHost bool

buckets HistogramBuckets
}

var (
Expand All @@ -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)
Expand Down Expand Up @@ -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,
),
Expand All @@ -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,
),
Expand All @@ -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,
),
Expand All @@ -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,
Expand All @@ -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,
Expand Down
13 changes: 12 additions & 1 deletion internal/ingress/metric/collectors/socket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/ingress/metric/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
}
Expand Down