Merge #80771

80771: tree: move telemetry references to sql package r=ajwerner a=otan Individual commits should be fairly easy to review by themselves! There are still indirect references as of writing, but this removes the direct ones! Co-authored-by: Oliver Tan <[email protected]>
cockroachdb · May 17, 2022 · dc79c28 · dc79c28
2 parents 2d53147 + c2138b7
commit dc79c28
Show file tree

Hide file tree

Showing 23 changed files with 288 additions and 304 deletions.
diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel
@@ -216,6 +216,7 @@ go_library(
         "tablewriter_insert.go",
         "tablewriter_update.go",
         "tablewriter_upsert_opt.go",
+        "telemetry.go",
         "telemetry_logging.go",
         "temporary_schema.go",
         "tenant.go",

diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go
@@ -98,7 +98,9 @@ func (p *planner) AlterTable(ctx context.Context, n *tree.AlterTable) (planNode,
 			tree.Name(tableDesc.GetName()), tree.Name(tableDesc.GetName()))
 	}
 
-	n.HoistAddColumnConstraints()
+	n.HoistAddColumnConstraints(func() {
+		telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra("table", "add_column.references"))
+	})
 
 	// See if there's any "inject statistics" in the query and type check the
 	// expressions.
@@ -165,7 +167,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 	}
 
 	for i, cmd := range n.n.Cmds {
-		telemetry.Inc(cmd.TelemetryCounter())
+		telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra("table", cmd.TelemetryName()))
 
 		if !n.tableDesc.HasPrimaryKey() && !isAlterCmdValidWithoutPrimaryKey(cmd) {
 			return errors.Newf("table %q does not have a primary key, cannot perform%s", n.tableDesc.Name, tree.AsString(cmd))

diff --git a/pkg/sql/alter_table_owner.go b/pkg/sql/alter_table_owner.go
@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/decodeusername"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
@@ -79,7 +80,10 @@ func (p *planner) AlterTableOwner(ctx context.Context, n *tree.AlterTableOwner)
 }
 
 func (n *alterTableOwnerNode) startExec(params runParams) error {
-	telemetry.Inc(n.n.TelemetryCounter())
+	telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra(
+		tree.GetTableType(n.n.IsSequence, n.n.IsView, n.n.IsMaterialized),
+		n.n.TelemetryName(),
+	))
 	ctx := params.ctx
 	p := params.p
 	tableDesc := n.desc

