diff --git a/docs/generated/settings/settings-for-tenants.txt b/docs/generated/settings/settings-for-tenants.txt index a033546acc99..a17eb1abbe4c 100644 --- a/docs/generated/settings/settings-for-tenants.txt +++ b/docs/generated/settings/settings-for-tenants.txt @@ -150,7 +150,7 @@ sql.metrics.max_mem_stmt_fingerprints integer 100000 the maximum number of state sql.metrics.max_mem_txn_fingerprints integer 100000 the maximum number of transaction fingerprints stored in memory sql.metrics.statement_details.dump_to_logs boolean false dump collected statement statistics to node logs when periodically cleared sql.metrics.statement_details.enabled boolean true collect per-statement query statistics -sql.metrics.statement_details.plan_collection.enabled boolean true periodically save a logical plan for each fingerprint +sql.metrics.statement_details.plan_collection.enabled boolean false periodically save a logical plan for each fingerprint sql.metrics.statement_details.plan_collection.period duration 5m0s the time until a new logical plan is collected sql.metrics.statement_details.threshold duration 0s minimum execution time to cause statement statistics to be collected. If configured, no transaction stats are collected. sql.metrics.transaction_details.enabled boolean true collect per-application transaction statistics diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index d1d968623f48..678fac170bb7 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -166,7 +166,7 @@ sql.metrics.max_mem_txn_fingerprintsinteger100000the maximum number of transaction fingerprints stored in memory sql.metrics.statement_details.dump_to_logsbooleanfalsedump collected statement statistics to node logs when periodically cleared sql.metrics.statement_details.enabledbooleantruecollect per-statement query statistics -sql.metrics.statement_details.plan_collection.enabledbooleantrueperiodically save a logical plan for each fingerprint +sql.metrics.statement_details.plan_collection.enabledbooleanfalseperiodically save a logical plan for each fingerprint sql.metrics.statement_details.plan_collection.periodduration5m0sthe time until a new logical plan is collected sql.metrics.statement_details.thresholdduration0sminimum execution time to cause statement statistics to be collected. If configured, no transaction stats are collected. sql.metrics.transaction_details.enabledbooleantruecollect per-application transaction statistics diff --git a/pkg/ccl/telemetryccl/telemetry_logging_test.go b/pkg/ccl/telemetryccl/telemetry_logging_test.go index 9bc549d6292c..127a1fe47c69 100644 --- a/pkg/ccl/telemetryccl/telemetry_logging_test.go +++ b/pkg/ccl/telemetryccl/telemetry_logging_test.go @@ -53,6 +53,10 @@ func TestTelemetryLogRegions(t *testing.T) { sqlDB.Exec(t, `ALTER TABLE three_regions SPLIT AT SELECT generate_series(1, 3)`) sqlDB.Exec(t, "ALTER TABLE three_regions EXPERIMENTAL_RELOCATE VALUES (ARRAY[1], 1), (ARRAY[2], 2), (ARRAY[3], 3)") + // Enable the sampling of all statements so that execution statistics + // (including the regions information) is collected. + sqlDB.Exec(t, `SET CLUSTER SETTING sql.txn_stats.sample_rate = 1.0`) + // Enable the telemetry logging and increase the sampling frequency so that // all statements are captured. sqlDB.Exec(t, `SET CLUSTER SETTING sql.telemetry.query_sampling.enabled = true;`) diff --git a/pkg/server/status_test.go b/pkg/server/status_test.go index 8edc3ea6f19c..e6c2eed07359 100644 --- a/pkg/server/status_test.go +++ b/pkg/server/status_test.go @@ -1945,10 +1945,6 @@ func TestStatusAPIStatements(t *testing.T) { // Ignore the ALTER USER ... VIEWACTIVITY statement. continue } - if len(respStatement.Stats.SensitiveInfo.MostRecentPlanDescription.Name) == 0 { - // Ensure that we populate the explain plan. - t.Fatal("expected MostRecentPlanDescription to be populated") - } statementsInResponse = append(statementsInResponse, respStatement.Key.KeyData.Query) } @@ -2061,11 +2057,6 @@ func TestStatusAPICombinedStatements(t *testing.T) { continue } - if len(respStatement.Stats.SensitiveInfo.MostRecentPlanDescription.Name) == 0 { - // Ensure that we populate the explain plan. - t.Fatal("expected MostRecentPlanDescription to be populated") - } - statementsInResponse = append(statementsInResponse, respStatement.Key.KeyData.Query) } diff --git a/pkg/sql/instrumentation.go b/pkg/sql/instrumentation.go index 901f28a03435..64a60cde88ea 100644 --- a/pkg/sql/instrumentation.go +++ b/pkg/sql/instrumentation.go @@ -220,11 +220,13 @@ func (ih *instrumentationHelper) Setup( ih.savePlanForStats = statsCollector.ShouldSaveLogicalPlanDesc(fingerprint, implicitTxn, p.SessionData().Database) - if ih.ShouldBuildExplainPlan() { - // Populate traceMetadata early in case we short-circuit the execution - // before reaching the bottom of this method. - ih.traceMetadata = make(execNodeTraceMetadata) - } + defer func() { + if ih.ShouldBuildExplainPlan() { + // Populate traceMetadata at the end once we have all properties of + // the helper setup. + ih.traceMetadata = make(execNodeTraceMetadata) + } + }() if sp := tracing.SpanFromContext(ctx); sp != nil { if sp.IsVerbose() { @@ -270,9 +272,6 @@ func (ih *instrumentationHelper) Setup( } ih.collectExecStats = true - if ih.traceMetadata == nil { - ih.traceMetadata = make(execNodeTraceMetadata) - } ih.evalCtx = p.EvalContext() newCtx, ih.sp = tracing.EnsureChildSpan(ctx, cfg.AmbientCtx.Tracer, "traced statement", tracing.WithRecording(tracing.RecordingVerbose)) ih.shouldFinishSpan = true @@ -424,7 +423,8 @@ func (ih *instrumentationHelper) ShouldUseJobForCreateStats() bool { // ShouldBuildExplainPlan returns true if we should build an explain plan and // call RecordExplainPlan. func (ih *instrumentationHelper) ShouldBuildExplainPlan() bool { - return ih.collectBundle || ih.savePlanForStats || ih.outputMode == explainAnalyzePlanOutput || + return ih.collectBundle || ih.collectExecStats || ih.savePlanForStats || + ih.outputMode == explainAnalyzePlanOutput || ih.outputMode == explainAnalyzeDistSQLOutput } diff --git a/pkg/sql/sqlstats/cluster_settings.go b/pkg/sql/sqlstats/cluster_settings.go index c254a52e02ae..e87ac5fe20bf 100644 --- a/pkg/sql/sqlstats/cluster_settings.go +++ b/pkg/sql/sqlstats/cluster_settings.go @@ -65,7 +65,7 @@ var SampleLogicalPlans = settings.RegisterBoolSetting( settings.TenantWritable, "sql.metrics.statement_details.plan_collection.enabled", "periodically save a logical plan for each fingerprint", - true, + false, ).WithPublic() // LogicalPlanCollectionPeriod specifies the interval between collections of diff --git a/pkg/sql/sqlstats/persistedsqlstats/datadriven_test.go b/pkg/sql/sqlstats/persistedsqlstats/datadriven_test.go index ed8d83fa8007..5618e5631b11 100644 --- a/pkg/sql/sqlstats/persistedsqlstats/datadriven_test.go +++ b/pkg/sql/sqlstats/persistedsqlstats/datadriven_test.go @@ -93,6 +93,10 @@ func TestSQLStatsDataDriven(t *testing.T) { observerConn := cluster.ServerConn(1 /* idx */) observer := sqlutils.MakeSQLRunner(observerConn) + _, err := sqlConn.Exec(`SET CLUSTER SETTING sql.metrics.statement_details.plan_collection.enabled = true;`) + if err != nil { + t.Errorf("failed to enable plan collection due to %s", err.Error()) + } execDataDrivenTestCmd := func(t *testing.T, d *datadriven.TestData) string { switch d.Cmd { diff --git a/pkg/util/log/logcrash/crash_reporting_packet_test.go b/pkg/util/log/logcrash/crash_reporting_packet_test.go index 26441ca9130e..361a486ba3e5 100644 --- a/pkg/util/log/logcrash/crash_reporting_packet_test.go +++ b/pkg/util/log/logcrash/crash_reporting_packet_test.go @@ -214,6 +214,10 @@ func TestInternalErrorReporting(t *testing.T) { s, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{}) defer s.Stopper().Stop(ctx) + _, err := sqlDB.Exec(`SET CLUSTER SETTING sql.metrics.statement_details.plan_collection.enabled = true;`) + if err != nil { + t.Errorf("failed to enable plan collection due to %s", err.Error()) + } if _, err := sqlDB.Exec("SELECT crdb_internal.force_assertion_error('woo')"); !testutils.IsError(err, "internal error") { t.Fatalf("expected internal error, got %v", err)