diff --git a/pkg/sql/instrumentation.go b/pkg/sql/instrumentation.go index c5f33bc2f0d2..3bf1e5b28e4c 100644 --- a/pkg/sql/instrumentation.go +++ b/pkg/sql/instrumentation.go @@ -471,7 +471,9 @@ func (ih *instrumentationHelper) Setup( ih.collectExecStats = collectTxnExecStats - if !collectTxnExecStats && !previouslySampled { + // Don't collect it if Stats Collection is disabled. If it is disabled the + // stats are not stored, so it always returns false for previouslySampled. + if !collectTxnExecStats && (!previouslySampled && sqlstats.StmtStatsEnable.Get(&cfg.Settings.SV)) { // We don't collect the execution stats for statements in this txn, but // this is the first time we see this statement ever, so we'll collect // its execution stats anyway (unless the user disabled txn stats diff --git a/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel b/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel index 995582da1621..781d83e2bd42 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel +++ b/pkg/sql/sqlstats/persistedsqlstats/BUILD.bazel @@ -88,6 +88,7 @@ go_test( "//pkg/security/securitytest", "//pkg/security/username", "//pkg/server", + "//pkg/settings/cluster", "//pkg/sql", "//pkg/sql/appstatspb", "//pkg/sql/catalog", diff --git a/pkg/sql/sqlstats/persistedsqlstats/controller_test.go b/pkg/sql/sqlstats/persistedsqlstats/controller_test.go index 7b765313b38e..c05f65e40c0a 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/controller_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/controller_test.go @@ -18,11 +18,13 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/base" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats/persistedsqlstats" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/errors" @@ -254,3 +256,41 @@ func TestActivityTablesReset(t *testing.T) { sqlDB.QueryRow(t, "SELECT count(*) FROM system.transaction_activity").Scan(&count) require.Equal(t, 0 /* expected */, count) } + +func TestStmtStatsEnable(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + // Start the cluster. (One node is sufficient; the outliers system is currently in-memory only.) + ctx := context.Background() + settings := cluster.MakeTestingClusterSettings() + args := base.TestClusterArgs{ServerArgs: base.TestServerArgs{Settings: settings}} + tc := testcluster.StartTestCluster(t, 1, args) + defer tc.Stopper().Stop(ctx) + + serverutils.SetClusterSetting(t, tc, "sql.metrics.statement_details.enabled", "false") + + sqlConn := sqlutils.MakeSQLRunner(tc.ServerConn(0)) + + sqlConn.Exec(t, "SELECT crdb_internal.reset_sql_stats()") + + appName := "TestStmtStatsEnable" + sqlConn.Exec(t, "SET application_name = $1", appName) + + sqlConn.Exec(t, "SELECT count_rows() FROM crdb_internal.statement_statistics") + sqlConn.Exec(t, "SELECT count_rows() FROM crdb_internal.transaction_statistics") + sqlConn.Exec(t, "SELECT count_rows() FROM crdb_internal.statement_statistics WHERE app_name = $1", appName) + sqlConn.Exec(t, "SELECT count_rows() FROM crdb_internal.transaction_statistics WHERE app_name = $1", appName) + + sqlConn.Exec(t, "SET application_name = $1", "ObserverTestStmtStatsEnable") + var count int + sqlConn.QueryRow(t, + "SELECT count(*) FROM crdb_internal.statement_statistics WHERE app_name = $1", appName). + Scan(&count) + require.Equal(t, 0 /* expected */, count) + + sqlConn.QueryRow(t, + "SELECT count(*) FROM crdb_internal.transaction_statistics WHERE app_name = $1 AND (statistics->'execution_statistics'->>'cnt')::int > 0 ", appName). + Scan(&count) + require.Less(t, count, 4, "statement execution stats collection is disabled there should be less than 4 rows. Actual: %d", count) +}