Skip to content

Commit

Permalink
Add reason label to total_reload metric
Browse files Browse the repository at this point in the history
  • Loading branch information
LorcanMcVeigh authored Aug 24, 2020
1 parent 679fc80 commit 7139569
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 33 deletions.
2 changes: 1 addition & 1 deletion docs-web/logging-and-monitoring/prometheus.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ The Ingress Controller exports the following metrics:
* NGINX/NGINX Plus metrics. Please see this [doc](https://github.com/nginxinc/nginx-prometheus-exporter#exported-metrics) to find more information about the exported metrics.

* Ingress Controller metrics
* `controller_nginx_reloads_total`. Number of successful NGINX reloads.
* `controller_nginx_reloads_total`. Number of successful NGINX reloads. This includes the label `reason` with 2 possible values `endpoints` (the reason for the reload was an endpoints update) and `other` (the reload was caused by something other than an endpoint update like an ingress update).
* `controller_nginx_reload_errors_total`. Number of unsuccessful NGINX reloads.
* `controller_nginx_last_reload_status`. Status of the last NGINX reload, 0 meaning down and 1 up.
* `controller_nginx_last_reload_milliseconds`. Duration in milliseconds of the last NGINX reload.
Expand Down
44 changes: 22 additions & 22 deletions internal/configs/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error {
return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}

Expand Down Expand Up @@ -222,7 +222,7 @@ func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIng
return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}

Expand Down Expand Up @@ -309,7 +309,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServer
return warnings, fmt.Errorf("Error adding or updating VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return warnings, fmt.Errorf("Error reloading NGINX for VirtualServer %v/%v: %v", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err)
}

Expand Down Expand Up @@ -356,7 +356,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS
allWarnings.Add(warnings)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return allWarnings, fmt.Errorf("Error when reloading NGINX when updating Policy: %v", err)
}

Expand All @@ -371,7 +371,7 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport
return fmt.Errorf("Error adding or updating TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX for TransportServer %v/%v: %v", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err)
}

Expand Down Expand Up @@ -530,7 +530,7 @@ func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, ingExes []I
}
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading NGINX when updating Secret: %v", err)
}

Expand All @@ -551,7 +551,7 @@ func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, sec
cnf.nginxManager.CreateSecret(secretName, data, nginx.TLSSecretFileMode)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading NGINX when updating the special Secrets: %v", err)
}

Expand Down Expand Up @@ -597,7 +597,7 @@ func (cnf *Configurator) DeleteSecret(key string, ingExes []IngressEx, mergeable
}

if len(ingExes)+len(mergeableIngresses)+len(virtualServerExes) > 0 {
if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading NGINX when deleting Secret %v: %v", key, err)
}
}
Expand All @@ -617,7 +617,7 @@ func (cnf *Configurator) DeleteIngress(key string) error {
cnf.deleteIngressMetricsLabels(key)
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when removing ingress %v: %v", key, err)
}

Expand All @@ -634,7 +634,7 @@ func (cnf *Configurator) DeleteVirtualServer(key string) error {
cnf.deleteVirtualServerMetricsLabels(fmt.Sprintf(key))
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when removing VirtualServer %v: %v", key, err)
}

Expand All @@ -648,7 +648,7 @@ func (cnf *Configurator) DeleteTransportServer(key string) error {
return fmt.Errorf("Error when removing TransportServer %v: %v", key, err)
}

err = cnf.nginxManager.Reload()
err = cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate)
if err != nil {
return fmt.Errorf("Error when removing TransportServer %v: %v", key, err)
}
Expand Down Expand Up @@ -694,7 +694,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error {
return nil
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err)
}

Expand Down Expand Up @@ -727,7 +727,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M
return nil
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX when updating endpoints for %v: %v", mergeableIngresses, err)
}

Expand Down Expand Up @@ -759,7 +759,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V
return nil
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err)
}

Expand Down Expand Up @@ -806,7 +806,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes
return nil
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("Error reloading NGINX when updating endpoints: %v", err)
}

Expand Down Expand Up @@ -948,7 +948,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres
}

cnf.nginxManager.SetOpenTracing(mainCfg.OpenTracingLoadModule)
if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return allWarnings, fmt.Errorf("Error when updating config from ConfigMap: %v", err)
}

Expand Down Expand Up @@ -981,7 +981,7 @@ func (cnf *Configurator) UpdateGlobalConfiguration(globalConfiguration *conf_v1a
}
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return updatedTransportServerExes, deletedTransportServerExes, fmt.Errorf("Error when updating global configuration: %v", err)
}

Expand Down Expand Up @@ -1097,7 +1097,7 @@ func (cnf *Configurator) AddOrUpdateSpiffeCerts(svidResponse *workload.X509SVIDs
cnf.nginxManager.CreateSecret(spiffeCertFileName, createSpiffeCert(svid.Certificates), spiffeCertsFileMode)
cnf.nginxManager.CreateSecret(spiffeBundleFileName, createSpiffeCert(svid.TrustBundle), spiffeCertsFileMode)

err = cnf.nginxManager.Reload()
err = cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate)
if err != nil {
return fmt.Errorf("error when reloading NGINX when updating the SPIFFE Certs: %v", err)
}
Expand Down Expand Up @@ -1174,7 +1174,7 @@ func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Un
}
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading NGINX when updating %v: %v", resource.GetKind(), err)
}

Expand All @@ -1201,7 +1201,7 @@ func (cnf *Configurator) DeleteAppProtectPolicy(polNamespaceame string, ingExes
}
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading NGINX when removing App Protect Policy: %v", err)
}

