Skip to content

Commit

Permalink
Update nginx-prometheus-exporter to 1.0.0 (#1347)
Browse files Browse the repository at this point in the history
Problem: The new version of the NGINX Prometheus Exporter
introduces a couple of breaking changes. One of these changes
is that the exporter now requires a logger to be passed to the 
collectors. This logger interface is incompatible with NGF's logger.

Solution: Update to version 1.0.0 of the NGINX Prometheus Exporter
and fix errors. Wraps the Prometheus exporter logger in an interface
so that we can control its log level alongside our base logger.
  • Loading branch information
dependabot[bot] authored Jan 8, 2024
1 parent 51e6109 commit bf87eec
Show file tree
Hide file tree
Showing 11 changed files with 501 additions and 79 deletions.
8 changes: 5 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ go 1.21.3
replace github.com/chzyer/logex v1.1.10 => github.com/chzyer/logex v1.2.0

require (
github.com/go-kit/log v0.2.1
github.com/go-logr/logr v1.4.1
github.com/google/go-cmp v0.6.0
github.com/maxbrunsfeld/counterfeiter/v6 v6.7.0
github.com/nginxinc/nginx-plus-go-client v0.10.0
github.com/nginxinc/nginx-prometheus-exporter v0.11.0
github.com/nginxinc/nginx-plus-go-client v1.2.0
github.com/nginxinc/nginx-prometheus-exporter v1.0.0
github.com/onsi/ginkgo/v2 v2.13.2
github.com/onsi/gomega v1.30.0
github.com/prometheus/client_golang v1.18.0
github.com/prometheus/common v0.45.0
github.com/spf13/cobra v1.8.0
github.com/tsenart/vegeta/v12 v12.11.1
go.uber.org/zap v1.26.0
Expand All @@ -36,6 +38,7 @@ require (
github.com/evanphx/json-patch/v5 v5.7.0 // indirect
github.com/fatih/color v1.15.0 // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/go-logfmt/logfmt v0.5.1 // indirect
github.com/go-logr/zapr v1.2.4 // indirect
github.com/go-openapi/jsonpointer v0.20.0 // indirect
github.com/go-openapi/jsonreference v0.20.2 // indirect
Expand Down Expand Up @@ -67,7 +70,6 @@ require (
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.5.0 // indirect
github.com/prometheus/common v0.45.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect
github.com/rs/dnscache v0.0.0-20211102005908-e0241e321417 // indirect
github.com/spf13/pflag v1.0.5 // indirect
Expand Down
12 changes: 8 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs=
github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/go-kit/log v0.2.1 h1:MRVx0/zhvdseW+Gza6N9rVzU/IVzaeE1SFI4raAhmBU=
github.com/go-kit/log v0.2.1/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0=
github.com/go-logfmt/logfmt v0.5.1 h1:otpy5pqBCBZ1ng9RQ0dPu4PN7ba75Y/aA+UpowDyNVA=
github.com/go-logfmt/logfmt v0.5.1/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs=
github.com/go-logr/logr v1.2.4/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.3.0/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ=
Expand Down Expand Up @@ -113,10 +117,10 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus=
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
github.com/nginxinc/nginx-plus-go-client v0.10.0 h1:3zsMMkPvRDo8D7ZSprXtbAEW/SDmezZWzxdyS+6oAlc=
github.com/nginxinc/nginx-plus-go-client v0.10.0/go.mod h1:0v3RsQCvRn/IyrMtW+DK6CNkz+PxEsXDJPjQ3yUMBF0=
github.com/nginxinc/nginx-prometheus-exporter v0.11.0 h1:21xjnqNgxtni2jDgAQ90bl15uDnrTreO9sIlu1YsX/U=
github.com/nginxinc/nginx-prometheus-exporter v0.11.0/go.mod h1:GdyHnWAb8q8OW1Pssrrqbcqra0SH0Vn6UXICMmyWkw8=
github.com/nginxinc/nginx-plus-go-client v1.2.0 h1:NVfRsHbMJ7lOhkqMG52uvODiDBhQZNp20c0tV2lU3wg=
github.com/nginxinc/nginx-plus-go-client v1.2.0/go.mod h1:n8OFLzrJulJ2fur28Cwa1Qp5DZNS2VicLV+Adt30LQ4=
github.com/nginxinc/nginx-prometheus-exporter v1.0.0 h1:rw5q6j6FQe9EWzJy5HzRgRBJ2tSVyC9By6k9ZFQ7lD8=
github.com/nginxinc/nginx-prometheus-exporter v1.0.0/go.mod h1:SPohlKx0SiOuZYi04js53GWWb0HhD281AT8q4ApVMIE=
github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
Expand Down
78 changes: 33 additions & 45 deletions internal/mode/static/config_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"fmt"

"github.com/go-logr/logr"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -17,54 +15,14 @@ import (
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
)

// ZapLogLevelSetter defines an interface for setting the logging level of a zap logger.
type ZapLogLevelSetter interface {
SetLevel(string) error
Enabled(zapcore.Level) bool
}

type zapSetterImpl struct {
atomicLevel zap.AtomicLevel
}

func newZapLogLevelSetter(atomicLevel zap.AtomicLevel) zapSetterImpl {
return zapSetterImpl{
atomicLevel: atomicLevel,
}
}

// SetLevel sets the logging level for the zap logger.
func (z zapSetterImpl) SetLevel(level string) error {
parsedLevel, err := zapcore.ParseLevel(level)
if err != nil {
fieldErr := field.NotSupported(
field.NewPath("logging.level"),
level,
[]string{
string(ngfAPI.ControllerLogLevelInfo),
string(ngfAPI.ControllerLogLevelDebug),
string(ngfAPI.ControllerLogLevelError),
})
return fieldErr
}
z.atomicLevel.SetLevel(parsedLevel)

return nil
}

// Enabled returns true if the given level is at or above the current level.
func (z zapSetterImpl) Enabled(level zapcore.Level) bool {
return z.atomicLevel.Enabled(level)
}

// updateControlPlane updates the control plane configuration with the given user spec.
// If any fields are not set within the user spec, the default configuration values are used.
func updateControlPlane(
cfg *ngfAPI.NginxGateway,
logger logr.Logger,
eventRecorder record.EventRecorder,
configNSName types.NamespacedName,
logLevelSetter ZapLogLevelSetter,
logLevelSetter logLevelSetter,
) error {
// build up default configuration
controlConfig := ngfAPI.NginxGatewaySpec{
Expand Down Expand Up @@ -100,6 +58,36 @@ func updateControlPlane(
)
}

// set the log level
return logLevelSetter.SetLevel(string(*controlConfig.Logging.Level))
level := *controlConfig.Logging.Level

if err := validateLogLevel(level); err != nil {
return err
}

if err := logLevelSetter.SetLevel(string(level)); err != nil {
return field.Invalid(
field.NewPath("logging.level"),
level,
err.Error(),
)
}

return nil
}

func validateLogLevel(level ngfAPI.ControllerLogLevel) error {
switch level {
case ngfAPI.ControllerLogLevelInfo, ngfAPI.ControllerLogLevelDebug, ngfAPI.ControllerLogLevelError:
default:
return field.NotSupported(
field.NewPath("logging.level"),
level,
[]string{
string(ngfAPI.ControllerLogLevelInfo),
string(ngfAPI.ControllerLogLevelDebug),
string(ngfAPI.ControllerLogLevelError),
})
}

return nil
}
133 changes: 133 additions & 0 deletions internal/mode/static/config_updater_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package static

import (
"errors"
"fmt"
"testing"

. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/log/zap"

ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1"
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/staticfakes"
)

func TestUpdateControlPlane(t *testing.T) {
debugLogCfg := &ngfAPI.NginxGateway{
Spec: ngfAPI.NginxGatewaySpec{
Logging: &ngfAPI.Logging{
Level: helpers.GetPointer(ngfAPI.ControllerLogLevelDebug),
},
},
}

invalidLevelConfig := &ngfAPI.NginxGateway{
Spec: ngfAPI.NginxGatewaySpec{
Logging: &ngfAPI.Logging{
Level: helpers.GetPointer[ngfAPI.ControllerLogLevel]("invalid"),
},
},
}

logger := zap.New()
fakeEventRecorder := record.NewFakeRecorder(1)
nsname := types.NamespacedName{Namespace: "test", Name: "test"}

tests := []struct {
setLevelErr error
nginxGateway *ngfAPI.NginxGateway
name string
expErrString string
expSetLevelCallCount int
expEvent bool
}{
{
name: "change log level",
nginxGateway: debugLogCfg,
expSetLevelCallCount: 1,
},
{
name: "invalid log level",
nginxGateway: invalidLevelConfig,
expErrString: `Unsupported value: "invalid"`,
expSetLevelCallCount: 0,
},
{
name: "nil NginxGateway",
nginxGateway: nil,
expEvent: true,
expSetLevelCallCount: 1,
},
{
name: "set log level fails",
nginxGateway: debugLogCfg,
setLevelErr: errors.New("set level failed"),
expErrString: "set level failed",
expSetLevelCallCount: 1,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)

fakeLogSetter := &staticfakes.FakeLogLevelSetter{
SetLevelStub: func(s string) error {
return test.setLevelErr
},
}

err := updateControlPlane(test.nginxGateway, logger, fakeEventRecorder, nsname, fakeLogSetter)

if test.expErrString != "" {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(ContainSubstring(test.expErrString))
} else {
g.Expect(err).ToNot(HaveOccurred())
}

if test.expEvent {
g.Expect(fakeEventRecorder.Events).To(HaveLen(1))
event := <-fakeEventRecorder.Events
g.Expect(event).To(ContainSubstring("ResourceDeleted"))
} else {
g.Expect(fakeEventRecorder.Events).To(BeEmpty())
}

g.Expect(fakeLogSetter.SetLevelCallCount()).To(Equal(test.expSetLevelCallCount))
})
}
}

func TestValidateLogLevel(t *testing.T) {
validLevels := []ngfAPI.ControllerLogLevel{
ngfAPI.ControllerLogLevelError,
ngfAPI.ControllerLogLevelInfo,
ngfAPI.ControllerLogLevelDebug,
}

invalidLevels := []ngfAPI.ControllerLogLevel{
ngfAPI.ControllerLogLevel("invalid"),
ngfAPI.ControllerLogLevel("high"),
ngfAPI.ControllerLogLevel("warn"),
}

for _, level := range validLevels {
t.Run(fmt.Sprintf("valid level %q", level), func(t *testing.T) {
g := NewWithT(t)

g.Expect(validateLogLevel(level)).To(Succeed())
})
}

for _, level := range invalidLevels {
t.Run(fmt.Sprintf("invalid level %q", level), func(t *testing.T) {
g := NewWithT(t)

g.Expect(validateLogLevel(level)).ToNot(Succeed())
})
}
}
2 changes: 1 addition & 1 deletion internal/mode/static/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type eventHandlerConfig struct {
// eventRecorder records events for Kubernetes resources.
eventRecorder record.EventRecorder
// logLevelSetter is used to update the logging level.
logLevelSetter ZapLogLevelSetter
logLevelSetter logLevelSetter
// metricsCollector collects metrics for this controller.
metricsCollector handlerMetricsCollector
// healthChecker sets the health of the Pod to Ready once we've written out our initial config
Expand Down
12 changes: 7 additions & 5 deletions internal/mode/static/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var _ = Describe("eventHandler", func() {
fakeEventRecorder *record.FakeRecorder
namespace = "nginx-gateway"
configName = "nginx-gateway-config"
zapLogLevelSetter zapLogLevelSetter
)

expectReconfig := func(expectedConf dataplane.Configuration, expectedFiles []file.File) {
Expand All @@ -68,12 +69,13 @@ var _ = Describe("eventHandler", func() {
fakeNginxRuntimeMgr = &runtimefakes.FakeManager{}
fakeStatusUpdater = &statusfakes.FakeUpdater{}
fakeEventRecorder = record.NewFakeRecorder(1)
zapLogLevelSetter = newZapLogLevelSetter(zap.NewAtomicLevel())

handler = newEventHandlerImpl(eventHandlerConfig{
k8sClient: fake.NewFakeClient(),
processor: fakeProcessor,
generator: fakeGenerator,
logLevelSetter: newZapLogLevelSetter(zap.NewAtomicLevel()),
logLevelSetter: zapLogLevelSetter,
nginxFileMgr: fakeNginxFileMgr,
nginxRuntimeMgr: fakeNginxRuntimeMgr,
statusUpdater: fakeStatusUpdater,
Expand Down Expand Up @@ -196,8 +198,8 @@ var _ = Describe("eventHandler", func() {
Expect(fakeStatusUpdater.UpdateCallCount()).Should(Equal(1))
_, statuses := fakeStatusUpdater.UpdateArgsForCall(0)
Expect(statuses).To(Equal(expStatuses(staticConds.NewNginxGatewayValid())))
Expect(handler.cfg.logLevelSetter.Enabled(zap.DebugLevel)).To(BeFalse())
Expect(handler.cfg.logLevelSetter.Enabled(zap.ErrorLevel)).To(BeTrue())
Expect(zapLogLevelSetter.Enabled(zap.DebugLevel)).To(BeFalse())
Expect(zapLogLevelSetter.Enabled(zap.ErrorLevel)).To(BeTrue())
})

It("handles an invalid config", func() {
Expand All @@ -216,7 +218,7 @@ var _ = Describe("eventHandler", func() {
"Warning UpdateFailed Failed to update control plane configuration: logging.level: Unsupported value: " +
"\"invalid\": supported values: \"info\", \"debug\", \"error\"",
))
Expect(handler.cfg.logLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue())
Expect(zapLogLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue())
})

It("handles a deleted config", func() {
Expand All @@ -225,7 +227,7 @@ var _ = Describe("eventHandler", func() {
Expect(len(fakeEventRecorder.Events)).To(Equal(1))
event := <-fakeEventRecorder.Events
Expect(event).To(Equal("Warning ResourceDeleted NginxGateway configuration was deleted; using defaults"))
Expect(handler.cfg.logLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue())
Expect(zapLogLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue())
})
})

Expand Down
Loading

0 comments on commit bf87eec

Please sign in to comment.