Skip to content

Commit 415d434

Browse files
wallrjmaelvls
andauthored
[VC-35738] Log with klog to stdout and stderr in Kubernetes text format (#596)
* Log with klog to stdout and stderr in Kubernetes text format Signed-off-by: Richard Wall <[email protected]> Co-authored-by: Maël Valais <[email protected]>
1 parent efebe5b commit 415d434

File tree

6 files changed

+620
-12
lines changed

6 files changed

+620
-12
lines changed

cmd/root.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"strings"
77

8+
"github.com/jetstack/preflight/pkg/logs"
89
"github.com/spf13/cobra"
910
"github.com/spf13/pflag"
1011
)
@@ -17,6 +18,9 @@ var rootCmd = &cobra.Command{
1718
configuration checks using Open Policy Agent (OPA).
1819
1920
Preflight checks are bundled into Packages`,
21+
PersistentPreRun: func(cmd *cobra.Command, args []string) {
22+
logs.Initialize()
23+
},
2024
}
2125

2226
func init() {
@@ -28,6 +32,8 @@ func init() {
2832
// Execute adds all child commands to the root command and sets flags appropriately.
2933
// This is called by main.main(). It only needs to happen once to the rootCmd.
3034
func Execute() {
35+
logs.AddFlags(rootCmd.PersistentFlags())
36+
3137
if err := rootCmd.Execute(); err != nil {
3238
fmt.Println(err)
3339
os.Exit(1)

go.mod

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/jetstack/preflight
33
go 1.22.0
44

55
require (
6+
github.com/Venafi/vcert/v5 v5.7.1
67
github.com/cenkalti/backoff v2.2.1+incompatible
78
github.com/d4l3k/messagediff v1.2.1
89
github.com/fatih/color v1.17.0
@@ -22,13 +23,13 @@ require (
2223
k8s.io/api v0.31.1
2324
k8s.io/apimachinery v0.31.1
2425
k8s.io/client-go v0.31.1
26+
k8s.io/component-base v0.31.0
2527
sigs.k8s.io/controller-runtime v0.19.0
2628
sigs.k8s.io/yaml v1.4.0
2729
)
2830

2931
require (
3032
github.com/Khan/genqlient v0.7.0 // indirect
31-
github.com/Venafi/vcert/v5 v5.7.1 // indirect
3233
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
3334
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
3435
github.com/aymerick/douceur v0.2.0 // indirect
@@ -37,6 +38,7 @@ require (
3738
github.com/fsnotify/fsnotify v1.7.0 // indirect
3839
github.com/fxamacker/cbor/v2 v2.7.0 // indirect
3940
github.com/go-http-utils/headers v0.0.0-20181008091004-fed159eddc2a // indirect
41+
github.com/go-logr/zapr v1.3.0 // indirect
4042
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
4143
github.com/google/cel-go v0.20.1 // indirect
4244
github.com/google/gnostic-models v0.6.9-0.20230804172637-c7be7c783f49 // indirect
@@ -62,7 +64,6 @@ require (
6264
gopkg.in/ini.v1 v1.67.0 // indirect
6365
k8s.io/apiextensions-apiserver v0.31.0 // indirect
6466
k8s.io/apiserver v0.31.0 // indirect
65-
k8s.io/component-base v0.31.0 // indirect
6667
)
6768

6869
require (
@@ -100,7 +101,7 @@ require (
100101
google.golang.org/protobuf v1.34.2 // indirect
101102
gopkg.in/inf.v0 v0.9.1 // indirect
102103
gopkg.in/yaml.v3 v3.0.1
103-
k8s.io/klog/v2 v2.130.1 // indirect
104+
k8s.io/klog/v2 v2.130.1
104105
k8s.io/kube-openapi v0.0.0-20240430033511-f0e62f92d13f // indirect
105106
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
106107
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect

hack/e2e/test.sh

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,11 @@ EOF
191191
envsubst <application-team-1.yaml | kubectl apply -f -
192192
kubectl -n team-1 wait certificate app-0 --for=condition=Ready
193193

194-
# Wait for log message indicating success.
195-
# Filter out distracting data gatherer errors and warnings.
196-
# Show other useful log messages on stderr.
194+
# Wait 60s for log message indicating success.
195+
# Parse logs as JSON using jq to ensure logs are all JSON formatted.
197196
# Disable pipefail to prevent SIGPIPE (141) errors from tee
198197
# See https://unix.stackexchange.com/questions/274120/pipe-fail-141-when-piping-output-into-tee-why
199-
set +o pipefail
200198
kubectl logs deployments/venafi-kubernetes-agent \
201-
--follow \
202-
--namespace venafi \
203-
| tee >(grep -v -e "reflector\.go" -e "datagatherer" -e "data gatherer" >/dev/stderr) \
204-
| grep -q "Data sent successfully"
199+
--follow \
200+
--namespace venafi \
201+
| timeout 60 jq 'if .msg | test("Data sent successfully") then . | halt_error(0) end'

hack/e2e/values.venafi-kubernetes-agent.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,6 @@ config:
66
authentication:
77
venafiConnection:
88
enabled: true
9+
10+
extraArgs:
11+
- --logging-format=json

pkg/logs/logs.go

Lines changed: 154 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,161 @@
11
package logs
22

33
import (
4+
"bytes"
5+
"fmt"
46
"log"
7+
"log/slog"
58
"os"
9+
"strings"
10+
11+
"github.com/spf13/pflag"
12+
"k8s.io/apimachinery/pkg/util/runtime"
13+
"k8s.io/apimachinery/pkg/util/sets"
14+
"k8s.io/component-base/featuregate"
15+
"k8s.io/component-base/logs"
16+
logsapi "k8s.io/component-base/logs/api/v1"
17+
_ "k8s.io/component-base/logs/json/register"
18+
)
19+
20+
// venafi-kubernetes-agent follows [Kubernetes Logging Conventions] and writes
21+
// logs in [Kubernetes text logging format] by default. It does not support
22+
// named levels (aka. severity), instead it uses arbitrary levels. Errors and
23+
// warnings are logged to stderr and Info messages to stdout, because that is
24+
// how some cloud logging systems (notably Google Cloud Logs Explorer) assign a
25+
// severity (INFO or ERROR) in the UI. The agent's and vcert's logs are written
26+
// logged as Info messages with level=0.
27+
//
28+
// Further reading:
29+
// - [Kubernetes logging conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md)
30+
// - [Kubernetes text logging format](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/logging.md#text-logging-format)
31+
// - [Why not named levels, like Info/Warning/Error?](https://github.com/go-logr/logr?tab=readme-ov-file#why-not-named-levels-like-infowarningerror)
32+
// - [GKE logs best practices](https://cloud.google.com/kubernetes-engine/docs/concepts/about-logs#best_practices)
33+
// - [Structured Logging KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/1602-structured-logging/README.md)
34+
// - [Examples of using k8s.io/component-base/logs](https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/component-base/logs/example),
35+
// upon which this code was based.
36+
37+
var (
38+
// This is the Agent's logger. For now, it is still a *log.Logger, but we
39+
// mean to migrate everything to slog with the klog backend. We avoid using
40+
// log.Default because log.Default is already used by the VCert library, and
41+
// we need to keep the agent's logger from the VCert's logger to be able to
42+
// remove the `vCert: ` prefix from the VCert logs.
43+
Log *log.Logger
44+
45+
// All but the essential logging flags will be hidden to avoid overwhelming
46+
// the user. The hidden flags can still be used. For example if a user does
47+
// not like the split-stream behavior and a Venafi field engineer can
48+
// instruct them to patch --log-json-split-stream=false on to the Deployment
49+
// arguments.
50+
visibleFlagNames = sets.New[string]("v", "vmodule", "logging-format")
51+
// This default logging configuration will be updated with values from the
52+
// logging flags, even those that are hidden.
53+
configuration = logsapi.NewLoggingConfiguration()
54+
// Logging features will be added to this feature gate, but the
55+
// feature-gates flag will be hidden from the user.
56+
features = featuregate.NewFeatureGate()
657
)
758

8-
var Log = log.New(os.Stderr, "", log.LstdFlags)
59+
func init() {
60+
runtime.Must(logsapi.AddFeatureGates(features))
61+
// Turn on ALPHA options to enable the split-stream logging options.
62+
runtime.Must(features.OverrideDefault(logsapi.LoggingAlphaOptions, true))
63+
}
64+
65+
// AddFlags adds log related flags to the supplied flag set.
66+
//
67+
// The split-stream options are enabled by default, so that errors are logged to
68+
// stderr and info to stdout, allowing cloud logging systems to assign a
69+
// severity INFO or ERROR to the messages.
70+
func AddFlags(fs *pflag.FlagSet) {
71+
var tfs pflag.FlagSet
72+
logsapi.AddFlags(configuration, &tfs)
73+
features.AddFlag(&tfs)
74+
tfs.VisitAll(func(f *pflag.Flag) {
75+
if !visibleFlagNames.Has(f.Name) {
76+
tfs.MarkHidden(f.Name)
77+
}
78+
79+
// The original usage string includes details about how
80+
// JSON logging is only available when BETA logging features are
81+
// enabled, but that's not relevant here because the feature is enabled
82+
// by default.
83+
if f.Name == "logging-format" {
84+
f.Usage = `Sets the log format. Permitted formats: "json", "text".`
85+
}
86+
if f.Name == "log-text-split-stream" {
87+
f.DefValue = "true"
88+
runtime.Must(f.Value.Set("true"))
89+
}
90+
if f.Name == "log-json-split-stream" {
91+
f.DefValue = "true"
92+
runtime.Must(f.Value.Set("true"))
93+
}
94+
95+
// Since `--v` (which is the long form of `-v`) isn't the standard in
96+
// our projects (it only exists in cert-manager, webhook, and such),
97+
// let's rename it to the more common `--log-level`, which appears in
98+
// openshift-routes, csi-driver, trust-manager, and approver-policy.
99+
// More details at:
100+
// https://github.com/jetstack/jetstack-secure/pull/596#issuecomment-2421708181
101+
if f.Name == "v" {
102+
f.Name = "log-level"
103+
f.Shorthand = "v"
104+
}
105+
})
106+
fs.AddFlagSet(&tfs)
107+
}
108+
109+
// Initialize uses k8s.io/component-base/logs, to configure the following global
110+
// loggers: log, slog, and klog. All are configured to write in the same format.
111+
func Initialize() {
112+
// This configures the global logger in klog *and* slog, if compiled with Go
113+
// >= 1.21.
114+
logs.InitLogs()
115+
if err := logsapi.ValidateAndApply(configuration, features); err != nil {
116+
fmt.Fprintf(os.Stderr, "Error in logging configuration: %v\n", err)
117+
os.Exit(2)
118+
}
119+
120+
// Thanks to logs.InitLogs, slog.Default now uses klog as its backend. Thus,
121+
// the client-go library, which relies on klog.Info, has the same logger as
122+
// the agent, which still uses log.Printf.
123+
slog := slog.Default()
124+
125+
Log = &log.Logger{}
126+
Log.SetOutput(LogToSlogWriter{Slog: slog, Source: "agent"})
127+
128+
// Let's make sure the VCert library, which is the only library we import to
129+
// be using the global log.Default, also uses the common slog logger.
130+
vcertLog := log.Default()
131+
vcertLog.SetOutput(LogToSlogWriter{Slog: slog, Source: "vcert"})
132+
// This is a work around for a bug in vcert where it adds a `vCert: ` prefix
133+
// to the global log logger. It can be removed when this is fixed upstream
134+
// in vcert: https://github.com/Venafi/vcert/pull/512
135+
vcertLog.SetPrefix("")
136+
}
137+
138+
type LogToSlogWriter struct {
139+
Slog *slog.Logger
140+
Source string
141+
}
142+
143+
func (w LogToSlogWriter) Write(p []byte) (n int, err error) {
144+
// log.Printf writes a newline at the end of the message, so we need to trim
145+
// it.
146+
p = bytes.TrimSuffix(p, []byte("\n"))
147+
148+
message := string(p)
149+
if strings.Contains(message, "error") ||
150+
strings.Contains(message, "failed") ||
151+
strings.Contains(message, "fatal") ||
152+
strings.Contains(message, "Failed") ||
153+
strings.Contains(message, "While evaluating configuration") ||
154+
strings.Contains(message, "data-path override present") ||
155+
strings.Contains(message, "Cannot marshal readings") {
156+
w.Slog.With("source", w.Source).Error(message)
157+
} else {
158+
w.Slog.With("source", w.Source).Info(message)
159+
}
160+
return len(p), nil
161+
}

0 commit comments

Comments
 (0)