diff --git a/pkg/sql/alter_table_set_schema.go b/pkg/sql/alter_table_set_schema.go
@@ -21,6 +21,7 @@ import (
 	"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"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
@@ -96,7 +97,10 @@ func (p *planner) AlterTableSetSchema(
 }
 
 func (n *alterTableSetSchemaNode) startExec(params runParams) error {
-	telemetry.Inc(n.n.TelemetryCounter())
+	telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra(
+		tree.GetTableType(n.n.IsSequence, n.n.IsView, n.n.IsMaterialized),
+		n.n.TelemetryName(),
+	))
 	ctx := params.ctx
 	p := params.p
 	tableDesc := n.tableDesc

diff --git a/pkg/sql/alter_type.go b/pkg/sql/alter_type.go
@@ -93,7 +93,7 @@ func (p *planner) AlterType(ctx context.Context, n *tree.AlterType) (planNode, e
 }
 
 func (n *alterTypeNode) startExec(params runParams) error {
-	telemetry.Inc(n.n.Cmd.TelemetryCounter())
+	telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra("type", n.n.Cmd.TelemetryName()))
 
 	typeName := tree.AsStringWithFQNames(n.n.Type, params.p.Ann())
 	eventLogDone := false

diff --git a/pkg/sql/drop_view.go b/pkg/sql/drop_view.go
@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlerrors"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 	"github.com/cockroachdb/errors"
@@ -90,7 +91,9 @@ 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(n.n.TelemetryCounter())
+	telemetry.Inc(sqltelemetry.SchemaChangeDropCounter(
+		tree.GetTableType(false /* isSequence */, true /* isView */, n.n.IsMaterialized),
+	))
 
 	ctx := params.ctx
 	for _, toDel := range n.td {

diff --git a/pkg/sql/refresh_materialized_view.go b/pkg/sql/refresh_materialized_view.go
@@ -20,6 +20,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 )
 
 type refreshMaterializedViewNode struct {
@@ -79,7 +80,7 @@ func (n *refreshMaterializedViewNode) startExec(params runParams) error {
 	// results of the view query into the new set of indexes, and then change the
 	// set of indexes over to the new set of indexes atomically.
 
-	telemetry.Inc(n.n.TelemetryCounter())
+	telemetry.Inc(sqltelemetry.SchemaRefreshMaterializedView)
 
 	// Inform the user that CONCURRENTLY is not needed.
 	if n.n.Concurrently {

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/BUILD.bazel
@@ -20,6 +20,7 @@ go_library(
     importpath = "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scbuild/internal/scbuildstmt",
     visibility = ["//pkg/sql/schemachanger/scbuild:__subpackages__"],
     deps = [
+        "//pkg/server/telemetry",
         "//pkg/settings/cluster",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/catpb",

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go
@@ -13,12 +13,14 @@ package scbuildstmt
 import (
 	"reflect"
 
+	"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/schemachanger/scerrors"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/errors"
 )
 
@@ -52,7 +54,9 @@ func init() {
 func AlterTable(b BuildCtx, n *tree.AlterTable) {
 	// Hoist the constraints to separate clauses because other code assumes that
 	// that is how the commands will look.
-	n.HoistAddColumnConstraints()
+	n.HoistAddColumnConstraints(func() {
+		telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra("table", "add_column.references"))
+	})
 	// Check if an entry exists for the statement type, in which
 	// case. It's either fully or partially supported. Check the commands
 	// first, since we don't want to do extra work in this transaction

diff --git a/pkg/sql/sem/builtins/all_builtins.go b/pkg/sql/sem/builtins/all_builtins.go
@@ -14,7 +14,9 @@ import (
 	"fmt"
 	"sort"
 
+	"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/sql/types"
 )
 
@@ -59,7 +61,7 @@ func init() {
 		} else if def.props.Class == tree.WindowClass {
 			AllWindowBuiltinNames = append(AllWindowBuiltinNames, name)
 		}
-		for _, overload := range def.overloads {
+		for i, overload := range def.overloads {
 			fnCount := 0
 			if overload.Fn != nil {
 				fnCount++
@@ -84,6 +86,10 @@ func init() {
 					name, fnCount,
 				))
 			}
+			c := sqltelemetry.BuiltinCounter(name, overload.Signature(false))
+			def.overloads[i].OnTypeCheck = func() {
+				telemetry.Inc(c)
+			}
 		}
 	}
 

diff --git a/pkg/sql/sem/tree/BUILD.bazel b/pkg/sql/sem/tree/BUILD.bazel
@@ -61,7 +61,6 @@ go_library(
         "name_part.go",
         "name_resolution.go",
         "object_name.go",
-        "operators.go",
         "overload.go",
         "parse_array.go",
         "parse_string.go",  # keep
@@ -119,7 +118,6 @@ go_library(
     deps = [
         "//pkg/geo",
         "//pkg/geo/geopb",
-        "//pkg/server/telemetry",
         "//pkg/sql/lex",
         "//pkg/sql/lexbase",
         "//pkg/sql/pgwire/pgcode",
@@ -134,7 +132,6 @@ go_library(
         "//pkg/sql/sem/tree/treewindow",
         "//pkg/sql/sem/volatility",
         "//pkg/sql/sessiondatapb",
-        "//pkg/sql/sqltelemetry",
         "//pkg/sql/types",
         "//pkg/util",
         "//pkg/util/bitarray",