Skip to content

Commit 7d99512

Browse files
feat: JobLoggerFactory (#1496)
Remove overriding io.Stdout in TestMaskValues to prevent deadlock in GitHub Actions
1 parent 0f8861b commit 7d99512

File tree

2 files changed

+62
-35
lines changed

2 files changed

+62
-35
lines changed

pkg/runner/logger.go

+46-32
Original file line numberDiff line numberDiff line change
@@ -57,31 +57,48 @@ func WithMasks(ctx context.Context, masks *[]string) context.Context {
5757
return context.WithValue(ctx, masksContextKeyVal, masks)
5858
}
5959

60+
type JobLoggerFactory interface {
61+
WithJobLogger() *logrus.Logger
62+
}
63+
64+
type jobLoggerFactoryContextKey string
65+
66+
var jobLoggerFactoryContextKeyVal = (jobLoggerFactoryContextKey)("jobloggerkey")
67+
68+
func WithJobLoggerFactory(ctx context.Context, factory JobLoggerFactory) context.Context {
69+
return context.WithValue(ctx, jobLoggerFactoryContextKeyVal, factory)
70+
}
71+
6072
// WithJobLogger attaches a new logger to context that is aware of steps
6173
func WithJobLogger(ctx context.Context, jobID string, jobName string, config *Config, masks *[]string, matrix map[string]interface{}) context.Context {
62-
mux.Lock()
63-
defer mux.Unlock()
64-
65-
var formatter logrus.Formatter
66-
if config.JSONLogger {
67-
formatter = &jobLogJSONFormatter{
68-
formatter: &logrus.JSONFormatter{},
69-
masker: valueMasker(config.InsecureSecrets, config.Secrets),
70-
}
74+
ctx = WithMasks(ctx, masks)
75+
76+
var logger *logrus.Logger
77+
if jobLoggerFactory, ok := ctx.Value(jobLoggerFactoryContextKeyVal).(JobLoggerFactory); ok && jobLoggerFactory != nil {
78+
logger = jobLoggerFactory.WithJobLogger()
7179
} else {
72-
formatter = &jobLogFormatter{
73-
color: colors[nextColor%len(colors)],
74-
masker: valueMasker(config.InsecureSecrets, config.Secrets),
80+
var formatter logrus.Formatter
81+
if config.JSONLogger {
82+
formatter = &logrus.JSONFormatter{}
83+
} else {
84+
mux.Lock()
85+
defer mux.Unlock()
86+
nextColor++
87+
formatter = &jobLogFormatter{
88+
color: colors[nextColor%len(colors)],
89+
}
7590
}
76-
}
7791

78-
nextColor++
79-
ctx = WithMasks(ctx, masks)
92+
logger = logrus.New()
93+
logger.SetOutput(os.Stdout)
94+
logger.SetLevel(logrus.GetLevel())
95+
logger.SetFormatter(formatter)
96+
}
8097

81-
logger := logrus.New()
82-
logger.SetFormatter(formatter)
83-
logger.SetOutput(os.Stdout)
84-
logger.SetLevel(logrus.GetLevel())
98+
logger.SetFormatter(&maskedFormatter{
99+
Formatter: logger.Formatter,
100+
masker: valueMasker(config.InsecureSecrets, config.Secrets),
101+
})
85102
rtn := logger.WithFields(logrus.Fields{
86103
"job": jobName,
87104
"jobID": jobID,
@@ -149,16 +166,22 @@ func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor
149166
}
150167
}
151168

152-
type jobLogFormatter struct {
153-
color int
169+
type maskedFormatter struct {
170+
logrus.Formatter
154171
masker entryProcessor
155172
}
156173

174+
func (f *maskedFormatter) Format(entry *logrus.Entry) ([]byte, error) {
175+
return f.Formatter.Format(f.masker(entry))
176+
}
177+
178+
type jobLogFormatter struct {
179+
color int
180+
}
181+
157182
func (f *jobLogFormatter) Format(entry *logrus.Entry) ([]byte, error) {
158183
b := &bytes.Buffer{}
159184

160-
entry = f.masker(entry)
161-
162185
if f.isColored(entry) {
163186
f.printColored(b, entry)
164187
} else {
@@ -225,12 +248,3 @@ func checkIfTerminal(w io.Writer) bool {
225248
return false
226249
}
227250
}
228-
229-
type jobLogJSONFormatter struct {
230-
masker entryProcessor
231-
formatter *logrus.JSONFormatter
232-
}
233-
234-
func (f *jobLogJSONFormatter) Format(entry *logrus.Entry) ([]byte, error) {
235-
return f.formatter.Format(f.masker(entry))
236-
}

pkg/runner/runner_test.go

+16-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package runner
22

33
import (
4+
"bytes"
45
"context"
56
"fmt"
7+
"io"
68
"os"
79
"path/filepath"
810
"runtime"
@@ -343,6 +345,17 @@ func TestRunDifferentArchitecture(t *testing.T) {
343345
tjfi.runTest(context.Background(), t, &Config{ContainerArchitecture: "linux/arm64"})
344346
}
345347

348+
type maskJobLoggerFactory struct {
349+
Output bytes.Buffer
350+
}
351+
352+
func (f *maskJobLoggerFactory) WithJobLogger() *log.Logger {
353+
logger := log.New()
354+
logger.SetOutput(io.MultiWriter(&f.Output, os.Stdout))
355+
logger.SetLevel(log.DebugLevel)
356+
return logger
357+
}
358+
346359
func TestMaskValues(t *testing.T) {
347360
assertNoSecret := func(text string, secret string) {
348361
index := strings.Index(text, "composite secret")
@@ -366,9 +379,9 @@ func TestMaskValues(t *testing.T) {
366379
platforms: platforms,
367380
}
368381

369-
output := captureOutput(t, func() {
370-
tjfi.runTest(context.Background(), t, &Config{})
371-
})
382+
logger := &maskJobLoggerFactory{}
383+
tjfi.runTest(WithJobLoggerFactory(common.WithLogger(context.Background(), logger.WithJobLogger()), logger), t, &Config{})
384+
output := logger.Output.String()
372385

373386
assertNoSecret(output, "secret value")
374387
assertNoSecret(output, "YWJjCg==")

0 commit comments

Comments
 (0)