Skip to content

Commit

Permalink
Merge #44766
Browse files Browse the repository at this point in the history 
44766: sql: add telemetry for schema change usage commands r=rohany a=otan

Resolves #44316.

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

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
  • Loading branch information
@otan
craig[bot] and otan committed Feb 6, 2020
2 parents 3527c96 + 5e19b9a commit dbeb04d
Show file tree
Hide file tree
Showing 24 changed files with 215 additions and 11 deletions.
3 changes: 3 additions & 0 deletions pkg/sql/alter_index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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(),
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/alter_sequence.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/alter_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/alter_user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/create_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/create_index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/create_sequence.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/create_stats.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/create_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/create_user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/create_view.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/drop_database.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/drop_index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/drop_sequence.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/drop_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/drop_user.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/drop_view.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/alter_primary_key
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_table
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1058,3 +1058,4 @@ ALTER TABLE t43092 ALTER COLUMN x DROP NOT NULL

statement ok
DROP TABLE t43092

10 changes: 10 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/zone_config
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit dbeb04d

Please sign in to comment.