Expand All @@ -1228,7 +1228,7 @@ func (cnf *Configurator) DeleteAppProtectLogConf(logConfNamespaceame string, ing
}
}

if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading NGINX when removing App Protect Log Configuration: %v", err)
}

Expand All @@ -1246,7 +1246,7 @@ func (cnf *Configurator) AddInternalRouteConfig() error {
return fmt.Errorf("Error when writing main Config: %v", err)
}
cnf.nginxManager.CreateMainConfig(mainCfgContent)
if err := cnf.nginxManager.Reload(); err != nil {
if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("Error when reloading nginx: %v", err)
}
return nil
Expand Down
21 changes: 15 additions & 6 deletions internal/metrics/collectors/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

// ManagerCollector is an interface for the metrics of the Nginx Manager
type ManagerCollector interface {
IncNginxReloadCount()
IncNginxReloadCount(isEndPointUpdate bool)
IncNginxReloadErrors()
UpdateLastReloadTime(ms time.Duration)
Register(registry *prometheus.Registry) error
Expand All @@ -17,7 +17,7 @@ type ManagerCollector interface {
// LocalManagerMetricsCollector implements NginxManagerCollector interface and prometheus.Collector interface
type LocalManagerMetricsCollector struct {
// Metrics
reloadsTotal prometheus.Counter
reloadsTotal *prometheus.CounterVec
reloadsError prometheus.Counter
lastReloadStatus prometheus.Gauge
lastReloadTime prometheus.Gauge
Expand All @@ -26,13 +26,14 @@ type LocalManagerMetricsCollector struct {
// NewLocalManagerMetricsCollector creates a new LocalManagerMetricsCollector
func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManagerMetricsCollector {
nc := &LocalManagerMetricsCollector{
reloadsTotal: prometheus.NewCounter(
reloadsTotal: prometheus.NewCounterVec(
prometheus.CounterOpts{
Name: "nginx_reloads_total",
Namespace: metricsNamespace,
Help: "Number of successful NGINX reloads",
ConstLabels: constLabels,
},
[]string{"reason"},
),
reloadsError: prometheus.NewCounter(
prometheus.CounterOpts{
Expand All @@ -59,12 +60,20 @@ func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManage
},
),
}
nc.reloadsTotal.WithLabelValues("other")
nc.reloadsTotal.WithLabelValues("endpoints")
return nc
}

// IncNginxReloadCount increments the counter of successful NGINX reloads and sets the last reload status to true
func (nc *LocalManagerMetricsCollector) IncNginxReloadCount() {
nc.reloadsTotal.Inc()
func (nc *LocalManagerMetricsCollector) IncNginxReloadCount(isEndPointUpdate bool) {
var label string
if isEndPointUpdate {
label = "endpoints"
} else {
label = "other"
}
nc.reloadsTotal.WithLabelValues(label).Inc()
nc.updateLastReloadStatus(true)
}

Expand Down Expand Up @@ -121,7 +130,7 @@ func NewManagerFakeCollector() *ManagerFakeCollector {
func (nc *ManagerFakeCollector) Register(registry *prometheus.Registry) error { return nil }

// IncNginxReloadCount implements a fake IncNginxReloadCount
func (nc *ManagerFakeCollector) IncNginxReloadCount() {}
func (nc *ManagerFakeCollector) IncNginxReloadCount(isEndPointUpdate bool) {}

// IncNginxReloadErrors implements a fake IncNginxReloadErrors
func (nc *ManagerFakeCollector) IncNginxReloadErrors() {}
Expand Down
2 changes: 1 addition & 1 deletion internal/nginx/fake_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (*FakeManager) Start(done chan error) {
}

// Reload provides a fake implementation of Reload.
func (*FakeManager) Reload() error {
func (*FakeManager) Reload(isEndpointsUpdate bool) error {
glog.V(3).Infof("Reloading nginx")
return nil
}
Expand Down
12 changes: 9 additions & 3 deletions internal/nginx/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ import (
"github.com/nginxinc/nginx-plus-go-client/client"
)

// ReloadForEndpointsUpdate means that is caused by an endpoints update.
const ReloadForEndpointsUpdate = true

// ReloadForOtherUpdate means that a reload is caused by an update for a resource(s) other than endpoints.
const ReloadForOtherUpdate = false

// TLSSecretFileMode defines the default filemode for files with TLS Secrets.
const TLSSecretFileMode = 0600

Expand Down Expand Up @@ -61,7 +67,7 @@ type Manager interface {
CreateDHParam(content string) (string, error)
CreateOpenTracingTracerConfig(content string) error
Start(done chan error)
Reload() error
Reload(isEndpointsUpdate bool) error
Quit()
UpdateConfigVersionFile(openTracing bool)
SetPlusClients(plusClient *client.NginxClient, plusConfigVersionCheckClient *http.Client)
Expand Down Expand Up @@ -268,7 +274,7 @@ func (lm *LocalManager) Start(done chan error) {
}

// Reload reloads NGINX.
func (lm *LocalManager) Reload() error {
func (lm *LocalManager) Reload(isEndpointsUpdate bool) error {
// write a new config version
lm.configVersion++
lm.UpdateConfigVersionFile(lm.OpenTracing)
Expand All @@ -287,7 +293,7 @@ func (lm *LocalManager) Reload() error {
return fmt.Errorf("could not get newest config version: %v", err)
}

lm.metricsCollector.IncNginxReloadCount()
lm.metricsCollector.IncNginxReloadCount(isEndpointsUpdate)

t2 := time.Now()
lm.metricsCollector.UpdateLastReloadTime(t2.Sub(t1))
Expand Down

0 comments on commit 7139569

Please sign in to comment.