roachprod/roachtest/logictest: unconditionally disable Sentry crash reporting

Previously, the environment variable, `COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING=1`
was used to disable both telemetry and crash reporting. However, the default for
`diagnostics.reporting.enabled` has flipped from `false` to `true` as of [1].
The same PR added a regression wrt to the environment variable–it no longer
disabled crash reporting via Sentry.

Recall, we prefer to disable Sentry crash reporting for _all_
tests since our (internal) test artifacts contain a superset
of contextual information; i.e., Sentry reports are of limited
use for test failures; as such they add more noise than signal.

Also recall, unit tests effectively disable Sentry by not invoking
`cli.doMain`, which invokes `SetupCrashReporter`. Both integration (docker)
and roachtests rely on `COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING=1`.
Mixedversion logic tests until this change, simply assumed that
`diagnostics.reporting.enabled` defaulted to `false`.

This change fixes the regression, introduced in [1], by correctly
updating `diagnostics.reporting.enabled` to affect the environment
variable. It also sets `COCKROACH_CRASH_REPORTS` to the empty string,
which disables crash reporting _before_ a server is fully initialized.

[1] #131097

Fixes: #145457
Epic: none
Release note: None

Author: srosenberg

pkg/roachprod/install/cockroach.go

@@ -864,7 +864,10 @@ func (c *SyncedCluster) generateStartCmd(
 		EnvVars: append(append([]string{
 			fmt.Sprintf("ROACHPROD=%s", c.roachprodEnvValue(node)),
 			"GOTRACEBACK=crash",
+			// N.B. disable telemetry (see `TelemetryOptOut`).
 			"COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING=1",
+			// N.B. set crash reporting URL (see `crashReportURL()`) to the empty string to disable Sentry crash reports.
+			"COCKROACH_CRASH_REPORTS=",
 		}, c.Env...), getEnvVars()...),
 		Binary:              cockroachNodeBinary(c, node),
 		Args:                args,

pkg/sql/logictest/logic.go

@@ -1334,6 +1334,8 @@ func (t *logicTest) newTestServerCluster(bootstrapBinaryPath, upgradeBinaryPath
 	t.logsDir = logsDir
 
 	var envVars []string
+	// Set crash reporting URL to the empty string to disable Sentry crash reports.
+	envVars = append(envVars, "COCKROACH_CRASH_REPORTS=")
 	if strings.Contains(upgradeBinaryPath, "cockroach-short") {
 		// If we're using a cockroach-short binary, that means it was
 		// locally built, so we need to opt-out of version offsetting to

pkg/upgrade/upgrades/permanent_upgrades.go

@@ -161,13 +161,16 @@ func addRootToAdminRole(ctx context.Context, txn isql.Txn) error {
 func optInToDiagnosticsStatReporting(
 	ctx context.Context, _ clusterversion.ClusterVersion, deps upgrade.TenantDeps,
 ) error {
+	// N.B. The default for `diagnostics.reporting.enabled` is `true` as of [1].
+	// [1] https://github.com/cockroachdb/cockroach/pull/131097
+	optIn := true
 	// We're opting-out of the automatic opt-in. See discussion in updates.go.
 	if cluster.TelemetryOptOut {
-		return nil
+		optIn = false
 	}
 	_, err := deps.InternalExecutor.Exec(
 		ctx, "optInToDiagnosticsStatReporting", nil, /* txn */
-		`SET CLUSTER SETTING diagnostics.reporting.enabled = true`)
+		fmt.Sprintf(`SET CLUSTER SETTING diagnostics.reporting.enabled = %t`, optIn))
 	if errors.Is(err, cluster.SettingOverrideErr) {
 		return nil
 	}

pkg/util/log/logcrash/crash_reporting.go

@@ -40,11 +40,13 @@ var (
 	// Collecting this data from production clusters helps us understand and improve
 	// how our storage systems behave in real-world use cases.
 	//
-	// Note: while the setting itself is actually defined with a default value of
-	// `false`, it is usually automatically set to `true` when a cluster is created
-	// (or is migrated from a earlier beta version). This can be prevented with the
+	// Note: while the setting defaults to `true`, it can be overridden with the
 	// env var COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING.
 	//
+	// Note: while this setting also controls crash reporting (see logcrash.ShouldSendReport), the updated setting
+	// isn't affected until after the server startup sequence is complete. Thus, if you also must disable crash reporting
+	// during server startup, you should set the env var `COCKROACH_CRASH_REPORTS=` to the empty value.
+	//
 	// Doing this, rather than just using a default of `true`, means that a node
 	// will not errantly send a report using a default before loading settings.
 	DiagnosticsReportingEnabled = settings.RegisterBoolSetting(