Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 78dcd57

Browse files
authored
fix/sg: fix mangled log output from sg start and sg run (#63405)
Right now `sg run` / `sg start` can horribly mangle multi-line output. A nicely annotated report from @unknwon: ![image](https://github.com/sourcegraph/sourcegraph/assets/23356519/38acbaf9-89dc-4d4b-9fd7-b601f5654240) Replacing the "buffered process logger" thing with https://github.com/bobheadxi/streamline which powers `sourcegraph/run` etc (fairly reliably if I do say so myself) fixes this for a few cases where I can reliably repro wonky misordered output 😁 ## Test plan `sg start dotcom` with `sg.config.overwrite.yaml`: ```yaml commands: enterprise-portal: env: SRC_LOG_LEVEL: debug PG_QUERY_LOGGING: true ``` Log scope `pgx.devtracer` is consistently formatted ✅ , even with high volume of logs ![image](https://github.com/sourcegraph/sourcegraph/assets/23356519/5c46f94f-e388-477a-94d3-151d5a3c7468) Also don't see anything suspicious happening after running for a while
1 parent c437c21 commit 78dcd57

File tree

5 files changed

+38
-130
lines changed

5 files changed

+38
-130
lines changed

dev/sg/internal/run/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ go_library(
3434
"@com_github_nxadm_tail//:tail",
3535
"@com_github_rjeczalik_notify//:notify",
3636
"@com_github_sourcegraph_conc//pool",
37+
"@dev_bobheadxi_go_streamline//pipe",
3738
],
3839
)
3940

dev/sg/internal/run/command.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/grafana/regexp"
1616
"github.com/sourcegraph/conc/pool"
17+
"go.bobheadxi.dev/streamline/pipe"
1718

1819
"github.com/sourcegraph/sourcegraph/dev/sg/internal/secrets"
1920
"github.com/sourcegraph/sourcegraph/dev/sg/internal/std"
@@ -405,14 +406,22 @@ func (sc *startedCmd) getOutputWriter(ctx context.Context, opts *outputOptions,
405406
close(opts.start)
406407
}
407408

408-
writers = append(writers, newBufferedCmdLogger(ctx, sc.opts.name, std.Out.Output, opts.start))
409+
writers = append(writers, newOutputPipe(ctx, sc.opts.name, std.Out.Output, opts.start))
409410
}
410411

411412
if sgConn != nil {
412-
sink := func(data string) {
413-
sgConn.Write([]byte(fmt.Sprintf("%s: %s\n", sc.opts.name, data)))
414-
}
415-
writers = append(writers, process.NewLogger(ctx, sink))
413+
w, stream := pipe.NewStream()
414+
go func() {
415+
err := stream.Stream(func(line string) {
416+
_, _ = sgConn.Write([]byte(fmt.Sprintf("%s: %s\n", sc.opts.name, line)))
417+
})
418+
_ = w.CloseWithError(err)
419+
}()
420+
go func() {
421+
<-ctx.Done()
422+
_ = w.CloseWithError(ctx.Err())
423+
}()
424+
writers = append(writers, w)
416425
}
417426

418427
return io.MultiWriter(writers...)

dev/sg/internal/run/logger.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@ package run
33
import (
44
"context"
55
"hash/fnv"
6+
"io"
67
"strconv"
8+
"sync"
9+
10+
"go.bobheadxi.dev/streamline/pipe"
711

812
"github.com/sourcegraph/sourcegraph/lib/output"
9-
"github.com/sourcegraph/sourcegraph/lib/process"
1013
)
1114

1215
func nameToColor(s string) output.Style {
@@ -31,20 +34,29 @@ var (
3134
lineFormat = "%s%s[%+" + strconv.Itoa(maxNameLength) + "s]%s %s"
3235
)
3336

34-
// newBufferedCmdLogger returns a new process.Logger with a unique color based on the name of the cmd
37+
// newOutputPipe returns a new output with a unique color based on the name of the cmd
3538
// that blocks until the given start signal and writes logs to the given output.Output.
36-
func newBufferedCmdLogger(ctx context.Context, name string, out *output.Output, start <-chan struct{}) *process.Logger {
39+
func newOutputPipe(ctx context.Context, name string, out *output.Output, start <-chan struct{}) io.Writer {
3740
name = compactName(name)
3841
color := nameToColor(name)
3942

40-
sink := func(data string) {
41-
go func() {
42-
<-start
43-
out.Writef(lineFormat, output.StyleBold, color, name, output.StyleReset, data)
44-
}()
45-
}
43+
w, stream := pipe.NewStream()
44+
go func() {
45+
var mux sync.Mutex
46+
<-start
47+
err := stream.Stream(func(line string) {
48+
mux.Lock()
49+
out.Writef(lineFormat, output.StyleBold, color, name, output.StyleReset, line)
50+
mux.Unlock()
51+
})
52+
_ = w.CloseWithError(err)
53+
}()
54+
go func() {
55+
<-ctx.Done()
56+
_ = w.CloseWithError(ctx.Err())
57+
}()
4658

47-
return process.NewLogger(ctx, sink)
59+
return w
4860
}
4961

5062
func compactName(name string) string {

lib/process/BUILD.bazel

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
33

44
go_library(
55
name = "process",
6-
srcs = [
7-
"logger.go",
8-
"pipe.go",
9-
],
6+
srcs = ["pipe.go"],
107
importpath = "github.com/sourcegraph/sourcegraph/lib/process",
118
tags = [TAG_PLATFORM_SOURCE],
129
visibility = ["//visibility:public"],

lib/process/logger.go

Lines changed: 0 additions & 111 deletions
This file was deleted.

0 commit comments

Comments
 (0)