diff --git a/go.mod b/go.mod index e11a2bde47..dc831a72ed 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -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 @@ -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 diff --git a/go.sum b/go.sum index be1bfc593f..742d56d3b2 100644 --- a/go.sum +++ b/go.sum @@ -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= @@ -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= diff --git a/internal/mode/static/config_updater.go b/internal/mode/static/config_updater.go index c08c9b12ca..884a918b2f 100644 --- a/internal/mode/static/config_updater.go +++ b/internal/mode/static/config_updater.go @@ -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" @@ -17,46 +15,6 @@ 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( @@ -64,7 +22,7 @@ func updateControlPlane( logger logr.Logger, eventRecorder record.EventRecorder, configNSName types.NamespacedName, - logLevelSetter ZapLogLevelSetter, + logLevelSetter logLevelSetter, ) error { // build up default configuration controlConfig := ngfAPI.NginxGatewaySpec{ @@ -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 } diff --git a/internal/mode/static/config_updater_test.go b/internal/mode/static/config_updater_test.go new file mode 100644 index 0000000000..4380a4af8e --- /dev/null +++ b/internal/mode/static/config_updater_test.go @@ -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()) + }) + } +} diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index f20980316a..53e17a02ff 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -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 diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 80dc803f30..61ad16a86b 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -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) { @@ -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, @@ -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() { @@ -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() { @@ -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()) }) }) diff --git a/internal/mode/static/log_level_setters.go b/internal/mode/static/log_level_setters.go new file mode 100644 index 0000000000..8f44809b87 --- /dev/null +++ b/internal/mode/static/log_level_setters.go @@ -0,0 +1,103 @@ +package static + +import ( + "errors" + + "github.com/go-kit/log" + "github.com/prometheus/common/promlog" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . logLevelSetter + +// logLevelSetter defines an interface for setting the logging level of a logger. +type logLevelSetter interface { + SetLevel(string) error +} + +// multiLogLevelSetter sets the log level for multiple logLevelSetters. +type multiLogLevelSetter struct { + setters []logLevelSetter +} + +func newMultiLogLevelSetter(setters ...logLevelSetter) multiLogLevelSetter { + return multiLogLevelSetter{setters: setters} +} + +func (m multiLogLevelSetter) SetLevel(level string) error { + allErrs := make([]error, 0, len(m.setters)) + + for _, s := range m.setters { + if err := s.SetLevel(level); err != nil { + allErrs = append(allErrs, err) + } + } + + return errors.Join(allErrs...) +} + +// zapLogLevelSetter sets the level for a zap logger. +type zapLogLevelSetter struct { + atomicLevel zap.AtomicLevel +} + +func newZapLogLevelSetter(atomicLevel zap.AtomicLevel) zapLogLevelSetter { + return zapLogLevelSetter{ + atomicLevel: atomicLevel, + } +} + +// SetLevel sets the logging level for the zap logger. +func (z zapLogLevelSetter) SetLevel(level string) error { + parsedLevel, err := zapcore.ParseLevel(level) + if err != nil { + return err + } + z.atomicLevel.SetLevel(parsedLevel) + + return nil +} + +// Enabled returns true if the given level is at or above the current level. +func (z zapLogLevelSetter) Enabled(level zapcore.Level) bool { + return z.atomicLevel.Enabled(level) +} + +// leveledPrometheusLogger is a leveled prometheus logger. +// This interface is required because the promlog.NewDynamic returns an unexported type *logger. +type leveledPrometheusLogger interface { + log.Logger + SetLevel(level *promlog.AllowedLevel) +} + +type promLogLevelSetter struct { + logger leveledPrometheusLogger +} + +func newPromLogLevelSetter(logger leveledPrometheusLogger) promLogLevelSetter { + return promLogLevelSetter{logger: logger} +} + +func newLeveledPrometheusLogger() (leveledPrometheusLogger, error) { + logFormat := &promlog.AllowedFormat{} + + if err := logFormat.Set("json"); err != nil { + return nil, err + } + + logConfig := &promlog.Config{Format: logFormat} + logger := promlog.NewDynamic(logConfig) + + return logger, nil +} + +func (p promLogLevelSetter) SetLevel(level string) error { + al := &promlog.AllowedLevel{} + if err := al.Set(level); err != nil { + return err + } + + p.logger.SetLevel(al) + return nil +} diff --git a/internal/mode/static/log_level_setters_test.go b/internal/mode/static/log_level_setters_test.go new file mode 100644 index 0000000000..214b9a15bc --- /dev/null +++ b/internal/mode/static/log_level_setters_test.go @@ -0,0 +1,73 @@ +package static + +import ( + "errors" + "testing" + + . "github.com/onsi/gomega" + "go.uber.org/zap" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/staticfakes" +) + +func TestMultiLogLevelSetter_SetLevel(t *testing.T) { + g := NewWithT(t) + + logr1 := &staticfakes.FakeLogLevelSetter{} + logr2 := &staticfakes.FakeLogLevelSetter{} + logr3 := &staticfakes.FakeLogLevelSetter{} + + multiSetter := newMultiLogLevelSetter(logr1, logr2, logr3) + g.Expect(multiSetter.SetLevel("test")).To(Succeed()) + + g.Expect(logr1.SetLevelCallCount()).To(Equal(1)) + g.Expect(logr2.SetLevelCallCount()).To(Equal(1)) + g.Expect(logr3.SetLevelCallCount()).To(Equal(1)) + + // error case + err1 := errors.New("error1") + err2 := errors.New("error2") + err3 := errors.New("error3") + + logr1.SetLevelReturns(err1) + logr2.SetLevelReturns(err2) + logr3.SetLevelReturns(err3) + + err := multiSetter.SetLevel("test") + g.Expect(err).To(HaveOccurred()) + + expErr := errors.Join(err1, err2, err3) + g.Expect(err).To(MatchError(expErr)) +} + +func TestZapLogLevelSetter_SetLevel(t *testing.T) { + g := NewWithT(t) + + zapSetter := newZapLogLevelSetter(zap.NewAtomicLevel()) + + g.Expect(zapSetter.SetLevel("error")).To(Succeed()) + g.Expect(zapSetter.Enabled(zap.ErrorLevel)).To(BeTrue()) + + g.Expect(zapSetter.SetLevel("info")).To(Succeed()) + g.Expect(zapSetter.Enabled(zap.InfoLevel)).To(BeTrue()) + + g.Expect(zapSetter.SetLevel("debug")).To(Succeed()) + g.Expect(zapSetter.Enabled(zap.DebugLevel)).To(BeTrue()) + + g.Expect(zapSetter.SetLevel("invalid")).ToNot(Succeed()) +} + +func TestPromLogLevelSetter_SetLevel(t *testing.T) { + g := NewWithT(t) + + logger, err := newLeveledPrometheusLogger() + g.Expect(err).ToNot(HaveOccurred()) + + promSetter := newPromLogLevelSetter(logger) + + g.Expect(promSetter.SetLevel("error")).To(Succeed()) + g.Expect(promSetter.SetLevel("info")).To(Succeed()) + g.Expect(promSetter.SetLevel("debug")).To(Succeed()) + + g.Expect(promSetter.SetLevel("invalid")).ToNot(Succeed()) +} diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index fcb2379be0..3aa4850813 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -94,7 +94,13 @@ func StartManager(cfg config.Config) error { recorderName := fmt.Sprintf("nginx-gateway-fabric-%s", cfg.GatewayClassName) recorder := mgr.GetEventRecorderFor(recorderName) - logLevelSetter := newZapLogLevelSetter(cfg.AtomicLevel) + + promLogger, err := newLeveledPrometheusLogger() + if err != nil { + return fmt.Errorf("error creating leveled prometheus logger: %w", err) + } + + logLevelSetter := newMultiLogLevelSetter(newZapLogLevelSetter(cfg.AtomicLevel), newPromLogLevelSetter(promLogger)) ctx := ctlr.SetupSignalHandler() @@ -150,9 +156,9 @@ func StartManager(cfg config.Config) error { constLabels := map[string]string{"class": cfg.GatewayClassName} var ngxCollector prometheus.Collector if cfg.Plus { - ngxCollector, err = collectors.NewNginxPlusMetricsCollector(constLabels) + ngxCollector, err = collectors.NewNginxPlusMetricsCollector(constLabels, promLogger) } else { - ngxCollector, err = collectors.NewNginxMetricsCollector(constLabels) + ngxCollector = collectors.NewNginxMetricsCollector(constLabels, promLogger) } if err != nil { return fmt.Errorf("cannot create nginx metrics collector: %w", err) @@ -245,7 +251,7 @@ func registerControllers( cfg config.Config, mgr manager.Manager, recorder record.EventRecorder, - logLevelSetter zapSetterImpl, + logLevelSetter logLevelSetter, eventCh chan interface{}, controlConfigNSName types.NamespacedName, ) error { @@ -406,7 +412,7 @@ func setInitialConfig( reader client.Reader, logger logr.Logger, eventRecorder record.EventRecorder, - logLevelSetter ZapLogLevelSetter, + logLevelSetter logLevelSetter, configName types.NamespacedName, ) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) diff --git a/internal/mode/static/metrics/collectors/nginx.go b/internal/mode/static/metrics/collectors/nginx.go index 3b2418db5c..b68123db29 100644 --- a/internal/mode/static/metrics/collectors/nginx.go +++ b/internal/mode/static/metrics/collectors/nginx.go @@ -5,8 +5,8 @@ import ( "fmt" "net" "net/http" - "time" + "github.com/go-kit/log" "github.com/nginxinc/nginx-plus-go-client/client" prometheusClient "github.com/nginxinc/nginx-prometheus-exporter/client" nginxCollector "github.com/nginxinc/nginx-prometheus-exporter/collector" @@ -16,32 +16,36 @@ import ( ) const ( - nginxStatusSock = "/var/run/nginx/nginx-status.sock" - nginxStatusURI = "http://config-status/stub_status" - nginxPlusAPISock = "/var/run/nginx/nginx-plus-api.sock" - nginxPlusAPIURI = "http://nginx-plus-api/api" - nginxStatusSockTimeout = 10 * time.Second + nginxStatusSock = "/var/run/nginx/nginx-status.sock" + nginxStatusURI = "http://config-status/stub_status" + nginxPlusAPISock = "/var/run/nginx/nginx-plus-api.sock" + nginxPlusAPIURI = "http://nginx-plus-api/api" ) // NewNginxMetricsCollector creates an NginxCollector which fetches stats from NGINX over a unix socket -func NewNginxMetricsCollector(constLabels map[string]string) (prometheus.Collector, error) { +func NewNginxMetricsCollector(constLabels map[string]string, logger log.Logger) prometheus.Collector { httpClient := getSocketClient(nginxStatusSock) + ngxClient := prometheusClient.NewNginxClient(&httpClient, nginxStatusURI) - client, err := prometheusClient.NewNginxClient(&httpClient, nginxStatusURI) - if err != nil { - return nil, err - } - return nginxCollector.NewNginxCollector(client, metrics.Namespace, constLabels), nil + return nginxCollector.NewNginxCollector(ngxClient, metrics.Namespace, constLabels, logger) } // NewNginxPlusMetricsCollector creates an NginxCollector which fetches stats from NGINX Plus API over a unix socket -func NewNginxPlusMetricsCollector(constLabels map[string]string) (prometheus.Collector, error) { +func NewNginxPlusMetricsCollector(constLabels map[string]string, logger log.Logger) (prometheus.Collector, error) { plusClient, err := createPlusClient() if err != nil { return nil, err } - variableLabelNames := nginxCollector.VariableLabelNames{} - return nginxCollector.NewNginxPlusCollector(plusClient, metrics.Namespace, variableLabelNames, constLabels), nil + + collector := nginxCollector.NewNginxPlusCollector( + plusClient, + metrics.Namespace, + nginxCollector.VariableLabelNames{}, + constLabels, + logger, + ) + + return collector, nil } // getSocketClient gets an http.Client with a unix socket transport. @@ -60,7 +64,7 @@ func createPlusClient() (*client.NginxClient, error) { var err error httpClient := getSocketClient(nginxPlusAPISock) - plusClient, err = client.NewNginxClient(&httpClient, nginxPlusAPIURI) + plusClient, err = client.NewNginxClient(nginxPlusAPIURI, client.WithHTTPClient(&httpClient)) if err != nil { return nil, fmt.Errorf("failed to create NginxClient for Plus: %w", err) } diff --git a/internal/mode/static/staticfakes/fake_log_level_setter.go b/internal/mode/static/staticfakes/fake_log_level_setter.go new file mode 100644 index 0000000000..154257ed5d --- /dev/null +++ b/internal/mode/static/staticfakes/fake_log_level_setter.go @@ -0,0 +1,107 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package staticfakes + +import ( + "sync" +) + +type FakeLogLevelSetter struct { + SetLevelStub func(string) error + setLevelMutex sync.RWMutex + setLevelArgsForCall []struct { + arg1 string + } + setLevelReturns struct { + result1 error + } + setLevelReturnsOnCall map[int]struct { + result1 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *FakeLogLevelSetter) SetLevel(arg1 string) error { + fake.setLevelMutex.Lock() + ret, specificReturn := fake.setLevelReturnsOnCall[len(fake.setLevelArgsForCall)] + fake.setLevelArgsForCall = append(fake.setLevelArgsForCall, struct { + arg1 string + }{arg1}) + stub := fake.SetLevelStub + fakeReturns := fake.setLevelReturns + fake.recordInvocation("SetLevel", []interface{}{arg1}) + fake.setLevelMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeLogLevelSetter) SetLevelCallCount() int { + fake.setLevelMutex.RLock() + defer fake.setLevelMutex.RUnlock() + return len(fake.setLevelArgsForCall) +} + +func (fake *FakeLogLevelSetter) SetLevelCalls(stub func(string) error) { + fake.setLevelMutex.Lock() + defer fake.setLevelMutex.Unlock() + fake.SetLevelStub = stub +} + +func (fake *FakeLogLevelSetter) SetLevelArgsForCall(i int) string { + fake.setLevelMutex.RLock() + defer fake.setLevelMutex.RUnlock() + argsForCall := fake.setLevelArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeLogLevelSetter) SetLevelReturns(result1 error) { + fake.setLevelMutex.Lock() + defer fake.setLevelMutex.Unlock() + fake.SetLevelStub = nil + fake.setLevelReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeLogLevelSetter) SetLevelReturnsOnCall(i int, result1 error) { + fake.setLevelMutex.Lock() + defer fake.setLevelMutex.Unlock() + fake.SetLevelStub = nil + if fake.setLevelReturnsOnCall == nil { + fake.setLevelReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.setLevelReturnsOnCall[i] = struct { + result1 error + }{result1} +} + +func (fake *FakeLogLevelSetter) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.setLevelMutex.RLock() + defer fake.setLevelMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *FakeLogLevelSetter) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +}