refactor: replace glog in healthcheck.go & remove glog dependency (#6628)

Author: AlexFenlon

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/go-chi/chi/v5 v5.1.0
 	github.com/go-kit/log v0.2.1
 	github.com/golang-jwt/jwt/v4 v4.5.0
-	github.com/golang/glog v1.2.2
 	github.com/google/go-cmp v0.6.0
 	github.com/jinzhu/copier v0.4.0
 	github.com/kr/pretty v0.3.1
@@ -151,8 +150,6 @@ require (
 	sigs.k8s.io/yaml v1.4.0 // indirect
 )
 
-replace github.com/golang/glog => github.com/nginxinc/glog v1.1.2
-
 replace google.golang.org/protobuf v1.26.0 => google.golang.org/protobuf v1.33.0
 
 replace google.golang.org/protobuf v1.26.0-rc.1 => google.golang.org/protobuf v1.33.0

go.sum

@@ -190,8 +190,6 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G
 github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
-github.com/nginxinc/glog v1.1.2 h1:zyoZXhCoYvNMJq4qMsKislUCGyJ4eU2gNl3Nt7cjESg=
-github.com/nginxinc/glog v1.1.2/go.mod h1:Q2FpGp/qFhJEVnuC88BVfbLDPmio9aHYUj4al6w0138=
 github.com/nginxinc/nginx-plus-go-client v1.3.0 h1:q/aeT4B5k0KLwWlefoBzfLfraBBvIKLuDg+lLFWAo4I=
 github.com/nginxinc/nginx-plus-go-client v1.3.0/go.mod h1:n8OFLzrJulJ2fur28Cwa1Qp5DZNS2VicLV+Adt30LQ4=
 github.com/nginxinc/nginx-prometheus-exporter v1.3.0 h1:1JtdxsZH0Uwhu1nL/j/QyOXytP5V5j68AEo2X+DFWb0=

internal/healthcheck/healthcheck.go

@@ -7,29 +7,32 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"strconv"
 	"strings"
 	"time"
 
+	nl "github.com/nginxinc/kubernetes-ingress/internal/logger"
+
 	v1 "k8s.io/api/core/v1"
 
 	"github.com/go-chi/chi/v5"
-	"github.com/golang/glog"
 	"github.com/nginxinc/kubernetes-ingress/internal/configs"
 	"github.com/nginxinc/nginx-plus-go-client/client"
 	"k8s.io/utils/strings/slices"
 )
 
 // RunHealthCheck starts the deep healthcheck service.
 func RunHealthCheck(port int, plusClient *client.NginxClient, cnf *configs.Configurator, healthProbeTLSSecret *v1.Secret) {
+	l := nl.LoggerFromContext(cnf.CfgParams.Context)
 	addr := fmt.Sprintf(":%s", strconv.Itoa(port))
 	hs, err := NewHealthServer(addr, plusClient, cnf, healthProbeTLSSecret)
 	if err != nil {
-		glog.Fatal(err)
+		nl.Fatal(l, err)
 	}
-	glog.Infof("Starting Service Insight listener on: %v%v", addr, "/probe")
-	glog.Fatal(hs.ListenAndServe())
+	nl.Infof(l, "Starting Service Insight listener on: %v%v", addr, "/probe")
+	nl.Fatal(l, hs.ListenAndServe())
 }
 
 // HealthServer holds data required for running
@@ -41,6 +44,7 @@ type HealthServer struct {
 	NginxUpstreams         func() (*client.Upstreams, error)
 	StreamUpstreamsForName func(host string) []string
 	NginxStreamUpstreams   func() (*client.StreamUpstreams, error)
+	Logger                 *slog.Logger
 }
 
 // NewHealthServer creates Health Server. If secret is provided,
