sql: add telemetry for schema change usage commands

This PR adds telemetry for usage of various CREATE, ALTER and DROP commands. Telemetry names may be subcategorized by different types, e.g.: ``` sql.schema.alter_table sql.schema.alter_table.add_check_constraint sql.schema.alter_table.add_column sql.schema.alter_table.add_constraint sql.schema.alter_table.column_mutation sql.schema.alter_table.drop_column sql.schema.alter_table.drop_constraint sql.schema.alter_table.set_audit sql.schema.alter_table.validate_constraint ``` Release note: None
cockroachdb · Feb 5, 2020 · 5e19b9a · 5e19b9a
1 parent 3e176a8
commit 5e19b9a
Show file tree

Hide file tree

Showing 24 changed files with 215 additions and 11 deletions.
diff --git a/pkg/sql/alter_index.go b/pkg/sql/alter_index.go
@@ -13,9 +13,11 @@ package sql
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/errors"
 	"github.com/gogo/protobuf/proto"
 )
@@ -59,6 +61,7 @@ func (n *alterIndexNode) startExec(params runParams) error {
 	for _, cmd := range n.n.Cmds {
 		switch t := cmd.(type) {
 		case *tree.AlterIndexPartitionBy:
+			telemetry.Inc(sqltelemetry.SchemaChangeAlterWithExtra("index", "partition_by"))
 			partitioning, err := CreatePartitioning(
 				params.ctx, params.extendedEvalCtx.Settings,
 				params.EvalContext(),

diff --git a/pkg/sql/alter_sequence.go b/pkg/sql/alter_sequence.go
@@ -13,9 +13,11 @@ package sql
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 )
 
 type alterSequenceNode struct {
@@ -48,6 +50,7 @@ func (p *planner) AlterSequence(ctx context.Context, n *tree.AlterSequence) (pla
 func (n *alterSequenceNode) ReadingOwnWrites() {}
 
 func (n *alterSequenceNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeAlter("sequence"))
 	desc := n.seqDesc
 
 	err := assignSequenceOptions(desc.SequenceOpts, n.n.Options, false /* setDefaults */, &params, desc.GetID())

diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
@@ -98,6 +98,8 @@ func (p *planner) AlterTable(ctx context.Context, n *tree.AlterTable) (planNode,
 func (n *alterTableNode) ReadingOwnWrites() {}
 
 func (n *alterTableNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeAlter("table"))
+
 	// Commands can either change the descriptor directly (for
 	// alterations that don't require a backfill) or add a mutation to
 	// the list.
@@ -107,6 +109,8 @@ func (n *alterTableNode) startExec(params runParams) error {
 	tn := params.p.ResolvedName(n.n.Table)
 
 	for i, cmd := range n.n.Cmds {
+		telemetry.Inc(cmd.TelemetryCounter())
+
 		switch t := cmd.(type) {
 		case *tree.AlterTableAddColumn:
 			d := t.ColumnDef
@@ -324,9 +328,6 @@ func (n *alterTableNode) startExec(params runParams) error {
 					"session variable experimental_enable_primary_key_changes is set to false, cannot perform primary key change")
 			}
 
-			// Increment telemetry about uses of primary key changes.
-			telemetry.Inc(sqltelemetry.AlterPrimaryKeyCounter)
-
 			// Ensure that there is not another primary key change attempted within this transaction.
 			currentMutationID := n.tableDesc.ClusterVersion.NextMutationID
 			for i := range n.tableDesc.Mutations {

diff --git a/pkg/sql/alter_user.go b/pkg/sql/alter_user.go
@@ -14,11 +14,13 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 )
 
 // alterUserSetPasswordNode represents an ALTER USER ... WITH PASSWORD statement.
@@ -61,6 +63,8 @@ type alterUserSetPasswordRun struct {
 }
 
 func (n *alterUserSetPasswordNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeAlter("user"))
+
 	normalizedUsername, hashedPassword, err := n.userAuthInfo.resolve()
 	if err != nil {
 		return err

diff --git a/pkg/sql/create_database.go b/pkg/sql/create_database.go
@@ -14,7 +14,9 @@ import (
 	"context"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 )
 
@@ -73,6 +75,7 @@ func (p *planner) CreateDatabase(ctx context.Context, n *tree.CreateDatabase) (p
 }
 
 func (n *createDatabaseNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("database"))
 	desc := makeDatabaseDesc(n.n)
 
 	created, err := params.p.createDatabase(params.ctx, &desc, n.n.IfNotExists)

diff --git a/pkg/sql/create_index.go b/pkg/sql/create_index.go
@@ -87,6 +87,7 @@ func MakeIndexDescriptor(n *tree.CreateIndex) (*sqlbase.IndexDescriptor, error)
 func (n *createIndexNode) ReadingOwnWrites() {}
 
 func (n *createIndexNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("index"))
 	_, dropped, err := n.tableDesc.FindIndexByName(string(n.n.Name))
 	if err == nil {
 		if dropped {

diff --git a/pkg/sql/create_sequence.go b/pkg/sql/create_sequence.go
@@ -15,9 +15,11 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/internal/client"
 	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -50,6 +52,7 @@ func (p *planner) CreateSequence(ctx context.Context, n *tree.CreateSequence) (p
 func (n *createSequenceNode) ReadingOwnWrites() {}
 
 func (n *createSequenceNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("sequence"))
 	// TODO(arul): Allow temporary sequences once temp tables work for regular tables.
 	if n.n.Temporary {
 		return unimplemented.NewWithIssuef(5807,

diff --git a/pkg/sql/create_stats.go b/pkg/sql/create_stats.go
@@ -72,6 +72,7 @@ type createStatsRun struct {
 }
 
 func (n *createStatsNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("stats"))
 	n.run.resultsCh = make(chan tree.Datums)
 	n.run.errCh = make(chan error)
 	go func() {

diff --git a/pkg/sql/create_table.go b/pkg/sql/create_table.go
@@ -154,6 +154,7 @@ func getTableCreateParams(
 }
 
 func (n *createTableNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("table"))
 	isTemporary := n.n.Temporary
 
 	tKey, schemaID, err := getTableCreateParams(params, n.dbDesc.ID, isTemporary, n.n.Table.Table())

diff --git a/pkg/sql/create_user.go b/pkg/sql/create_user.go
@@ -15,11 +15,13 @@ import (
 	"regexp"
 
 	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/errors"
 )
 
@@ -69,6 +71,8 @@ func (p *planner) CreateUserNode(
 }
 
 func (n *CreateUserNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("user"))
+
 	normalizedUsername, hashedPassword, err := n.userAuthInfo.resolve()
 	if err != nil {
 		return err

diff --git a/pkg/sql/create_view.go b/pkg/sql/create_view.go
@@ -43,8 +43,10 @@ type createViewNode struct {
 func (n *createViewNode) ReadingOwnWrites() {}
 
 func (n *createViewNode) startExec(params runParams) error {
-	isTemporary := n.temporary
+	telemetry.Inc(sqltelemetry.SchemaChangeCreate("view"))
+
 	viewName := string(n.viewName)
+	isTemporary := n.temporary
 	log.VEventf(params.ctx, 2, "dependencies for view %s:\n%s", viewName, n.planDeps.String())
 
 	// First check the backrefs and see if any of them are temporary.

diff --git a/pkg/sql/drop_database.go b/pkg/sql/drop_database.go
@@ -17,11 +17,13 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/internal/client"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 )
 
@@ -121,6 +123,8 @@ func (p *planner) DropDatabase(ctx context.Context, n *tree.DropDatabase) (planN
 }
 
 func (n *dropDatabaseNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeDrop("database"))
+
 	ctx := params.ctx
 	p := params.p
 	tbNameStrings := make([]string, 0, len(n.td))

diff --git a/pkg/sql/drop_index.go b/pkg/sql/drop_index.go
@@ -16,12 +16,14 @@ import (
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/errors"
 )
@@ -66,6 +68,8 @@ func (p *planner) DropIndex(ctx context.Context, n *tree.DropIndex) (planNode, e
 func (n *dropIndexNode) ReadingOwnWrites() {}
 
 func (n *dropIndexNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeDrop("index"))
+
 	ctx := params.ctx
 	for _, index := range n.idxNames {
 		// Need to retrieve the descriptor again for each index name in

diff --git a/pkg/sql/drop_sequence.go b/pkg/sql/drop_sequence.go
@@ -13,10 +13,12 @@ package sql
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 )
 
@@ -61,6 +63,8 @@ func (p *planner) DropSequence(ctx context.Context, n *tree.DropSequence) (planN
 func (n *dropSequenceNode) ReadingOwnWrites() {}
 
 func (n *dropSequenceNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeDrop("sequence"))
+
 	ctx := params.ctx
 	for _, toDel := range n.td {
 		droppedDesc := toDel.desc

diff --git a/pkg/sql/drop_table.go b/pkg/sql/drop_table.go
@@ -18,9 +18,11 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
@@ -102,6 +104,8 @@ func (p *planner) DropTable(ctx context.Context, n *tree.DropTable) (planNode, e
 func (n *dropTableNode) ReadingOwnWrites() {}
 
 func (n *dropTableNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeDrop("table"))
+
 	ctx := params.ctx
 	for _, toDel := range n.td {
 		droppedDesc := toDel.desc

diff --git a/pkg/sql/drop_user.go b/pkg/sql/drop_user.go
@@ -14,11 +14,13 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/pkg/errors"
 )
@@ -71,6 +73,8 @@ type dropUserRun struct {
 }
 
 func (n *DropUserNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeDrop("user"))
+
 	var entryType string
 	if n.isRole {
 		entryType = "role"

diff --git a/pkg/sql/drop_view.go b/pkg/sql/drop_view.go
@@ -14,9 +14,11 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/cockroachdb/cockroach/pkg/server/telemetry"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
 )
@@ -75,6 +77,8 @@ func (p *planner) DropView(ctx context.Context, n *tree.DropView) (planNode, err
 func (n *dropViewNode) ReadingOwnWrites() {}
 
 func (n *dropViewNode) startExec(params runParams) error {
+	telemetry.Inc(sqltelemetry.SchemaChangeDrop("view"))
+
 	ctx := params.ctx
 	for _, toDel := range n.td {
 		droppedDesc := toDel.desc

diff --git a/pkg/sql/logictest/testdata/logic_test/alter_primary_key b/pkg/sql/logictest/testdata/logic_test/alter_primary_key
@@ -37,9 +37,11 @@ SELECT * from t@primary
 9 10 11 12
 
 query T
-select feature_name FROM crdb_internal.feature_usage WHERE feature_name = 'sql.schema.alter_primary_key' AND usage_count > 0
+SELECT feature_name FROM crdb_internal.feature_usage
+WHERE feature_name IN ('sql.schema.alter_table.alter_primary_key') AND usage_count > 0
+ORDER BY feature_name
 ----
-sql.schema.alter_primary_key
+sql.schema.alter_table.alter_primary_key
 
 # Test primary key changes on storing indexes with different column families (the randomizer will do this for us).
 statement ok

diff --git a/pkg/sql/logictest/testdata/logic_test/alter_table b/pkg/sql/logictest/testdata/logic_test/alter_table
@@ -1058,3 +1058,4 @@ ALTER TABLE t43092 ALTER COLUMN x DROP NOT NULL
 
 statement ok
 DROP TABLE t43092
+
diff --git a/pkg/sql/logictest/testdata/logic_test/zone_config b/pkg/sql/logictest/testdata/logic_test/zone_config
@@ -131,3 +131,13 @@ query I
 SELECT zone_id FROM [SHOW ZONE CONFIGURATION FOR TABLE a]
 ----
 0
+
+subtest alter_table_telemetry
+
+query T
+SELECT feature_name FROM crdb_internal.feature_usage
+WHERE feature_name IN ('sql.schema.alter_range.configure_zone', 'sql.schema.alter_table.configure_zone')
+ORDER BY feature_name
+----
+sql.schema.alter_range.configure_zone
+sql.schema.alter_table.configure_zone