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.

Sign up for GitHub

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

Use zap logger with logrotation, file logging for ip-reconciler #210

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use zap logger with logrotation, file logging for ip-reconciler #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions cmd/reconciler/ip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ const defaultReconcilerTimeout = 30

func main() {
kubeConfigFile := flag.String("kubeconfig", "", "the path to the Kubernetes configuration file")
logLevel := flag.String("log-level", "error", "the logging level for the `ip-reconciler` app. Valid values are: \"debug\", \"verbose\", \"error\", and \"panic\".")
logFile := flag.String("log-file", "/host/var/log/ip-reconciler.log", "File on host for ip-reconciler to log to")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you are removing the log level parameter. It still makes sense.

Also, you cannot use this default - i.e. what if the cronjob spec does not feature the volume mount ? It will fail.

You should use a default on the container file system, but override that in the daemonset spec shipped in whereabouts.

reconcilerTimeout := flag.Int("timeout", defaultReconcilerTimeout, "the value for a request timeout in seconds.")
flag.Parse()

logging.SetLogLevel(*logLevel)
logging.ConfigureLogger(*logFile)

var err error
var ipReconcileLoop *reconciler.ReconcileLooper
Expand Down
8 changes: 6 additions & 2 deletions doc/crds/ip-reconciler-job.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ metadata:
app: whereabouts
spec:
concurrencyPolicy: Forbid
successfulJobsHistoryLimit: 0
successfulJobsHistoryLimit: 3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're now logging to the host's file-system, why would we want to preserve these around ?

schedule: "*/5 * * * *"
jobTemplate:
spec:
Expand All @@ -30,12 +30,16 @@ spec:
memory: "50Mi"
command:
- /ip-reconciler
- -log-level=verbose
volumeMounts:
- name: cni-net-dir
mountPath: /host/etc/cni/net.d
- name: reconciler-log
mountPath: /host/var/log
volumes:
- name: cni-net-dir
hostPath:
path: /etc/cni/net.d
- name: reconciler-log
hostPath:
path: /var/log
restartPolicy: OnFailure
2 changes: 2 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ require (
github.com/onsi/ginkgo v1.14.1
github.com/onsi/gomega v1.10.2
github.com/pkg/errors v0.9.1
go.uber.org/zap v1.15.0
golang.org/x/net v0.0.0-20220225172249-27dd8689420f // indirect
golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9 // indirect
golang.org/x/tools v0.1.9 // indirect
gomodules.xyz/jsonpatch/v2 v2.1.0
gopkg.in/natefinch/lumberjack.v2 v2.0.0
k8s.io/api v0.22.6
k8s.io/apimachinery v0.22.6
k8s.io/client-go v0.22.6
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
gopkg.in/mgo.v2 v2.0.0-20180705113604-9856a29383ce/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA=
gopkg.in/natefinch/lumberjack.v2 v2.0.0 h1:1Lc07Kr7qY4U2YPouBjpCLxpiyxIVoxqXgkXLknAOE8=
gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k=
gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI=
Expand Down
5 changes: 1 addition & 4 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,7 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*types.IPAMConfig, string, er

// Logging
if n.IPAM.LogFile != "" {
logging.SetLogFile(n.IPAM.LogFile)
}
if n.IPAM.LogLevel != "" {
logging.SetLogLevel(n.IPAM.LogLevel)
logging.ConfigureLogger(n.IPAM.LogFile)
}

if foundflatfile != "" {
Expand Down
145 changes: 30 additions & 115 deletions pkg/logging/logging.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,140 +16,55 @@ package logging

import (
"fmt"
"os"
"strings"
"time"

"github.com/pkg/errors"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"gopkg.in/natefinch/lumberjack.v2"
)

// Level type
type Level uint32

// PanicLevel ErrorLevel etc are our logging level constants.
const (
PanicLevel Level = iota
ErrorLevel
VerboseLevel
DebugLevel
MaxLevel
UnknownLevel
)

var loggingStderr bool
var loggingFp *os.File
var loggingLevel Level

const defaultTimestampFormat = time.RFC3339

func (l Level) String() string {
switch l {
case PanicLevel:
return "panic"
case VerboseLevel:
return "verbose"
case ErrorLevel:
return "error"
case DebugLevel:
return "debug"
}
return "unknown"
}

// Printf provides basic Printf functionality for logs
func Printf(level Level, format string, a ...interface{}) {
header := "%s [%s] "
t := time.Now()
if level > loggingLevel {
return
}

if loggingStderr {
fmt.Fprintf(os.Stderr, header, t.Format(defaultTimestampFormat), level)
fmt.Fprintf(os.Stderr, format, a...)
fmt.Fprintf(os.Stderr, "\n")
}

if loggingFp != nil {
fmt.Fprintf(loggingFp, header, t.Format(defaultTimestampFormat), level)
fmt.Fprintf(loggingFp, format, a...)
fmt.Fprintf(loggingFp, "\n")
}
}

// Debugf defines our printf for debug level.
func Debugf(format string, a ...interface{}) {
Printf(DebugLevel, format, a...)
zap.S().Debugf(format, a...)
}

// Verbosef defines our printf for Verbose level.
func Verbosef(format string, a ...interface{}) {
Printf(VerboseLevel, format, a...)
zap.S().Warnf(format, a...)
}

// Errorf defines our printf for error level.
func Errorf(format string, a ...interface{}) error {
Printf(ErrorLevel, format, a...)
zap.S().Errorf(format, a...)
return fmt.Errorf(format, a...)
}

// Panicf defines our printf for panic level.
func Panicf(format string, a ...interface{}) {
Printf(PanicLevel, format, a...)
Printf(PanicLevel, "========= Stack trace output ========")
Printf(PanicLevel, "%+v", errors.New("Whereabouts Panic"))
Printf(PanicLevel, "========= Stack trace output end ========")
}

// GetLoggingLevel returns loggingLevel
func GetLoggingLevel() Level {
return loggingLevel
}

func getLoggingLevel(levelStr string) Level {
switch strings.ToLower(levelStr) {
case "debug":
return DebugLevel
case "verbose":
return VerboseLevel
case "error":
return ErrorLevel
case "panic":
return PanicLevel
}
fmt.Fprintf(os.Stderr, "Whereabouts logging: cannot set logging level to %s\n", levelStr)
return UnknownLevel
}

// SetLogLevel sets loggingLevel
func SetLogLevel(levelStr string) {
level := getLoggingLevel(levelStr)
if level < MaxLevel {
loggingLevel = level
}
}

// SetLogStderr enables logging to stderr
func SetLogStderr(enable bool) {
loggingStderr = enable
}

// SetLogFile defines which log file we'll log to
func SetLogFile(filename string) {
if filename == "" {
return
}

fp, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0644)
if err != nil {
loggingFp = nil
fmt.Fprintf(os.Stderr, "Whereabouts logging: cannot open %s", filename)
}
loggingFp = fp
zap.S().Panicf(format, a...)
zap.S().Panicf("========= Stack trace output ========")
zap.S().Panicf("%+v", errors.New("Whereabouts Panic"))
zap.S().Panicf("========= Stack trace output end ========")
}

func init() {
loggingStderr = true
loggingFp = nil
loggingLevel = DebugLevel
func ConfigureLogger(logFile string) {
w := zapcore.AddSync(&lumberjack.Logger{
Filename: logFile,
MaxSize: 100, // mb
MaxBackups: 5,
MaxAge: 30,
Compress: true,
})

core := zapcore.NewCore(
zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()),
w,
zap.DebugLevel,
)

logger := zap.New(core)
defer logger.Sync()
zap.ReplaceGlobals(logger)
zap.S().Debugf("Started zap logger")
return
}
4 changes: 4 additions & 0 deletions vendor/modules.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ go.uber.org/atomic
# go.uber.org/multierr v1.5.0
go.uber.org/multierr
# go.uber.org/zap v1.15.0
## explicit
go.uber.org/zap
go.uber.org/zap/buffer
go.uber.org/zap/internal/bufferpool
Expand Down Expand Up @@ -271,6 +272,9 @@ google.golang.org/protobuf/types/known/durationpb
google.golang.org/protobuf/types/known/timestamppb
# gopkg.in/inf.v0 v0.9.1
gopkg.in/inf.v0
# gopkg.in/natefinch/lumberjack.v2 v2.0.0
## explicit
gopkg.in/natefinch/lumberjack.v2
# gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
gopkg.in/tomb.v1
# gopkg.in/yaml.v2 v2.4.0
Expand Down