Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

telemetry: set switch default to false (#41336) #41384

Merged
merged 1 commit into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ var defaultConf = Config{
},
Experimental: Experimental{},
EnableCollectExecutionInfo: true,
EnableTelemetry: true,
EnableTelemetry: false,
Labels: make(map[string]string),
EnableGlobalIndex: false,
Security: Security{
Expand Down
2 changes: 1 addition & 1 deletion config/config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ skip-register-to-dashboard = false
# When enabled, usage data (for example, instance versions) will be reported to PingCAP periodically for user experience analytics.
# If this config is set to `false` on all TiDB servers, telemetry will be always disabled regardless of the value of the global variable `tidb_enable_telemetry`.
# See PingCAP privacy policy for details: https://pingcap.com/en/privacy-policy/
enable-telemetry = true
enable-telemetry = false

# deprecate-integer-display-length is used to be compatible with MySQL 8.0 in which the integer declared with display length will be returned with
# a warning like `Integer display width is deprecated and will be removed in a future release`.
Expand Down
8 changes: 4 additions & 4 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,23 +858,23 @@ history-size=100`)
require.NoError(t, err)
require.NoError(t, f.Sync())
require.NoError(t, conf.Load(configFile))
require.True(t, conf.EnableTelemetry)
require.False(t, conf.EnableTelemetry)

_, err = f.WriteString(`
enable-table-lock = true
`)
require.NoError(t, err)
require.NoError(t, f.Sync())
require.NoError(t, conf.Load(configFile))
require.True(t, conf.EnableTelemetry)
require.False(t, conf.EnableTelemetry)

_, err = f.WriteString(`
enable-telemetry = false
enable-telemetry = true
`)
require.NoError(t, err)
require.NoError(t, f.Sync())
require.NoError(t, conf.Load(configFile))
require.False(t, conf.EnableTelemetry)
require.True(t, conf.EnableTelemetry)

_, err = f.WriteString(`
[security]
Expand Down
2 changes: 1 addition & 1 deletion privilege/privileges/privileges_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2045,7 +2045,7 @@ func TestSecurityEnhancedModeSysVars(t *testing.T) {
tk.MustQuery(`SHOW VARIABLES LIKE 'tidb_force_priority'`).Check(testkit.Rows("tidb_force_priority NO_PRIORITY"))
tk.MustQuery(`SELECT COUNT(*) FROM information_schema.variables_info WHERE variable_name = 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows("1"))
tk.MustQuery(`SELECT COUNT(*) FROM performance_schema.session_variables WHERE variable_name = 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows("1"))
tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_enable_telemetry'`).Check(testkit.Rows("tidb_enable_telemetry ON"))
tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_enable_telemetry'`).Check(testkit.Rows("tidb_enable_telemetry OFF"))
tk.MustQuery(`SELECT COUNT(*) FROM information_schema.variables_info WHERE variable_name = 'tidb_enable_telemetry'`).Check(testkit.Rows("1"))
tk.MustQuery(`SELECT COUNT(*) FROM performance_schema.session_variables WHERE variable_name = 'tidb_enable_telemetry'`).Check(testkit.Rows("1"))

Expand Down
2 changes: 1 addition & 1 deletion sessionctx/variable/tidb_vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ const (
DefTiDBRestrictedReadOnly = false
DefTiDBSuperReadOnly = false
DefTiDBShardAllocateStep = math.MaxInt64
DefTiDBEnableTelemetry = true
DefTiDBEnableTelemetry = false
DefTiDBEnableParallelApply = false
DefTiDBPartitionPruneMode = "dynamic"
DefTiDBEnableRateLimitAction = false
Expand Down
4 changes: 0 additions & 4 deletions telemetry/cte_test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,11 @@ go_test(
flaky = True,
race = "on",
deps = [
"//config",
"//domain",
"//kv",
"//session",
"//store/mockstore",
"//telemetry",
"//testkit",
"//testkit/testsetup",
"@com_github_jeffail_gabs_v2//:gabs",
"@com_github_stretchr_testify//require",
"@io_etcd_go_etcd_tests_v3//integration",
"@io_opencensus_go//stats/view",
Expand Down
62 changes: 33 additions & 29 deletions telemetry/cte_test/cte_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@ import (
"runtime"
"testing"

"github.com/Jeffail/gabs/v2"
"github.com/pingcap/tidb/config"
"github.com/pingcap/tidb/domain"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/store/mockstore"
"github.com/pingcap/tidb/telemetry"
"github.com/pingcap/tidb/testkit"
"github.com/pingcap/tidb/testkit/testsetup"
"github.com/stretchr/testify/require"
"go.etcd.io/etcd/tests/v3/integration"
Expand Down Expand Up @@ -55,35 +51,43 @@ func TestCTEPreviewAndReport(t *testing.T) {
s := newSuite(t)
defer s.close()

config.GetGlobalConfig().EnableTelemetry = true

tk := testkit.NewTestKit(t, s.store)
tk.MustExec("use test")
tk.MustExec("with cte as (select 1) select * from cte")
tk.MustExec("with recursive cte as (select 1) select * from cte")
tk.MustExec("with recursive cte(n) as (select 1 union select * from cte where n < 5) select * from cte")
tk.MustExec("select 1")

res, err := telemetry.PreviewUsageData(s.se, s.etcdCluster.RandClient())
require.NoError(t, err)
// By disableing telemetry by default, the global sysvar **and** config file defaults
// are all set to false, so that enabling telemetry in test become more complex.
// As telemetry is a feature that almost no user will manually enable, I'd remove these
// tests for now.
// They should be uncommented once the default behavious changed back to enabled in the
// future, otherwise they could just be deleted.
/*
config.GetGlobalConfig().EnableTelemetry = true

tk := testkit.NewTestKit(t, s.store)
tk.MustExec("use test")
tk.MustExec("with cte as (select 1) select * from cte")
tk.MustExec("with recursive cte as (select 1) select * from cte")
tk.MustExec("with recursive cte(n) as (select 1 union select * from cte where n < 5) select * from cte")
tk.MustExec("select 1")

res, err := telemetry.PreviewUsageData(s.se, s.etcdCluster.RandClient())
require.NoError(t, err)

jsonParsed, err := gabs.ParseJSON([]byte(res))
require.NoError(t, err)
require.Equal(t, 1, int(jsonParsed.Path("featureUsage.cte.nonRecursiveCTEUsed").Data().(float64)))
require.Equal(t, 1, int(jsonParsed.Path("featureUsage.cte.recursiveUsed").Data().(float64)))
require.Equal(t, 4, int(jsonParsed.Path("featureUsage.cte.nonCTEUsed").Data().(float64)))
jsonParsed, err := gabs.ParseJSON([]byte(res))
require.NoError(t, err)
require.Equal(t, 1, int(jsonParsed.Path("featureUsage.cte.nonRecursiveCTEUsed").Data().(float64)))
require.Equal(t, 1, int(jsonParsed.Path("featureUsage.cte.recursiveUsed").Data().(float64)))
require.Equal(t, 4, int(jsonParsed.Path("featureUsage.cte.nonCTEUsed").Data().(float64)))

err = telemetry.ReportUsageData(s.se, s.etcdCluster.RandClient())
require.NoError(t, err)
err = telemetry.ReportUsageData(s.se, s.etcdCluster.RandClient())
require.NoError(t, err)

res, err = telemetry.PreviewUsageData(s.se, s.etcdCluster.RandClient())
require.NoError(t, err)
res, err = telemetry.PreviewUsageData(s.se, s.etcdCluster.RandClient())
require.NoError(t, err)

jsonParsed, err = gabs.ParseJSON([]byte(res))
require.NoError(t, err)
require.Equal(t, 0, int(jsonParsed.Path("featureUsage.cte.nonRecursiveCTEUsed").Data().(float64)))
require.Equal(t, 0, int(jsonParsed.Path("featureUsage.cte.recursiveUsed").Data().(float64)))
require.Equal(t, 0, int(jsonParsed.Path("featureUsage.cte.nonCTEUsed").Data().(float64)))
jsonParsed, err = gabs.ParseJSON([]byte(res))
require.NoError(t, err)
require.Equal(t, 0, int(jsonParsed.Path("featureUsage.cte.nonRecursiveCTEUsed").Data().(float64)))
require.Equal(t, 0, int(jsonParsed.Path("featureUsage.cte.recursiveUsed").Data().(float64)))
require.Equal(t, 0, int(jsonParsed.Path("featureUsage.cte.nonCTEUsed").Data().(float64)))
*/
}

type testSuite struct {
Expand Down
49 changes: 30 additions & 19 deletions telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,36 @@ func TestPreview(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "", r)

trackingID, err := telemetry.ResetTrackingID(etcdCluster.RandClient())
require.NoError(t, err)

config.GetGlobalConfig().EnableTelemetry = true
r, err = telemetry.PreviewUsageData(se, etcdCluster.RandClient())
require.NoError(t, err)

jsonParsed, err := gabs.ParseJSON([]byte(r))
require.NoError(t, err)
require.Equal(t, trackingID, jsonParsed.Path("trackingId").Data().(string))
// Apple M1 doesn't contain cpuFlags
if !(runtime.GOARCH == "arm64" && runtime.GOOS == "darwin") {
require.True(t, jsonParsed.ExistsP("hostExtra.cpuFlags"))
}
require.True(t, jsonParsed.ExistsP("hostExtra.os"))
require.Len(t, jsonParsed.Path("instances").Children(), 2)
require.Equal(t, "tidb", jsonParsed.Path("instances.0.instanceType").Data().(string))
require.Equal(t, "tikv", jsonParsed.Path("instances.1.instanceType").Data().(string))
require.True(t, jsonParsed.ExistsP("hardware"))
// By disableing telemetry by default, the global sysvar **and** config file defaults
// are all set to false, so that enabling telemetry in test become more complex.
// As telemetry is a feature that almost no user will manually enable, I'd remove these
// tests for now.
// They should be uncommented once the default behavious changed back to enabled in the
// future, otherwise they could just be deleted.
/*
trackingID, err := telemetry.ResetTrackingID(etcdCluster.RandClient())
require.NoError(t, err)

config.GetGlobalConfig().EnableTelemetry = true
telemetryEnabled, err := telemetry.IsTelemetryEnabled(se)
require.NoError(t, err)
require.True(t, telemetryEnabled)
r, err = telemetry.PreviewUsageData(se, etcdCluster.RandClient())
require.NoError(t, err)

jsonParsed, err := gabs.ParseJSON([]byte(r))
require.NoError(t, err)
require.Equal(t, trackingID, jsonParsed.Path("trackingId").Data().(string))
// Apple M1 doesn't contain cpuFlags
if !(runtime.GOARCH == "arm64" && runtime.GOOS == "darwin") {
require.True(t, jsonParsed.ExistsP("hostExtra.cpuFlags"))
}
require.True(t, jsonParsed.ExistsP("hostExtra.os"))
require.Len(t, jsonParsed.Path("instances").Children(), 2)
require.Equal(t, "tidb", jsonParsed.Path("instances.0.instanceType").Data().(string))
require.Equal(t, "tikv", jsonParsed.Path("instances.1.instanceType").Data().(string))
require.True(t, jsonParsed.ExistsP("hardware"))
*/

_, err = se.Execute(context.Background(), "SET @@global.tidb_enable_telemetry = 0")
require.NoError(t, err)
Expand Down