util/must: add runtime assertion API

This patch adds a convenient and canonical API for runtime assertions,
inspired by the Testify package used for Go test assertions. It is
intended to encourage liberal use of runtime assertions throughout the
code base, by making it as easy as possible to write assertions that
follow best practices. It does not attempt to reinvent the wheel, but
instead builds on existing infrastructure.

Assertion failures are fatal in all non-release builds, including
roachprod clusters and roachtests, to ensure they will be noticed. In
release builds, they instead log the failure and report it to Sentry (if
enabled), and return an assertion error to the caller for propagation.
This avoids excessive disruption in production environments, where an
assertion failure is often scoped to an individual RPC request,
transaction, or range, and crashing the node can turn a minor problem
into a full-blown outage. It is still possible to kill the node when
appropriate via `log.Fatalf`, but this should be the exception rather
than the norm.

It also supports expensive assertions that must be compiled out of
normal dev/test/release builds for performance reasons. These are
instead enabled in special test builds.

This is intended to be used instead of other existing assertion
mechanisms, which have various shortcomings:

* `log.Fatalf`: kills the node even in release builds, which can cause
  severe disruption over often minor issues.

* `errors.AssertionFailedf`: only suitable when we have an error return
  path, does not fatal in non-release builds, and are not always
  notified in release builds.

* `logcrash.ReportOrPanic`: panics rather than fatals, which can leave
  the node limping along. Requires the caller to implement separate
  assertion handling in release builds, which is easy to forget. Also
  requires propagating cluster settings, which aren't always available.

* `buildutil.CrdbTestBuild`: only enabled in Go tests, not roachtests,
  roachprod clusters, or production clusters.

* `util.RaceEnabled`: only enabled in race builds. Expensive assertions
  should be possible to run without the additional overhead of the race
  detector.

For more details and examples, see the `must` package documentation.

Epic: none
Release note: None

Author: erikgrinaker

pkg/BUILD.bazel

@@ -661,6 +661,7 @@ ALL_TESTS = [
     "//pkg/util/metric/aggmetric:aggmetric_test",
     "//pkg/util/metric:metric_test",
     "//pkg/util/mon:mon_test",
+    "//pkg/util/must:must_test",
     "//pkg/util/netutil/addr:addr_test",
     "//pkg/util/netutil:netutil_test",
     "//pkg/util/optional:optional_test",
@@ -2337,6 +2338,8 @@ GO_TARGETS = [
     "//pkg/util/metric:metric_test",
     "//pkg/util/mon:mon",
     "//pkg/util/mon:mon_test",
+    "//pkg/util/must:must",
+    "//pkg/util/must:must_test",
     "//pkg/util/netutil/addr:addr",
     "//pkg/util/netutil/addr:addr_test",
     "//pkg/util/netutil:netutil",

pkg/testutils/lint/gcassert_paths.txt

@@ -29,3 +29,4 @@ util
 util/admission
 util/hlc
 util/intsets
+util/must

pkg/testutils/lint/passes/fmtsafe/functions.go

@@ -162,6 +162,35 @@ var requireConstFmt = map[string]bool{
 
 	"(github.com/cockroachdb/cockroach/pkg/kv/kvpb.TestPrinter).Printf": true,
 
+	// must assertions.
+	"github.com/cockroachdb/cockroach/pkg/util/must.Fail":           true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.failDepth":      true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.True":           true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.False":          true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Equal":          true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotEqual":       true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Greater":        true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.GreaterOrEqual": true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Less":           true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.LessOrEqual":    true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.EqualBytes":     true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotEqualBytes":  true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.PrefixBytes":    true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotPrefixBytes": true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Len":            true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Contains":       true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotContains":    true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Empty":          true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotEmpty":       true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Nil":            true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotNil":         true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Same":           true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotSame":        true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Zero":           true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NotZero":        true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.Error":          true,
+	"github.com/cockroachdb/cockroach/pkg/util/must.NoError":        true,
+
 	// Error things are populated in the init() message.
 }
 

pkg/util/log/logcrash/BUILD.bazel

@@ -14,6 +14,7 @@ go_library(
         "//pkg/util/envutil",
         "//pkg/util/log",
         "//pkg/util/log/severity",
+        "//pkg/util/must",
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_getsentry_sentry_go//:sentry-go",

pkg/util/log/logcrash/crash_reporting.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/severity"
+	"github.com/cockroachdb/cockroach/pkg/util/must"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	sentry "github.com/getsentry/sentry-go"
@@ -318,7 +319,7 @@ const (
 	// was generated via a log.Fatal call.
 	ReportTypeLogFatal
 	// ReportTypeAssertionFailure signifies that an assertion was violated (see
-	// expect package).
+	// must package).
 	ReportTypeAssertionFailure
 )
 
@@ -453,4 +454,7 @@ func init() {
 	log.MaybeSendCrashReport = func(ctx context.Context, err error) {
 		maybeSendCrashReport(ctx, err, ReportTypeLogFatal)
 	}
+	must.MaybeSendReport = func(ctx context.Context, err error) {
+		maybeSendCrashReport(ctx, err, ReportTypeAssertionFailure)
+	}
 }

pkg/util/must/BUILD.bazel

@@ -0,0 +1,36 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "must",
+    srcs = ["must.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/util/must",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/build",
+        "//pkg/util",
+        "//pkg/util/envutil",
+        "//pkg/util/log",
+        "@com_github_cockroachdb_errors//:errors",
+        "@org_golang_x_exp//constraints",
+    ],
+)
+
+go_test(
+    name = "must_test",
+    srcs = ["must_test.go"],
+    args = ["-test.timeout=295s"],
+    data = glob(["testdata/**"]),
+    deps = [
+        ":must",
+        "//pkg/build",
+        "//pkg/keys",
+        "//pkg/testutils/datapathutils",
+        "//pkg/testutils/echotest",
+        "//pkg/util",
+        "//pkg/util/hlc",
+        "//pkg/util/leaktest",
+        "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
+        "@com_github_stretchr_testify//require",
+    ],
+)