@@ -57,6 +61,7 @@ func NewHealthServer(addr string, nc *client.NginxClient, cnf *configs.Configura
 		NginxUpstreams:         nc.GetUpstreams,
 		StreamUpstreamsForName: cnf.StreamUpstreamsForName,
 		NginxStreamUpstreams:   nc.GetStreamUpstreams,
+		Logger:                 nl.LoggerFromContext(cnf.CfgParams.Context),
 	}
 
 	if secret != nil {
@@ -97,22 +102,22 @@ func (hs *HealthServer) UpstreamStats(w http.ResponseWriter, r *http.Request) {
 
 	upstreamNames := hs.UpstreamsForHost(host)
 	if len(upstreamNames) == 0 {
-		glog.Errorf("no upstreams for requested hostname %s or hostname does not exist", host)
+		nl.Errorf(hs.Logger, "no upstreams for requested hostname %s or hostname does not exist", host)
 		w.WriteHeader(http.StatusNotFound)
 		return
 	}
 
 	upstreams, err := hs.NginxUpstreams()
 	if err != nil {
-		glog.Errorf("error retrieving upstreams for requested hostname: %s", host)
+		nl.Errorf(hs.Logger, "error retrieving upstreams for requested hostname: %s", host)
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
 
 	stats := countStats(upstreams, upstreamNames)
 	data, err := json.Marshal(stats)
 	if err != nil {
-		glog.Error("error marshaling result", err)
+		nl.Error(hs.Logger, "error marshaling result", err)
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
@@ -124,7 +129,7 @@ func (hs *HealthServer) UpstreamStats(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	}
 	if _, err = w.Write(data); err != nil {
-		glog.Error("error writing result", err)
+		nl.Error(hs.Logger, "error writing result", err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
 	}
 }
@@ -136,20 +141,20 @@ func (hs *HealthServer) StreamStats(w http.ResponseWriter, r *http.Request) {
 	n := sanitize(name)
 	streamUpstreamNames := hs.StreamUpstreamsForName(n)
 	if len(streamUpstreamNames) == 0 {
-		glog.Errorf("no stream upstreams for requested name '%s' or name does not exist", n)
+		nl.Errorf(hs.Logger, "no stream upstreams for requested name '%s' or name does not exist", n)
 		w.WriteHeader(http.StatusNotFound)
 		return
 	}
 	streams, err := hs.NginxStreamUpstreams()
 	if err != nil {
-		glog.Errorf("error retrieving stream upstreams for requested name: %s", n)
+		nl.Errorf(hs.Logger, "error retrieving stream upstreams for requested name: %s", n)
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
 	stats := countStreamStats(streams, streamUpstreamNames)
 	data, err := json.Marshal(stats)
 	if err != nil {
-		glog.Error("error marshaling result", err)
+		nl.Error(hs.Logger, "error marshaling result", err)
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
@@ -161,7 +166,7 @@ func (hs *HealthServer) StreamStats(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 	}
 	if _, err := w.Write(data); err != nil {
-		glog.Error("error writing result", err)
+		nl.Error(hs.Logger, "error writing result", err)
 		http.Error(w, "internal error", http.StatusInternalServerError)
 	}
 }

internal/healthcheck/healthcheck_test.go

@@ -3,10 +3,15 @@ package healthcheck_test
 import (
 	"encoding/json"
 	"errors"
+	"io"
+	"log/slog"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
+	nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
+	"github.com/nginxinc/kubernetes-ingress/internal/logger/levels"
+
 	"github.com/go-chi/chi/v5"
 	"github.com/google/go-cmp/cmp"
 	"github.com/nginxinc/kubernetes-ingress/internal/healthcheck"
@@ -25,6 +30,7 @@ func TestHealthCheckServer_Returns404OnMissingHostname(t *testing.T) {
 	hs := healthcheck.HealthServer{
 		UpstreamsForHost: getUpstreamsForHost,
 		NginxUpstreams:   getUpstreamsFromNGINXAllUp,
+		Logger:           slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -45,6 +51,7 @@ func TestHealthCheckServer_ReturnsCorrectStatsForHostnameOnAllPeersUp(t *testing
 	hs := healthcheck.HealthServer{
 		UpstreamsForHost: getUpstreamsForHost,
 		NginxUpstreams:   getUpstreamsFromNGINXAllUp,
+		Logger:           slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -78,6 +85,7 @@ func TestHealthCheckServer_ReturnsCorrectStatsForHostnameOnAllPeersDown(t *testi
 	hs := healthcheck.HealthServer{
 		UpstreamsForHost: getUpstreamsForHost,
 		NginxUpstreams:   getUpstreamsFromNGINXAllUnhealthy,
+		Logger:           slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -112,6 +120,7 @@ func TestHealthCheckServer_ReturnsCorrectStatsForValidHostnameOnPartOfPeersDown(
 	hs := healthcheck.HealthServer{
 		UpstreamsForHost: getUpstreamsForHost,
 		NginxUpstreams:   getUpstreamsFromNGINXPartiallyUp,
+		Logger:           slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -146,6 +155,7 @@ func TestHealthCheckServer_RespondsWith404OnNotExistingHostname(t *testing.T) {
 	hs := healthcheck.HealthServer{
 		UpstreamsForHost: getUpstreamsForHost,
 		NginxUpstreams:   getUpstreamsFromNGINXNotExistingHost,
+		Logger:           slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -166,6 +176,7 @@ func TestHealthCheckServer_RespondsWith500OnErrorFromNGINXAPI(t *testing.T) {
 	hs := healthcheck.HealthServer{
 		UpstreamsForHost: getUpstreamsForHost,
 		NginxUpstreams:   getUpstreamsFromNGINXErrorFromAPI,
+		Logger:           slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -186,6 +197,7 @@ func TestHealthCheckServer_Returns404OnMissingTransportServerActionName(t *testi
 	hs := healthcheck.HealthServer{
 		StreamUpstreamsForName: streamUpstreamsForName,
 		NginxStreamUpstreams:   streamUpstreamsFromNGINXAllUp,
+		Logger:                 slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -206,6 +218,7 @@ func TestHealthCheckServer_Returns404OnBogusTransportServerActionName(t *testing
 	hs := healthcheck.HealthServer{
 		StreamUpstreamsForName: streamUpstreamsForName,
 		NginxStreamUpstreams:   streamUpstreamsFromNGINXAllUp,
+		Logger:                 slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -226,6 +239,7 @@ func TestHealthCheckServer_ReturnsCorrectTransportServerStatsForNameOnAllPeersUp
 	hs := healthcheck.HealthServer{
 		StreamUpstreamsForName: streamUpstreamsForName,
 		NginxStreamUpstreams:   streamUpstreamsFromNGINXAllUp,
+		Logger:                 slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -259,6 +273,7 @@ func TestHealthCheckServer_ReturnsCorrectTransportServerStatsForNameOnSomePeersU
 	hs := healthcheck.HealthServer{
 		StreamUpstreamsForName: streamUpstreamsForName,
 		NginxStreamUpstreams:   streamUpstreamsFromNGINXPartiallyUp,
+		Logger:                 slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))
@@ -292,6 +307,7 @@ func TestHealthCheckServer_ReturnsCorrectTransportServerStatsForNameOnAllPeersDo
 	hs := healthcheck.HealthServer{
 		StreamUpstreamsForName: streamUpstreamsForName,
 		NginxStreamUpstreams:   streamUpstreamsFromNGINXAllPeersDown,
+		Logger:                 slog.New(nic_glog.New(io.Discard, &nic_glog.Options{Level: levels.LevelInfo})),
 	}
 
 	ts := httptest.NewServer(testHandler(&hs))