Skip to content

Commit

Permalink
pkg/util/debugutil: introduce debug.Stack() wrapper
Browse files Browse the repository at this point in the history
Until now, all stack traces were treated as untyped strings and hence
redacted. Stack traces don't need to be redacted. Because they don't
contain any PII.

This commit introduces a new function, Stack(), in the debugutil package
as a replacement for the standard debug.Stack(). The new function wraps
the stack trace contents in a []byte-aliased type "SafeStack" that
implements the SafeValue interface, ensuring it is not redacted. This
function can be used as a drop-in replacement for debug.Stack().

Part of: CRDB-15292
Epic: CRDB-37533
Release note: None
  • Loading branch information
arjunmahishi committed Dec 9, 2024
1 parent 683618f commit 5d83c6c
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 2 deletions.
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,7 @@ ALL_TESTS = [
"//pkg/util/ctxgroup:ctxgroup_test",
"//pkg/util/ctxlog:ctxlog_test",
"//pkg/util/ctxutil:ctxutil_test",
"//pkg/util/debugutil:debugutil_test",
"//pkg/util/duration:duration_test",
"//pkg/util/encoding/csv:csv_test",
"//pkg/util/encoding:encoding_test",
Expand Down Expand Up @@ -2459,6 +2460,7 @@ GO_TARGETS = [
"//pkg/util/ctxutil:ctxutil",
"//pkg/util/ctxutil:ctxutil_test",
"//pkg/util/debugutil:debugutil",
"//pkg/util/debugutil:debugutil_test",
"//pkg/util/duration:duration",
"//pkg/util/duration:duration_test",
"//pkg/util/encoding/csv:csv",
Expand Down
3 changes: 3 additions & 0 deletions pkg/testutils/lint/passes/redactcheck/redactcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
"github.com/cockroachdb/redact/internal/redact": {
"safeWrapper": {},
},
"github.com/cockroachdb/cockroach/pkg/util/debugutil": {
"SafeStack": {},
},
}
ty := recv[0].Type
reportFailure := func() {
Expand Down
18 changes: 16 additions & 2 deletions pkg/util/debugutil/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "debugutil",
srcs = ["debugutil.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/util/debugutil",
visibility = ["//visibility:public"],
deps = ["@com_github_elastic_gosigar//:gosigar"],
deps = [
"@com_github_cockroachdb_redact//:redact",
"@com_github_cockroachdb_redact//interfaces",
"@com_github_elastic_gosigar//:gosigar",
],
)

go_test(
name = "debugutil_test",
srcs = ["debugutil_test.go"],
embed = [":debugutil"],
deps = [
"@com_github_cockroachdb_redact//:redact",
"@com_github_stretchr_testify//require",
],
)
16 changes: 16 additions & 0 deletions pkg/util/debugutil/debugutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package debugutil
import (
"os"
"path/filepath"
"runtime/debug"
"sync/atomic"

"github.com/elastic/gosigar"
Expand Down Expand Up @@ -43,3 +44,18 @@ func init() {
return false
}(os.Getppid()))
}

type SafeStack []byte

func (s SafeStack) SafeValue() {}

// Stack wraps the output of debug.Stack() with redact.Safe() to avoid
// unnecessary redaction.
//
// WARNING: Calling this function grabs system-level locks and could cause high
// system CPU usage resulting in the Go runtime to lock up if called too
// frequently, even if called only in error-handling pathways. Use sporadically
// and only when necessary.
func Stack() SafeStack {
return SafeStack(debug.Stack())
}
24 changes: 24 additions & 0 deletions pkg/util/debugutil/debugutil_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2023 The Cockroach Authors.
//
// Use of this software is governed by the CockroachDB Software License
// included in the /LICENSE file.

package debugutil

import (
"testing"

"github.com/cockroachdb/redact"
"github.com/stretchr/testify/require"
)

func TestStack(t *testing.T) {
var buf redact.StringBuilder
buf.Print(Stack())
buf.Printf("%s", Stack())
buf.Printf("%v", Stack())
out := buf.RedactableString()

require.NotContains(t, out, "‹")
require.NotContains(t, out, "›")
}

0 comments on commit 5d83c6c

Please sign in to comment.