remove phone-home diagnostics, update checks, crash reports

go.mod

@@ -61,7 +61,6 @@ require (
 	github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a
 	github.com/fraugster/parquet-go v0.10.0
 	github.com/fsnotify/fsnotify v1.5.1
-	github.com/getsentry/sentry-go v0.12.0
 	github.com/ghemawat/stream v0.0.0-20171120220530-696b145b53b9
 	github.com/go-sql-driver/mysql v1.6.0
 	github.com/go-swagger/go-swagger v0.26.1
@@ -199,6 +198,7 @@ require (
 	github.com/dimchansky/utfbom v1.1.1 // indirect
 	github.com/docker/go-units v0.4.0 // indirect
 	github.com/felixge/httpsnoop v1.0.1 // indirect
+	github.com/getsentry/sentry-go v0.12.0 // indirect
 	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/go-kit/log v0.1.0 // indirect
 	github.com/go-logfmt/logfmt v0.5.1 // indirect

pkg/cli/cli.go

@@ -125,11 +125,6 @@ func doMain(cmd *cobra.Command, cmdName string) error {
 		}
 	}
 
-	logcrash.SetupCrashReporter(
-		context.Background(),
-		cmdName,
-	)
-
 	defer logcrash.RecoverAndReportPanic(context.Background(), &serverCfg.Settings.SV)
 
 	return Run(os.Args[1:])

pkg/cli/context.go

@@ -618,7 +618,6 @@ func setDemoContextDefaults() {
 	demoCtx.RunWorkload = false
 	demoCtx.Localities = nil
 	demoCtx.GeoPartitionedReplicas = false
-	demoCtx.DisableTelemetry = false
 	demoCtx.DefaultKeySize = defaultKeySize
 	demoCtx.DefaultCALifetime = defaultCALifetime
 	demoCtx.DefaultCertLifetime = defaultCertLifetime

pkg/cli/demo.go

@@ -20,7 +20,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cli/cliflags"
 	"github.com/cockroachdb/cockroach/pkg/cli/democluster"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
-	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
@@ -40,10 +39,6 @@ subcommands: e.g. "cockroach demo startrek". See --help for a full list.
 
 By default, the 'movr' dataset is pre-loaded. You can also use --no-example-database
 to avoid pre-loading a dataset.
-
-cockroach demo attempts to connect to a Cockroach Labs server to send
-telemetry back to Cockroach Labs. In order to disable this behavior, set the
-environment variable "COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING" to true.
 `,
 	Example: `  cockroach demo`,
 	Args:    cobra.NoArgs,
@@ -141,8 +136,6 @@ func checkDemoConfiguration(
 		return nil, errors.Newf("--%s cannot be used with --%s", cliflags.Global.Name, cliflags.DemoNodeLocality.Name)
 	}
 
-	demoCtx.DisableTelemetry = cluster.TelemetryOptOut()
-
 	// Whether or not we enable enterprise feature is a combination of:
 	//
 	// - whether the user wants them (they can disable enterprise
@@ -280,17 +273,6 @@ func runDemo(cmd *cobra.Command, gen workload.Generator) (resErr error) {
 			)
 		}
 
-		// Only print details about the telemetry configuration if the
-		// user has control over it.
-		if demoCtx.DisableTelemetry {
-			cliCtx.PrintlnUnlessEmbedded("#\n# Telemetry disabled by configuration.")
-		} else {
-			cliCtx.PrintlnUnlessEmbedded("#\n" +
-				"# This demo session will send telemetry to Cockroach Labs in the background.\n" +
-				"# To disable this behavior, set the environment variable\n" +
-				"# COCKROACH_SKIP_ENABLING_DIAGNOSTIC_REPORTING=true.")
-		}
-
 		if demoCtx.disableEnterpriseFeatures {
 			cliCtx.PrintlnUnlessEmbedded("#\n# Enterprise features are disabled by configuration.\n")
 		}

pkg/cli/democluster/context.go

@@ -35,9 +35,6 @@ type Context struct {
 	// CacheSize is the size of the storage cache for each KV server.
 	CacheSize int64
 
-	// DisableTelemetry requests that telemetry be disabled.
-	DisableTelemetry bool
-
 	// NoExampleDatabase prevents the auto-creation of a demo database
 	// from a workload.
 	NoExampleDatabase bool

pkg/cli/democluster/demo_cluster.go

@@ -489,14 +489,6 @@ func (c *transientCluster) Start(
 			}
 		}
 
-		// Start up the update check loop.
-		// We don't do this in (*server.Server).Start() because we don't want this
-		// overhead and possible interference in tests.
-		if !c.demoCtx.DisableTelemetry {
-			c.infoLog(ctx, "starting telemetry")
-			c.firstServer.StartDiagnostics(ctx)
-		}
-
 		return nil
 	}(phaseCtx); err != nil {
 		return err

pkg/cli/mt_start_sql.go

@@ -20,7 +20,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/cli/clierrorplus"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/server"
-	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/sdnotify"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -152,13 +151,6 @@ func runStartSQL(cmd *cobra.Command, args []string) error {
 		log.Ops.Errorf(ctx, "failed to signal readiness using systemd protocol: %s", err)
 	}
 
-	// Start up the diagnostics reporting loop.
-	// We don't do this in (*server.SQLServer).preStart() because we don't
-	// want this overhead and possible interference in tests.
-	if !cluster.TelemetryOptOut() {
-		sqlServer.StartDiagnostics(ctx)
-	}
-
 	tenantClusterID := sqlServer.LogicalClusterID()
 
 	// Report the server identifiers and other server details

pkg/cli/start.go

@@ -647,12 +647,6 @@ If problems persist, please see %s.`
 			serverStatusMu.started = true
 			serverStatusMu.Unlock()
 
-			// Start up the diagnostics reporting and update check loops.
-			// We don't do this in (*server.Server).Start() because we don't
-			// want this overhead and possible interference in tests.
-			if !cluster.TelemetryOptOut() {
-				s.StartDiagnostics(ctx)
-			}
 			initialStart := s.InitialStart()
 
 			// Run SQL for new clusters.

pkg/server/admin.go

@@ -57,7 +57,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/iterutil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
-	"github.com/cockroachdb/cockroach/pkg/util/log/logcrash"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/quotapool"
@@ -1885,7 +1884,7 @@ func (s *adminServer) Cluster(
 	return &serverpb.ClusterResponse{
 		// TODO(knz): Respond with the logical cluster ID as well.
 		ClusterID:         storageClusterID.String(),
-		ReportingEnabled:  logcrash.DiagnosticsReportingEnabled.Get(&s.server.st.SV),
+		ReportingEnabled:  false,
 		EnterpriseEnabled: enterpriseEnabled,
 	}, nil
 }

pkg/server/admin_test.go

@@ -1027,10 +1027,10 @@ func TestAdminAPIEvents(t *testing.T) {
 		{"create_database", false, 0, false, 3},
 		{"drop_table", false, 0, false, 2},
 		{"create_table", false, 0, false, 3},
-		{"set_cluster_setting", false, 0, false, 4},
+		{"set_cluster_setting", false, 0, false, 3},
 		// We use limit=true with no limit here because otherwise the
 		// expCount will mess up the expected total count below.
-		{"set_cluster_setting", true, 0, true, 4},
+		{"set_cluster_setting", true, 0, true, 3},
 		{"create_table", true, 0, false, 3},
 		{"create_table", true, -1, false, 3},
 		{"create_table", true, 2, false, 2},
@@ -1389,46 +1389,40 @@ func TestAdminAPIUISeparateData(t *testing.T) {
 func TestClusterAPI(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
-	s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
+	s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
 	defer s.Stopper().Stop(context.Background())
 
-	testutils.RunTrueAndFalse(t, "reportingOn", func(t *testing.T, reportingOn bool) {
-		testutils.RunTrueAndFalse(t, "enterpriseOn", func(t *testing.T, enterpriseOn bool) {
-			// Override server license check.
-			if enterpriseOn {
-				old := base.CheckEnterpriseEnabled
-				base.CheckEnterpriseEnabled = func(_ *cluster.Settings, _ uuid.UUID, _, _ string) error {
-					return nil
-				}
-				defer func() { base.CheckEnterpriseEnabled = old }()
+	testutils.RunTrueAndFalse(t, "enterpriseOn", func(t *testing.T, enterpriseOn bool) {
+		// Override server license check.
+		if enterpriseOn {
+			old := base.CheckEnterpriseEnabled
+			base.CheckEnterpriseEnabled = func(_ *cluster.Settings, _ uuid.UUID, _, _ string) error {
+				return nil
 			}
+			defer func() { base.CheckEnterpriseEnabled = old }()
+		}
 
-			if _, err := db.Exec(`SET CLUSTER SETTING diagnostics.reporting.enabled = $1`, reportingOn); err != nil {
-				t.Fatal(err)
+		// We need to retry, because the cluster ID isn't set until after
+		// bootstrapping and because setting a cluster setting isn't necessarily
+		// instantaneous.
+		//
+		// Also note that there's a migration that affects `diagnostics.reporting.enabled`,
+		// so manipulating the cluster setting var directly is a bad idea.
+		testutils.SucceedsSoon(t, func() error {
+			var resp serverpb.ClusterResponse
+			if err := getAdminJSONProto(s, "cluster", &resp); err != nil {
+				return err
 			}
-
-			// We need to retry, because the cluster ID isn't set until after
-			// bootstrapping and because setting a cluster setting isn't necessarily
-			// instantaneous.
-			//
-			// Also note that there's a migration that affects `diagnostics.reporting.enabled`,
-			// so manipulating the cluster setting var directly is a bad idea.
-			testutils.SucceedsSoon(t, func() error {
-				var resp serverpb.ClusterResponse
-				if err := getAdminJSONProto(s, "cluster", &resp); err != nil {
-					return err
-				}
-				if a, e := resp.ClusterID, s.RPCContext().StorageClusterID.String(); a != e {
-					return errors.Errorf("cluster ID %s != expected %s", a, e)
-				}
-				if a, e := resp.ReportingEnabled, reportingOn; a != e {
-					return errors.Errorf("reportingEnabled = %t, wanted %t", a, e)
-				}
-				if a, e := resp.EnterpriseEnabled, enterpriseOn; a != e {
-					return errors.Errorf("enterpriseEnabled = %t, wanted %t", a, e)
-				}
-				return nil
-			})
+			if a, e := resp.ClusterID, s.RPCContext().StorageClusterID.String(); a != e {
+				return errors.Errorf("cluster ID %s != expected %s", a, e)
+			}
+			if resp.ReportingEnabled {
+				return errors.Errorf("reportingEnabled = %t, wanted %t", resp.ReportingEnabled, false)
+			}
+			if a, e := resp.EnterpriseEnabled, enterpriseOn; a != e {
+				return errors.Errorf("enterpriseEnabled = %t, wanted %t", a, e)
+			}
+			return nil
 		})
 	})
 }

pkg/server/diagnostics/diagnostics.go

@@ -12,117 +12,17 @@ package diagnostics
 
 import (
 	"context"
-	"math/rand"
-	"net/url"
-	"strconv"
-	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb"
 	"github.com/cockroachdb/cockroach/pkg/util/cloudinfo"
-	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/system"
-	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/shirou/gopsutil/v3/cpu"
 	"github.com/shirou/gopsutil/v3/host"
 	"github.com/shirou/gopsutil/v3/load"
 	"github.com/shirou/gopsutil/v3/mem"
 )
 
-// updatesURL is the URL used to check for new versions. Can be nil if an empty
-// URL is set.
-var updatesURL *url.URL
-
-const defaultUpdatesURL = `https://register.cockroachdb.com/api/clusters/updates`
-
-// reportingURL is the URL used to report diagnostics/telemetry. Can be nil if
-// an empty URL is set.
-var reportingURL *url.URL
-
-const defaultReportingURL = `https://register.cockroachdb.com/api/clusters/report`
-
-func init() {
-	var err error
-	updatesURL, err = url.Parse(
-		envutil.EnvOrDefaultString("COCKROACH_UPDATE_CHECK_URL", defaultUpdatesURL),
-	)
-	if err != nil {
-		panic(err)
-	}
-	reportingURL, err = url.Parse(
-		envutil.EnvOrDefaultString("COCKROACH_USAGE_REPORT_URL", defaultReportingURL),
-	)
-	if err != nil {
-		panic(err)
-	}
-}
-
-// TestingKnobs groups testing knobs for diagnostics.
-type TestingKnobs struct {
-	// OverrideUpdatesURL if set, overrides the URL used to check for new
-	// versions. It is a pointer to pointer to allow overriding to the nil URL.
-	OverrideUpdatesURL **url.URL
-
-	// OverrideReportingURL if set, overrides the URL used to report diagnostics.
-	// It is a pointer to pointer to allow overriding to the nil URL.
-	OverrideReportingURL **url.URL
-}
-
-// ClusterInfo contains cluster information that will become part of URLs.
-type ClusterInfo struct {
-	StorageClusterID uuid.UUID
-	LogicalClusterID uuid.UUID
-	TenantID         roachpb.TenantID
-	IsInsecure       bool
-	IsInternal       bool
-}
-
-// addInfoToURL sets query parameters on the URL used to report diagnostics. If
-// this is a CRDB node, then nodeInfo is filled (and nodeInfo.NodeID is
-// non-zero). Otherwise, this is a SQL-only tenant and sqlInfo is filled.
-func addInfoToURL(
-	url *url.URL,
-	clusterInfo *ClusterInfo,
-	env *diagnosticspb.Environment,
-	nodeID roachpb.NodeID,
-	sqlInfo *diagnosticspb.SQLInstanceInfo,
-) *url.URL {
-	if url == nil {
-		return nil
-	}
-	result := *url
-	q := result.Query()
-
-	// Don't set nodeid if this is a SQL-only instance.
-	if nodeID != 0 {
-		q.Set("nodeid", strconv.Itoa(int(nodeID)))
-	}
-
-	b := env.Build
-	q.Set("sqlid", strconv.Itoa(int(sqlInfo.SQLInstanceID)))
-	q.Set("uptime", strconv.Itoa(int(sqlInfo.Uptime)))
-	q.Set("licensetype", env.LicenseType)
-	q.Set("version", b.Tag)
-	q.Set("platform", b.Platform)
-	q.Set("uuid", clusterInfo.StorageClusterID.String())
-	q.Set("logical_uuid", clusterInfo.LogicalClusterID.String())
-	q.Set("tenantid", clusterInfo.TenantID.String())
-	q.Set("insecure", strconv.FormatBool(clusterInfo.IsInsecure))
-	q.Set("internal", strconv.FormatBool(clusterInfo.IsInternal))
-	q.Set("buildchannel", b.Channel)
-	q.Set("envchannel", b.EnvChannel)
-	result.RawQuery = q.Encode()
-	return &result
-}
-
-// randomly shift `d` to be up to `jitterSeconds` shorter or longer.
-func addJitter(d time.Duration) time.Duration {
-	const jitterSeconds = 120
-	j := time.Duration(rand.Intn(jitterSeconds*2)-jitterSeconds) * time.Second
-	return d + j
-}
-
 var populateMutex syncutil.Mutex
 
 // populateHardwareInfo populates OS, CPU, memory, etc. information about the