sql: make multi-tenancy errors report to telemetry

pkg/base/node_id.go

@@ -124,8 +124,8 @@ func (c *SQLIDContainer) OptionalNodeID() (roachpb.NodeID, bool) {
 
 // OptionalNodeIDErr is like OptionalNodeID, but returns an error (referring to
 // the optionally supplied Github issues) if the ID is not present.
-func (c *SQLIDContainer) OptionalNodeIDErr(issueNos ...int) (roachpb.NodeID, error) {
- v, err := c.w.OptionalErr(issueNos...)
+func (c *SQLIDContainer) OptionalNodeIDErr(issue int) (roachpb.NodeID, error) {
+ v, err := c.w.OptionalErr(issue)
  if err != nil {
  return 0, err
  }

pkg/gossip/gossip.go

@@ -1646,8 +1646,8 @@ type OptionalGossip struct {
 //
 // Use of Gossip from within the SQL layer is **deprecated**. Please do not
 // introduce new uses of it.
-func (og OptionalGossip) OptionalErr(issueNos ...int) (*Gossip, error) {
- v, err := og.w.OptionalErr(issueNos...)
+func (og OptionalGossip) OptionalErr(issue int) (*Gossip, error) {
+ v, err := og.w.OptionalErr(issue)
  if err != nil {
  return nil, err
  }
@@ -1660,7 +1660,7 @@ func (og OptionalGossip) OptionalErr(issueNos ...int) (*Gossip, error) {
 //
 // Use of Gossip from within the SQL layer is **deprecated**. Please do not
 // introduce new uses of it.
-func (og OptionalGossip) Optional(issueNos ...int) (*Gossip, bool) {
+func (og OptionalGossip) Optional(issue int) (*Gossip, bool) {
  v, ok := og.w.Optional()
  if !ok {
  return nil, false

pkg/jobs/deprecated.go

@@ -86,7 +86,7 @@ WHERE status IN ($1, $2, $3, $4, $5) ORDER BY created DESC`
  // If no liveness is available, adopt all jobs. This is reasonable because this
  // only affects SQL tenants, which have at most one SQL server running on their
  // behalf at any given time.
- if nl, ok := nlw.Optional(47892); ok {
+ if nl, ok := nlw.Optional(54251); ok {
  // We subtract the leniency interval here to artificially
  // widen the range of times over which the job registry will
  // consider the node to be alive. We rely on the fact that

pkg/jobs/registry.go

@@ -775,7 +775,7 @@ func (r *Registry) maybeCancelJobs(ctx context.Context, s sqlliveness.Session) {
 func (r *Registry) maybeCancelJobsDeprecated(
  ctx context.Context, nlw optionalnodeliveness.Container,
 ) {
- nl, ok := nlw.Optional(47892)
+ nl, ok := nlw.Optional(54251)
  if !ok {
  // At most one container is running on behalf of a SQL tenant, so it must be
  // this one, and there's no point canceling anything.

pkg/server/serverpb/status.go

@@ -53,9 +53,9 @@ type NodesStatusServer interface {
 // available. If it is not, an error referring to the optionally supplied issues
 // is returned.
 func (s *OptionalNodesStatusServer) OptionalNodesStatusServer(
- issueNos ...int,
+ issue int,
 ) (NodesStatusServer, error) {
- v, err := s.w.OptionalErr(issueNos...)
+ v, err := s.w.OptionalErr(issue)
  if err != nil {
  return nil, err
  }

pkg/sql/crdb_internal.go

@@ -53,6 +53,7 @@ import (
  "github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
  "github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
  "github.com/cockroachdb/cockroach/pkg/sql/types"
+ "github.com/cockroachdb/cockroach/pkg/util/errorutil"
  "github.com/cockroachdb/cockroach/pkg/util/hlc"
  "github.com/cockroachdb/cockroach/pkg/util/json"
  "github.com/cockroachdb/cockroach/pkg/util/log"
@@ -3397,7 +3398,8 @@ CREATE TABLE crdb_internal.kv_node_status (
  if err := p.RequireAdminRole(ctx, "read crdb_internal.kv_node_status"); err != nil {
  return err
  }
- ss, err := p.extendedEvalCtx.NodesStatusServer.OptionalNodesStatusServer()
+ ss, err := p.extendedEvalCtx.NodesStatusServer.OptionalNodesStatusServer(
+ errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
  if err != nil {
  return err
  }
@@ -3511,7 +3513,8 @@ CREATE TABLE crdb_internal.kv_store_status (
  if err := p.RequireAdminRole(ctx, "read crdb_internal.kv_store_status"); err != nil {
  return err
  }
- ss, err := p.ExecCfg().NodesStatusServer.OptionalNodesStatusServer()
+ ss, err := p.ExecCfg().NodesStatusServer.OptionalNodesStatusServer(
+ errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
  if err != nil {
  return err
  }

pkg/sql/opt_exec_factory.go

@@ -1692,7 +1692,7 @@ func (ef *execFactory) ConstructAlterTableSplit(
  index cat.Index, input exec.Node, expiration tree.TypedExpr,
 ) (exec.Node, error) {
  if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(54254)
  }
 
  expirationTime, err := parseExpirationTime(ef.planner.EvalContext(), expiration)
@@ -1713,7 +1713,7 @@ func (ef *execFactory) ConstructAlterTableUnsplit(
  index cat.Index, input exec.Node,
 ) (exec.Node, error) {
  if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(54254)
  }
 
  return &unsplitNode{
@@ -1726,7 +1726,7 @@ func (ef *execFactory) ConstructAlterTableUnsplit(
 // ConstructAlterTableUnsplitAll is part of the exec.Factory interface.
 func (ef *execFactory) ConstructAlterTableUnsplitAll(index cat.Index) (exec.Node, error) {
  if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(54254)
  }
 
  return &unsplitAllNode{
@@ -1740,7 +1740,7 @@ func (ef *execFactory) ConstructAlterTableRelocate(
  index cat.Index, input exec.Node, relocateLease bool,
 ) (exec.Node, error) {
  if !ef.planner.ExecCfg().Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(54250)
  }
 
  return &relocateNode{

pkg/sql/optionalnodeliveness/node_liveness.go

@@ -47,8 +47,8 @@ func MakeContainer(nl Interface) Container {
 //
 // Use of NodeLiveness from within the SQL layer is **deprecated**. Please do
 // not introduce new uses of it.
-func (nl *Container) OptionalErr(issueNos ...int) (Interface, error) {
- v, err := nl.w.OptionalErr(issueNos...)
+func (nl *Container) OptionalErr(issue int) (Interface, error) {
+ v, err := nl.w.OptionalErr(issue)
  if err != nil {
  return nil, err
  }
@@ -62,7 +62,7 @@ var _ = (*Container)(nil).OptionalErr // silence unused lint
 //
 // Use of NodeLiveness from within the SQL layer is **deprecated**. Please do
 // not introduce new uses of it.
-func (nl *Container) Optional(issueNos ...int) (Interface, bool) {
+func (nl *Container) Optional(issue int) (Interface, bool) {
  v, ok := nl.w.Optional()
  if !ok {
  return nil, false

pkg/sql/relocate.go

@@ -92,7 +92,7 @@ func (n *relocateNode) Next(params runParams) (bool, error) {
  // Lookup the store in gossip.
  var storeDesc roachpb.StoreDescriptor
  gossipStoreKey := gossip.MakeStoreKey(storeID)
- g, err := params.extendedEvalCtx.ExecCfg.Gossip.OptionalErr()
+ g, err := params.extendedEvalCtx.ExecCfg.Gossip.OptionalErr(54250)
  if err != nil {
  return false, err
  }

pkg/sql/scatter.go

@@ -34,7 +34,7 @@ type scatterNode struct {
 // Privileges: INSERT on table.
 func (p *planner) Scatter(ctx context.Context, n *tree.Scatter) (planNode, error) {
  if !p.ExecCfg().Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(54255)
  }
 
  tableDesc, index, err := p.getTableAndIndex(ctx, &n.TableOrIndex, privilege.INSERT)

pkg/sql/sem/builtins/generator_builtins.go

@@ -1129,7 +1129,8 @@ func makeCheckConsistencyGenerator(
  ctx *tree.EvalContext, args tree.Datums,
 ) (tree.ValueGenerator, error) {
  if !ctx.Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(
+ errorutil.FeatureNotAvailableToNonSystemTenantsIssue)
  }
 
  keyFrom := roachpb.Key(*args[1].(*tree.DBytes))

pkg/sql/set_zone_config.go

@@ -573,7 +573,7 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
  return err
  }
 
- ss, err := params.extendedEvalCtx.NodesStatusServer.OptionalNodesStatusServer()
+ ss, err := params.extendedEvalCtx.NodesStatusServer.OptionalNodesStatusServer(multitenancyZoneCfgIssueNo)
  if err != nil {
  return err
  }

pkg/sql/show_zone_config.go

@@ -64,7 +64,7 @@ const (
 
 func (p *planner) ShowZoneConfig(ctx context.Context, n *tree.ShowZoneConfig) (planNode, error) {
  if !p.ExecCfg().Codec.ForSystemTenant() {
- return nil, errorutil.UnsupportedWithMultiTenancy()
+ return nil, errorutil.UnsupportedWithMultiTenancy(multitenancyZoneCfgIssueNo)
  }
 
  return &delayedNode{

pkg/util/errorutil/tenant.go

@@ -10,28 +10,20 @@
 
 package errorutil
 
-import (
- "fmt"
- "strings"
-
- "github.com/cockroachdb/errors"
-)
+import "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 
 // UnsupportedWithMultiTenancy returns an error suitable for returning when an
 // operation could not be carried out due to the SQL server running in
 // multi-tenancy mode. In that mode, Gossip and other components of the KV layer
 // are not available.
-func UnsupportedWithMultiTenancy(issues ...int) error {
- var buf strings.Builder
- buf.WriteString("operation is unsupported in multi-tenancy mode")
- if len(issues) > 0 {
- buf.WriteString("; see:\n")
- const prefix = "\nhttps://github.com/cockroachdb/cockroach/issues/"
- for _, n := range issues {
- fmt.Fprint(&buf, prefix, n)
- }
- }
- // TODO(knz): This should be done in a different way,
- // see: https://github.com/cockroachdb/cockroach/issues/48164
- return errors.Newf("%s", buf.String())
+func UnsupportedWithMultiTenancy(issue int) error {
+ return unimplemented.NewWithIssue(issue, "operation is unsupported in multi-tenancy mode")
 }
+
+// FeatureNotAvailableToNonSystemTenantsIssue is to be used with the
+// Optional and related error interfaces when a feature is simply not
+// available to non-system tenants (i.e. we're not planning to change
+// this).
+// For all other multitenancy errors where there is a plan to
+// improve the situation, a specific issue should be created instead.
+const FeatureNotAvailableToNonSystemTenantsIssue = 54252

pkg/util/errorutil/tenant_deprecated_wrapper.go

@@ -73,10 +73,10 @@ func (w TenantSQLDeprecatedWrapper) Optional() (interface{}, bool) {
 
 // OptionalErr calls Optional and returns an error (referring to the optionally
 // supplied Github issues) if the wrapped object is not available.
-func (w TenantSQLDeprecatedWrapper) OptionalErr(issueNos ...int) (interface{}, error) {
+func (w TenantSQLDeprecatedWrapper) OptionalErr(issue int) (interface{}, error) {
  v, ok := w.Optional()
  if !ok {
- return nil, UnsupportedWithMultiTenancy(issueNos...)
+ return nil, UnsupportedWithMultiTenancy(issue)
  }
  return v, nil
 }