Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
82891: log: embed error hints in unstructured entries r=abargainier,cameronnunez a=knz

Fixes cockroachdb#80875.
cc `@dhartunian` 

With this commit, if a `log.Infof` (or warning, error, fatal etc) call
is passed an `error` object as argument, and that error object
contains hints, the hints are copied into the resulting log entry and
emitted to the sink.

At this time, we only have hints in very few places. This change is
thus merely infrastructure that will promote the implementation of
hints over time.

Example, before:
```
W220614 15:55:33.935575 1 cli/start.go:184  [n?] 5  cannot create trace dir; traces will not be dumped: mkdir inflight_trace_dump: permission denied
```

Example, after:
```
W220614 15:57:58.669410 1 cli/start.go:185  [n?] 5  cannot create trace dir; traces will not be dumped: mkdir inflight_trace_dump: permission denied
W220614 15:57:58.669410 1 cli/start.go:185  [n?] 5 +HINT: Try changing the CWD of the cockroach process to a writable directory.
```

Release note (cli change): In some cases, CockroachDB will now include
recommended remediation actions alongside log messages. We expect more
suggestions to be added in later versions.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
  • Loading branch information
craig[bot] and knz committed Nov 1, 2022
2 parents 3506349 + d6f284e commit c25a0dc
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ func initTraceDir(ctx context.Context, dir string) {
// to the current directory (.). If running the process
// from a directory which is not writable, we won't
// be able to create a sub-directory here.
log.Warningf(ctx, "cannot create trace dir; traces will not be dumped: %+v", err)
err = errors.WithHint(err, "Try changing the CWD of the cockroach process to a writable directory.")
log.Warningf(ctx, "cannot create trace dir; traces will not be dumped: %v", err)
return
}
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/util/log/clog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/logtags"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -142,6 +143,20 @@ func TestInfo(t *testing.T) {
}
}

// Test that error hints are included.
func TestUnstructuredEntryEmbedsErrorHints(t *testing.T) {
defer leaktest.AfterTest(t)()
defer ScopeWithoutShowLogs(t).Close(t)

defer capture()()
err := errors.WithHint(errors.New("hello"), "world")
Infof(context.Background(), "hello %v", err)
t.Logf("log contents:\n%s", contents())
if !contains("HINT: "+startRedactable+"world"+endRedactable, t) {
t.Error("hint failed")
}
}

// Test that copyStandardLogTo panics on bad input.
func TestCopyStandardLogToPanic(t *testing.T) {
defer leaktest.AfterTest(t)()
Expand Down
20 changes: 20 additions & 0 deletions pkg/util/log/log_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/log/logpb"
"github.com/cockroachdb/cockroach/pkg/util/log/severity"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/errors"
"github.com/cockroachdb/logtags"
"github.com/cockroachdb/redact"
"github.com/cockroachdb/redact/interfaces"
Expand Down Expand Up @@ -242,10 +243,29 @@ func makeUnstructuredEntry(
} else {
buf.Printf(format, args...)
}
// Collect and append the hints, if any.
for _, a := range args {
if e, ok := a.(error); ok {
h := errors.FlattenHints(e)
if h != "" {
buf.Printf("\nHINT: %s", h)
}
}
}
res.payload = makeRedactablePayload(ctx, buf.RedactableString())
} else {
var buf strings.Builder
formatArgs(&buf, format, args...)
// Collect and append the hints, if any.
for _, a := range args {
if e, ok := a.(error); ok {
h := errors.FlattenHints(e)
if h != "" {
buf.WriteString("\nHINT:")
buf.WriteString(h)
}
}
}
res.payload = makeUnsafePayload(ctx, buf.String())
}

Expand Down

0 comments on commit c25a0dc

Please sign in to comment.