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 reason label to total_reload metric #1099

Merged
merged 5 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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 `endpoint` ( the reason for the reload was an endpoint was updated ) and `other` ( the reload was caused by something other than a reload ).
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
* `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(true); err != nil {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(true); err != nil {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(true); err != nil {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(false); 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(true); err != nil {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(false); 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(false); 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(false); 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(true); err != nil {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(true); err != nil {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(true)
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
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(true); 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(true); 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(true); 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(true); 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(false); 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(false); 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(false)
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(false); 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(false); 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(false); 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(false); err != nil {
return fmt.Errorf("Error when reloading nginx: %v", err)
}
return nil
Expand Down
19 changes: 13 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(isEndPoint 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 Down Expand Up @@ -63,8 +64,14 @@ func NewLocalManagerMetricsCollector(constLabels map[string]string) *LocalManage
}

// 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(isEndPoint bool) {
var label string
if isEndPoint {
label = "endpoint"
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
} else {
label = "other"
}
nc.reloadsTotal.WithLabelValues(label).Inc()
nc.updateLastReloadStatus(true)
}

Expand Down Expand Up @@ -121,7 +128,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(isEnPoint 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(isEndpoint bool) error {
glog.V(3).Infof("Reloading nginx")
return nil
}
Expand Down
6 changes: 3 additions & 3 deletions internal/nginx/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type Manager interface {
CreateDHParam(content string) (string, error)
CreateOpenTracingTracerConfig(content string) error
Start(done chan error)
Reload() error
Reload(isEndpoint bool) error
Quit()
UpdateConfigVersionFile(openTracing bool)
SetPlusClients(plusClient *client.NginxClient, plusConfigVersionCheckClient *http.Client)
Expand Down Expand Up @@ -268,7 +268,7 @@ func (lm *LocalManager) Start(done chan error) {
}

// Reload reloads NGINX.
func (lm *LocalManager) Reload() error {
func (lm *LocalManager) Reload(isEndPoint bool) error {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
// write a new config version
lm.configVersion++
lm.UpdateConfigVersionFile(lm.OpenTracing)
Expand All @@ -287,7 +287,7 @@ func (lm *LocalManager) Reload() error {
return fmt.Errorf("could not get newest config version: %v", err)
}

lm.metricsCollector.IncNginxReloadCount()
lm.metricsCollector.IncNginxReloadCount(isEndPoint)

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