From 0a658e4227a8318b3b68de8faf3d5cb7f29576f0 Mon Sep 17 00:00:00 2001 From: Tom Booth Date: Tue, 22 Aug 2017 18:27:02 +0100 Subject: [PATCH 1/5] When a location belongs to a Service key metrics We want to be able to track the metrics for a particular service or namespace, so that we can attribute the issues we see in aggregate in server/upstream errors. --- controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 20e659214..e51aedd36 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -781,6 +781,11 @@ stream { proxy_set_header Accept-Encoding ""; {{ end }} + {{ if (and $all.Cfg.EnableVtsStatus $location.Service) }} + vhost_traffic_status_filter_by_set_key {{ $location.Service.ObjectMeta.Namespace }} kubernetes::namespace; + vhost_traffic_status_filter_by_set_key {{ $location.Service.ObjectMeta.Namespace }}.{{ $location.Service.ObjectMeta.Name }} kubernetes::service; + {{ end }} + {{/* Add any additional configuration defined */}} {{ $location.ConfigurationSnippet }} From 21a0a90b5532013541907bb7642f207dfa743ea2 Mon Sep 17 00:00:00 2001 From: Tom Booth Date: Wed, 23 Aug 2017 11:39:46 +0100 Subject: [PATCH 2/5] Make filterZone metrics more generic We don't only have country filters now, so structure the metrics to handle this --- controllers/nginx/pkg/metric/collector/vts.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/controllers/nginx/pkg/metric/collector/vts.go b/controllers/nginx/pkg/metric/collector/vts.go index 33eeac492..45656d171 100644 --- a/controllers/nginx/pkg/metric/collector/vts.go +++ b/controllers/nginx/pkg/metric/collector/vts.go @@ -97,17 +97,17 @@ func NewNGINXVTSCollector(watchNamespace, ingressClass string, port int, path st filterZoneBytes: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_bytes_total"), "Nginx bytes count", - []string{"ingress_class", "namespace", "server_zone", "key", "direction"}, nil), + []string{"ingress_class", "namespace", "filter_name", "filter_key", "direction"}, nil), filterZoneResponses: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_responses_total"), "The number of responses with status codes 1xx, 2xx, 3xx, 4xx, and 5xx.", - []string{"ingress_class", "namespace", "server_zone", "key", "status_code"}, nil), + []string{"ingress_class", "namespace", "filter_name", "filter_key", "status_code"}, nil), filterZoneCache: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_cache_total"), "Nginx cache count", - []string{"ingress_class", "namespace", "server_zone", "key", "type"}, nil), + []string{"ingress_class", "namespace", "filter_name", "filter_key", "type"}, nil), upstreamBackup: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "upstream_backup"), @@ -247,15 +247,15 @@ func (p vtsCollector) scrapeVts(ch chan<- prometheus.Metric) { prometheus.CounterValue, zone.OutBytes, p.ingressClass, p.watchNamespace, name, "out") } - for serverZone, keys := range nginxMetrics.FilterZones { - for name, zone := range keys { - reflectMetrics(&zone.Responses, p.data.filterZoneResponses, ch, p.ingressClass, p.watchNamespace, serverZone, name) - reflectMetrics(&zone.Cache, p.data.filterZoneCache, ch, p.ingressClass, p.watchNamespace, serverZone, name) + for filterName, filterKeys := range nginxMetrics.FilterZones { + for filterKey, zone := range filterKeys { + reflectMetrics(&zone.Responses, p.data.filterZoneResponses, ch, p.ingressClass, p.watchNamespace, filterName, filterKey) + reflectMetrics(&zone.Cache, p.data.filterZoneCache, ch, p.ingressClass, p.watchNamespace, filterName, filterKey) ch <- prometheus.MustNewConstMetric(p.data.filterZoneBytes, - prometheus.CounterValue, zone.InBytes, p.ingressClass, p.watchNamespace, serverZone, name, "in") + prometheus.CounterValue, zone.InBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, "in") ch <- prometheus.MustNewConstMetric(p.data.filterZoneBytes, - prometheus.CounterValue, zone.OutBytes, p.ingressClass, p.watchNamespace, serverZone, name, "out") + prometheus.CounterValue, zone.OutBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, "out") } } } From 5159e0c0f7c588ef798f1a8bd9311103a8be4d75 Mon Sep 17 00:00:00 2001 From: Tom Booth Date: Wed, 23 Aug 2017 14:21:09 +0100 Subject: [PATCH 3/5] Include the average request time for filter zones We would like to know what the request time is for a given service exposed through this ingress --- .../nginx/pkg/metric/collector/status.go | 1 + controllers/nginx/pkg/metric/collector/vts.go | 42 +++++++++++-------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/controllers/nginx/pkg/metric/collector/status.go b/controllers/nginx/pkg/metric/collector/status.go index e68e08b5d..4d66f7eb5 100644 --- a/controllers/nginx/pkg/metric/collector/status.go +++ b/controllers/nginx/pkg/metric/collector/status.go @@ -81,6 +81,7 @@ type filterZone struct { InBytes float64 `json:"inBytes"` OutBytes float64 `json:"outBytes"` Cache cache `json:"cache"` + RequestMsec float64 `json:"requestMsec"` Responses response `json:"responses"` } diff --git a/controllers/nginx/pkg/metric/collector/vts.go b/controllers/nginx/pkg/metric/collector/vts.go index 45656d171..8fbcd989d 100644 --- a/controllers/nginx/pkg/metric/collector/vts.go +++ b/controllers/nginx/pkg/metric/collector/vts.go @@ -37,23 +37,24 @@ type ( } vtsData struct { - bytes *prometheus.Desc - cache *prometheus.Desc - connections *prometheus.Desc - responses *prometheus.Desc - requests *prometheus.Desc - filterZoneBytes *prometheus.Desc - filterZoneResponses *prometheus.Desc - filterZoneCache *prometheus.Desc - upstreamBackup *prometheus.Desc - upstreamBytes *prometheus.Desc - upstreamDown *prometheus.Desc - upstreamFailTimeout *prometheus.Desc - upstreamMaxFails *prometheus.Desc - upstreamResponses *prometheus.Desc - upstreamRequests *prometheus.Desc - upstreamResponseMsec *prometheus.Desc - upstreamWeight *prometheus.Desc + bytes *prometheus.Desc + cache *prometheus.Desc + connections *prometheus.Desc + responses *prometheus.Desc + requests *prometheus.Desc + filterZoneBytes *prometheus.Desc + filterZoneResponses *prometheus.Desc + filterZoneRequestMsec *prometheus.Desc + filterZoneCache *prometheus.Desc + upstreamBackup *prometheus.Desc + upstreamBytes *prometheus.Desc + upstreamDown *prometheus.Desc + upstreamFailTimeout *prometheus.Desc + upstreamMaxFails *prometheus.Desc + upstreamResponses *prometheus.Desc + upstreamRequests *prometheus.Desc + upstreamResponseMsec *prometheus.Desc + upstreamWeight *prometheus.Desc } ) @@ -104,6 +105,11 @@ func NewNGINXVTSCollector(watchNamespace, ingressClass string, port int, path st "The number of responses with status codes 1xx, 2xx, 3xx, 4xx, and 5xx.", []string{"ingress_class", "namespace", "filter_name", "filter_key", "status_code"}, nil), + filterZoneRequestMsec: prometheus.NewDesc( + prometheus.BuildFQName(ns, "", "filterzone_request_msecs_avg"), + "The average of only filter zone request processing times in milliseconds.", + []string{"ingress_class", "namespace", "filter_name", "filter_key"}, nil), + filterZoneCache: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_cache_total"), "Nginx cache count", @@ -252,6 +258,8 @@ func (p vtsCollector) scrapeVts(ch chan<- prometheus.Metric) { reflectMetrics(&zone.Responses, p.data.filterZoneResponses, ch, p.ingressClass, p.watchNamespace, filterName, filterKey) reflectMetrics(&zone.Cache, p.data.filterZoneCache, ch, p.ingressClass, p.watchNamespace, filterName, filterKey) + ch <- prometheus.MustNewConstMetric(p.data.filterZoneRequestMsec, + prometheus.CounterValue, zone.RequestMsec, p.ingressClass, p.watchNamespace, filterName, filterKey) ch <- prometheus.MustNewConstMetric(p.data.filterZoneBytes, prometheus.CounterValue, zone.InBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, "in") ch <- prometheus.MustNewConstMetric(p.data.filterZoneBytes, From 20b64eb069255d1495304959c6e61eaf5c671b1f Mon Sep 17 00:00:00 2001 From: Tom Booth Date: Wed, 23 Aug 2017 18:11:35 +0100 Subject: [PATCH 4/5] Collect nginx metrics keyed by Ingress too Store the ingress resource that created a particular location so that we can then aggregate nginx metrics based on the ingress name too. --- controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl | 7 ++++++- core/pkg/ingress/controller/controller.go | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index e51aedd36..c06554890 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -781,10 +781,15 @@ stream { proxy_set_header Accept-Encoding ""; {{ end }} - {{ if (and $all.Cfg.EnableVtsStatus $location.Service) }} + {{ if $all.Cfg.EnableVtsStatus }} + {{ if $location.Service }} vhost_traffic_status_filter_by_set_key {{ $location.Service.ObjectMeta.Namespace }} kubernetes::namespace; vhost_traffic_status_filter_by_set_key {{ $location.Service.ObjectMeta.Namespace }}.{{ $location.Service.ObjectMeta.Name }} kubernetes::service; {{ end }} + {{ if $location.Ingress }} + vhost_traffic_status_filter_by_set_key {{ $location.Ingress.ObjectMeta.Namespace }}.{{ $location.Ingress.ObjectMeta.Name }} kubernetes::ingress; + {{ end }} + {{ end }} {{/* Add any additional configuration defined */}} {{ $location.ConfigurationSnippet }} diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 37df71a03..666b62f88 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -979,6 +979,7 @@ func (ic *GenericController) createServers(data []*extensions.Ingress, Backend: un, Proxy: ngxProxy, Service: &apiv1.Service{}, + Ingress: ing, }, }, SSLPassthrough: sslpt, From f4a6cbc4d42647a62df1734f5eddce3a0e9c122b Mon Sep 17 00:00:00 2001 From: Tom Booth Date: Wed, 30 Aug 2017 10:42:37 +0100 Subject: [PATCH 5/5] Reinstate old labels for filter metrics To ensure backwards compatibility for this change we should still use the old labels for metrics. We still introduce the new style more generic labels. --- controllers/nginx/pkg/metric/collector/vts.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/controllers/nginx/pkg/metric/collector/vts.go b/controllers/nginx/pkg/metric/collector/vts.go index 8fbcd989d..632083eab 100644 --- a/controllers/nginx/pkg/metric/collector/vts.go +++ b/controllers/nginx/pkg/metric/collector/vts.go @@ -98,22 +98,22 @@ func NewNGINXVTSCollector(watchNamespace, ingressClass string, port int, path st filterZoneBytes: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_bytes_total"), "Nginx bytes count", - []string{"ingress_class", "namespace", "filter_name", "filter_key", "direction"}, nil), + []string{"ingress_class", "namespace", "server_zone", "country", "filter_name", "filter_key", "direction"}, nil), filterZoneResponses: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_responses_total"), "The number of responses with status codes 1xx, 2xx, 3xx, 4xx, and 5xx.", - []string{"ingress_class", "namespace", "filter_name", "filter_key", "status_code"}, nil), + []string{"ingress_class", "namespace", "server_zone", "country", "filter_name", "filter_key", "status_code"}, nil), filterZoneRequestMsec: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_request_msecs_avg"), "The average of only filter zone request processing times in milliseconds.", - []string{"ingress_class", "namespace", "filter_name", "filter_key"}, nil), + []string{"ingress_class", "namespace", "server_zone", "country", "filter_name", "filter_key"}, nil), filterZoneCache: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "filterzone_cache_total"), "Nginx cache count", - []string{"ingress_class", "namespace", "filter_name", "filter_key", "type"}, nil), + []string{"ingress_class", "namespace", "server_zone", "country", "filter_name", "filter_key", "type"}, nil), upstreamBackup: prometheus.NewDesc( prometheus.BuildFQName(ns, "", "upstream_backup"), @@ -255,15 +255,15 @@ func (p vtsCollector) scrapeVts(ch chan<- prometheus.Metric) { for filterName, filterKeys := range nginxMetrics.FilterZones { for filterKey, zone := range filterKeys { - reflectMetrics(&zone.Responses, p.data.filterZoneResponses, ch, p.ingressClass, p.watchNamespace, filterName, filterKey) - reflectMetrics(&zone.Cache, p.data.filterZoneCache, ch, p.ingressClass, p.watchNamespace, filterName, filterKey) + reflectMetrics(&zone.Responses, p.data.filterZoneResponses, ch, p.ingressClass, p.watchNamespace, filterName, filterKey, filterName, filterKey) + reflectMetrics(&zone.Cache, p.data.filterZoneCache, ch, p.ingressClass, p.watchNamespace, filterName, filterKey, filterName, filterKey) ch <- prometheus.MustNewConstMetric(p.data.filterZoneRequestMsec, - prometheus.CounterValue, zone.RequestMsec, p.ingressClass, p.watchNamespace, filterName, filterKey) + prometheus.CounterValue, zone.RequestMsec, p.ingressClass, p.watchNamespace, filterName, filterKey, filterName, filterKey) ch <- prometheus.MustNewConstMetric(p.data.filterZoneBytes, - prometheus.CounterValue, zone.InBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, "in") + prometheus.CounterValue, zone.InBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, filterName, filterKey, "in") ch <- prometheus.MustNewConstMetric(p.data.filterZoneBytes, - prometheus.CounterValue, zone.OutBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, "out") + prometheus.CounterValue, zone.OutBytes, p.ingressClass, p.watchNamespace, filterName, filterKey, filterName, filterKey, "out") } } }