Merge #80895

80895: roleoption: remove sqltelemetry dependency r=ajwerner a=otan For GetStmts, instead use a function hook to inject any necessary telemetry calls. This is one more step towards removing `util/log` dependencies from the `tree` package. Release note: None Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
cockroachdb · May 3, 2022 · d63a369 · d63a369
2 parents 01572da + d5170bb
commit d63a369
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 9 deletions.
diff --git a/pkg/sql/alter_role.go b/pkg/sql/alter_role.go
@@ -215,7 +215,9 @@ func (n *alterRoleNode) startExec(params runParams) error {
 	}
 
 	// Get a map of statements to execute for role options and their values.
-	stmts, err := n.roleOptions.GetSQLStmts(sqltelemetry.AlterRole)
+	stmts, err := n.roleOptions.GetSQLStmts(func(o roleoption.Option) {
+		sqltelemetry.IncIAMOptionCounter(sqltelemetry.AlterRole, strings.ToLower(o.String()))
+	})
 	if err != nil {
 		return err
 	}

diff --git a/pkg/sql/create_role.go b/pkg/sql/create_role.go
@@ -13,6 +13,7 @@ package sql
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/security"
 	"github.com/cockroachdb/cockroach/pkg/security/password"
@@ -168,7 +169,9 @@ func (n *CreateRoleNode) startExec(params runParams) error {
 	}
 
 	// Get a map of statements to execute for role options and their values.
-	stmts, err := n.roleOptions.GetSQLStmts(sqltelemetry.CreateRole)
+	stmts, err := n.roleOptions.GetSQLStmts(func(o roleoption.Option) {
+		sqltelemetry.IncIAMOptionCounter(sqltelemetry.CreateRole, strings.ToLower(o.String()))
+	})
 	if err != nil {
 		return err
 	}

diff --git a/pkg/sql/roleoption/BUILD.bazel b/pkg/sql/roleoption/BUILD.bazel
@@ -12,7 +12,6 @@ go_library(
     deps = [
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
-        "//pkg/sql/sqltelemetry",
         "@com_github_cockroachdb_errors//:errors",
     ],
 )

diff --git a/pkg/sql/roleoption/role_option.go b/pkg/sql/roleoption/role_option.go
@@ -15,7 +15,6 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
-	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/errors"
 )
 
@@ -148,7 +147,9 @@ type List []RoleOption
 
 // GetSQLStmts returns a map of SQL stmts to apply each role option.
 // Maps stmts to values (value of the role option).
-func (rol List) GetSQLStmts(op string) (map[string]func() (bool, string, error), error) {
+func (rol List) GetSQLStmts(
+	onRoleOption func(Option),
+) (map[string]func() (bool, string, error), error) {
 	if len(rol) <= 0 {
 		return nil, nil
 	}
@@ -161,10 +162,9 @@ func (rol List) GetSQLStmts(op string) (map[string]func() (bool, string, error),
 	}
 
 	for _, ro := range rol {
-		sqltelemetry.IncIAMOptionCounter(
-			op,
-			strings.ToLower(ro.Option.String()),
-		)
+		if onRoleOption != nil {
+			onRoleOption(ro.Option)
+		}
 		// Skip PASSWORD and DEFAULTSETTINGS options.
 		// Since PASSWORD still resides in system.users, we handle setting PASSWORD
 		// outside of this set stmt.