server: fix misreported CRDB environment variables during startup

During server startup, reportConfiguration (see cli/start.go), logs the
values of CRDB and other, unredacted environment variables to the Ops channel.
These environment variables are read from envVarRegistry.cache (see
GetEnvVarsUsed) which is assumed to have been populated during the
initialization of the (Go) runtime. Specifically, EnvOrDefaultXXX must
be invoked, which has a side-effect of populating the cache.
When it's invoked outside of the initializer (e.g., inside a func
that's not invoked until after initilization), the corresponding
environment variable will _not_ be in envVarRegistry.cache. Hence, it
will not be reported.

Reporting values of modified environment variables is useful for quickly
assembling a configuration context during debugging. It's unfortunate
that the current design relies on indirectly populating
envVarRegistry.cache during the initialization _withoout_ any sort of
enforcement. The change in this PR doesn't attempt to fix it. Instead,
the following misreported environment variables are now reported,

 - COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING
 - COCKROACH_UPGRADE_TO_DEV_VERSION

Note, we also remove COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED which
was only used by roachprod. It has no effect after the PR [1] reverted
the related change.

Release note: None

[1] cockroachdb#57003

pkg/cli/demo.go

@@ -282,7 +282,7 @@ func runDemoInternal(
 
 		// Only print details about the telemetry configuration if the
 		// user has control over it.
-		if cluster.TelemetryOptOut() {
+		if cluster.TelemetryOptOut {
 			cliCtx.PrintlnUnlessEmbedded("#\n# Telemetry disabled by configuration.")
 		} else {
 			cliCtx.PrintlnUnlessEmbedded("#\n" +

pkg/clusterversion/cockroach_versions.go

@@ -615,6 +615,11 @@ const (
 	finalVersion = invalidVersionKey
 )
 
+// N.B. EnvOrDefaultBool has the desired side-effect of populating envVarRegistry.cache.
+// It has to be invoked during init; otherwise, cli/start.go:reportConfiguration will not report the
+// value of this environment variable in the server log, upon startup.
+var allowUpgradeToDev = envutil.EnvOrDefaultBool("COCKROACH_UPGRADE_TO_DEV_VERSION", false)
+
 var versionsSingleton = func() keyedVersions {
 	if developmentBranch {
 		// If this is a dev branch, we offset every version +1M major versions into
@@ -639,7 +644,7 @@ var versionsSingleton = func() keyedVersions {
 		// renumber only 2-4 to be +1M. It would then step from 3 "up" to 1000002 --
 		// which conceptually is actually back down to 2 -- then back to to 1000003,
 		// then on to 1000004, etc.
-		skipFirst := envutil.EnvOrDefaultBool("COCKROACH_UPGRADE_TO_DEV_VERSION", false)
+		skipFirst := allowUpgradeToDev
 		const devOffset = 1000000
 		first := true
 		for i := range rawVersionsSingleton {

pkg/roachprod/config/config.go

@@ -86,12 +86,11 @@ func DefaultEnvVars() []string {
 		// when moving snapshots around, though.
 		// (For other perf. related knobs, see https://github.com/cockroachdb/cockroach/issues/17165)
 		"COCKROACH_ENABLE_RPC_COMPRESSION=false",
-		// Get rid of an annoying popup in the UI.
-		"COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true",
 		// Allow upgrading a stable release data-dir to a dev version.
 		// N.B. many roachtests which perform upgrade scenarios require this env. var after changes in [1]; otherwise,
 		// the tests will fail even on release branches when attempting to upgrade previous (stable) release to an alpha.
 		// [1] https://github.com/cockroachdb/cockroach/pull/87468
+		// TODO(SR): should remove this in light of https://github.com/cockroachdb/cockroach/issues/92608
 		"COCKROACH_UPGRADE_TO_DEV_VERSION=true",
 	}
 }

pkg/server/server.go

@@ -1724,7 +1724,7 @@ func (s *Server) PreStart(ctx context.Context) error {
 	}
 
 	// If enabled, start reporting diagnostics.
-	if s.cfg.StartDiagnosticsReporting && !cluster.TelemetryOptOut() {
+	if s.cfg.StartDiagnosticsReporting && !cluster.TelemetryOptOut {
 		s.startDiagnostics(workersCtx)
 	}
 

pkg/server/tenant.go

@@ -582,7 +582,7 @@ func (s *SQLServerWrapper) PreStart(ctx context.Context) error {
 	}
 
 	// If enabled, start reporting diagnostics.
-	if s.sqlServer.cfg.StartDiagnosticsReporting && !cluster.TelemetryOptOut() {
+	if s.sqlServer.cfg.StartDiagnosticsReporting && !cluster.TelemetryOptOut {
 		s.startDiagnostics(workersCtx)
 	}
 

pkg/settings/cluster/cluster_settings.go

@@ -69,11 +69,6 @@ type OverridesInformer interface {
 	IsOverridden(settingName string) bool
 }
 
-// TelemetryOptOut is a place for controlling whether to opt out of telemetry or not.
-func TelemetryOptOut() bool {
-	return envutil.EnvOrDefaultBool("COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING", false)
-}
-
 // NoSettings is used when a func requires a Settings but none is available
 // (for example, a CLI subcommand that does not connect to a cluster).
 var NoSettings *Settings // = nil
@@ -92,6 +87,9 @@ const (
 	CPUProfileWithLabels
 )
 
+// TelemetryOptOut is a place for controlling whether to opt out of telemetry or not.
+var TelemetryOptOut = envutil.EnvOrDefaultBool("COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING", false)
+
 // CPUProfileType returns the type of CPU profile being recorded, if any.
 // This can be used by moving parts across the system to add profiler labels
 // which are too expensive to be enabled at all times. If no profile is

pkg/upgrade/upgrades/permanent_upgrades.go

@@ -64,7 +64,7 @@ func optInToDiagnosticsStatReporting(
 	ctx context.Context, _ clusterversion.ClusterVersion, deps upgrade.TenantDeps,
 ) error {
 	// We're opting-out of the automatic opt-in. See discussion in updates.go.
-	if cluster.TelemetryOptOut() {
+	if cluster.TelemetryOptOut {
 		return nil
 	}
 	_, err := deps.InternalExecutor.Exec(