Skip to content

Commit

Permalink
Close the log Writer used for the std log
Browse files Browse the repository at this point in the history
  • Loading branch information
codebien committed Dec 23, 2022
1 parent ab92a6c commit 6c51141
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 5 deletions.
13 changes: 12 additions & 1 deletion cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,18 @@ func (c *rootCommand) persistentPreRunE(cmd *cobra.Command, args []string) error
c.loggerIsRemote = true
}

stdlog.SetOutput(c.globalState.logger.Writer())
// Sometimes the Go runtime uses the standard log output to
// log some messages directly.
// It does when an invalid char is found in a Cookie.
// Check for details https://github.com/grafana/k6/issues/711#issue-341414887
stdlog.SetOutput(func() io.Writer {
w := c.globalState.logger.Writer()
go func() {
<-c.globalState.ctx.Done()
_ = w.Close()
}()
return w
}())
c.globalState.logger.Debugf("k6 version: v%s", consts.FullVersion())
return nil
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,7 @@ func TestMain(m *testing.M) {
}()

defer func() {
// TODO: figure out why logrus' `Entry.WriterLevel` goroutine sticks
// around and remove this exception.
opt := goleak.IgnoreTopFunction("io.(*pipe).read")
if err := goleak.Find(opt); err != nil {
if err := goleak.Find(); err != nil {
fmt.Println(err) //nolint:forbidigo
exitCode = 3
}
Expand All @@ -91,6 +88,10 @@ func getFreeBindAddr(t *testing.T) string {
for i := 0; i < 100; i++ {
port := atomic.AddUint64(&portRangeStart, 1)
addr := net.JoinHostPort("localhost", strconv.FormatUint(port, 10))
:q
:q
:q
:q

listener, err := net.Listen("tcp", addr)
if err != nil {
Expand Down
48 changes: 48 additions & 0 deletions cmd/stdlog_integration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package cmd

import (
"bytes"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.k6.io/k6/lib/testutils/httpmultibin"
)

// SetOutput sets the global log so it is racy with other tests
//
//nolint:paralleltest
func TestStdLogOutputIsSet(t *testing.T) {
tb := httpmultibin.NewHTTPMultiBin(t)
ts := newGlobalTestState(t)

// Sometimes the Go runtime uses the standard log output to
// log some messages directly.
// It does when an invalid char is found in a Cookie.
// Check for details https://github.com/grafana/k6/issues/711#issue-341414887
ts.stdIn = bytes.NewReader([]byte(tb.Replacer.Replace(`
import http from 'k6/http';
export const options = {
hosts: {
"HTTPSBIN_DOMAIN": "HTTPSBIN_IP",
},
insecureSkipTLSVerify: true,
}
export default function () {
http.get("HTTPSBIN_URL/get", {
"cookies": {
"test": "\""
},
})
}`)))

ts.args = []string{"k6", "version"}
ts.args = []string{"k6", "run", "-i", "1", "-"}
newRootCommand(ts.globalState).execute()

entries := ts.loggerHook.Drain()
require.Len(t, entries, 1)
assert.Contains(t, entries[0].Message, "Cookie.Value; dropping invalid bytes")
}

0 comments on commit 6c51141

Please sign in to comment.