sql,log: productionize the event logging

CockroachDB needs to report structured events for "important" events
on the logical structure of a cluster, including changes to the SQL
logical schema, node activity, privilege changes etc.

Prior to this patch, these events were reported mainly into the
table `system.eventlog`, with a partial copy of the payload into the
debugging external log (`cockroach.log`).

This solution was incomplete and unsatisfactory in many ways:

- the event payloads were not documented.

- the event payloads were not centrally defined, which was preventing
  the generation of automatic documentation.

- the payload types were declared "inline" in the log calls, which
  made it easy for team members to inadvertently change the structure of
  the payload and make them backward-incompatible for users consuming
  this data externally.

- the payload fields were inconsistently named across event types.

- the metadata fields on the payloads were incompletely and
  inconsistently populated:

  - the SQL instance ID was missing in some cases.
  - the descriptor ID of affected descriptor was missing in some
    cases.

- the same event type was mistakenly used for different events (e.g.
  "rename_database" for both RENAME DATABASE and CONVERT TO SCHEMA)

- the same event type was abusingly over-used for multiple separate
  operations, e.g. a single event would be generated for a multi-table,
  multi-user GRANT or REVOKE operation.

- the copy in the external log was not parseable. Generally, the
  logging package was unaware of the internal structure of events
  and would “flatten” them.

- no provision was available to partially redact events. From the
  logging system's perspective, the entire payload is sensitive.

This commit changes the situation as follows:

- it centralizes the payload definitions and standardizes them into a
  new package `eventspb`.
- it enables automatic generation of documentation for events.
- it ensures that field names are consistent across event payloads.
- it ensures that event metadata is consistently populted.
- it decomposes complex GRANT/REVOKE operations into individual
  events.

(FIXME - remaining to be done:)

- it automates the generation of a reference documentation for all
  event types.
- it provide a guardrail against the introduction of new DDL
  statements without a corresponding event log.

The following problems continue to exist and need to be resolved
separately:

- privilege changes that occur as a side effect of certain operations
  do not get events logged:

  cockroachdb#57573
  cockroachdb#57576
  cockroachdb#57739
  cockroachdb#57741

- the name fields in certain DDL events is not properly qualified,
  which prevents the determination of the logical schema or database
  where the object was altered:

  cockroachdb#57734
  cockroachdb#57735
  cockroachdb#57738
  cockroachdb#57740

Release note (sql change): FIXME

pkg/sql/alter_table.go

@@ -838,24 +838,15 @@ func (n *alterTableNode) startExec(params runParams) error {
 	// Record this table alteration in the event log. This is an auditable log
 	// event and is recorded in the same transaction as the table descriptor
 	// update.
-	return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogAlterTable,
-		int32(n.tableDesc.ID),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			TableName           string
-			Statement           string
-			User                string
-			MutationID          uint32
-			CascadeDroppedViews []string
-		}{
-			params.p.ResolvedName(n.n.Table).FQString(),
-			n.n.String(),
-			params.SessionData().User().Normalized(),
-			uint32(mutationID), droppedViews},
-	)
+	return params.p.logEvent(params.ctx,
+		n.tableDesc.ID,
+		&eventlog.AlterTable{
+			TableName:  params.p.ResolvedName(n.n.Table).FQString(),
+			MutationID: uint32(mutationID),
+			// TODO(knz): This is missing some qualification, see
+			// https://github.com/cockroachdb/cockroach/issues/57735
+			CascadeDroppedViews: droppedViews,
+		})
 }
 
 func (p *planner) setAuditMode(

pkg/sql/alter_type.go

@@ -24,6 +24,7 @@ import (
 	"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/eventpb"
 	"github.com/cockroachdb/errors"
 )
 
@@ -83,18 +84,40 @@ 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())
+
+	eventLogDone := false
 	var err error
 	switch t := n.n.Cmd.(type) {
 	case *tree.AlterTypeAddValue:
 		err = params.p.addEnumValue(params.ctx, n, t)
 	case *tree.AlterTypeRenameValue:
 		err = params.p.renameTypeValue(params.ctx, n, string(t.OldVal), string(t.NewVal))
 	case *tree.AlterTypeRename:
-		err = params.p.renameType(params.ctx, n, string(t.NewName))
+		if err = params.p.renameType(params.ctx, n, string(t.NewName)); err != nil {
+			return err
+		}
+		err = params.p.logEvent(params.ctx, n.desc.ID, &eventpb.RenameType{
+			// TODO(knz): This name is insufficiently qualified.
+			// See: https://github.com/cockroachdb/cockroach/issues/57734
+			TypeName:    n.desc.Name,
+			NewTypeName: string(t.NewName),
+		})
+		eventLogDone = true
 	case *tree.AlterTypeSetSchema:
+		// TODO(knz): this is missing dedicated logging,
+		// See https://github.com/cockroachdb/cockroach/issues/57741
 		err = params.p.setTypeSchema(params.ctx, n, string(t.Schema))
 	case *tree.AlterTypeOwner:
-		err = params.p.alterTypeOwner(params.ctx, n, t.Owner)
+		if err = params.p.alterTypeOwner(params.ctx, n, t.Owner); err != nil {
+			return err
+		}
+		err = params.p.logEvent(params.ctx, n.desc.ID, &eventpb.AlterTypeOwner{
+			// TODO(knz): This name is insufficiently qualified.
+			// See: https://github.com/cockroachdb/cockroach/issues/57734
+			TypeName: n.desc.Name,
+			Owner:    t.Owner.Normalized(),
+		})
+		eventLogDone = true
 	default:
 		err = errors.AssertionFailedf("unknown alter type cmd %s", t)
 	}
@@ -108,23 +131,17 @@ func (n *alterTypeNode) startExec(params runParams) error {
 		return err
 	}
 
-	// Write a log event.
-	return MakeEventLogger(params.p.ExecCfg()).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogAlterType,
-		int32(n.desc.ID),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			TypeName  string
-			Statement string
-			User      string
-		}{
-			n.desc.Name,
-			tree.AsStringWithFQNames(n.n, params.Ann()),
-			params.p.User().Normalized(),
-		},
-	)
+	if !eventLogDone {
+		// Write a log event.
+		if err := params.p.logEvent(params.ctx,
+			n.desc.ID,
+			&eventpb.AlterType{
+				TypeName: n.desc.Name,
+			}); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func (p *planner) addEnumValue(

pkg/sql/comment_on_column.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
 type commentOnColumnNode struct {
@@ -88,25 +89,20 @@ func (n *commentOnColumnNode) startExec(params runParams) error {
 		}
 	}
 
-	return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogCommentOnColumn,
-		int32(n.tableDesc.ID),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			TableName  string
-			ColumnName string
-			Statement  string
-			User       string
-			Comment    *string
-		}{
-			n.tableDesc.Name,
-			string(n.n.ColumnItem.ColumnName),
-			n.n.String(),
-			params.p.User().Normalized(),
-			n.n.Comment},
-	)
+	comment := ""
+	if n.n.Comment != nil {
+		comment = *n.n.Comment
+	}
+	return params.p.logEvent(params.ctx,
+		n.tableDesc.ID,
+		&eventpb.CommentOnColumn{
+			// TODO(knz): This table name is improperly qualified.
+			// See: https://github.com/cockroachdb/cockroach/issues/57740
+			TableName:   n.tableDesc.Name,
+			ColumnName:  string(n.n.ColumnItem.ColumnName),
+			Comment:     comment,
+			NullComment: n.n.Comment == nil,
+		})
 }
 
 func (n *commentOnColumnNode) Next(runParams) (bool, error) { return false, nil }

pkg/sql/comment_on_database.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
 type commentOnDatabaseNode struct {
@@ -79,23 +80,17 @@ func (n *commentOnDatabaseNode) startExec(params runParams) error {
 		}
 	}
 
-	return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogCommentOnDatabase,
-		int32(n.dbDesc.GetID()),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			DatabaseName string
-			Statement    string
-			User         string
-			Comment      *string
-		}{
-			n.n.Name.String(),
-			n.n.String(),
-			params.p.User().Normalized(),
-			n.n.Comment},
-	)
+	comment := ""
+	if n.n.Comment != nil {
+		comment = *n.n.Comment
+	}
+	return params.p.logEvent(params.ctx,
+		n.dbDesc.GetID(),
+		&eventpb.CommentOnDatabase{
+			DatabaseName: n.n.Name.String(),
+			Comment:      comment,
+			NullComment:  n.n.Comment == nil,
+		})
 }
 
 func (n *commentOnDatabaseNode) Next(runParams) (bool, error) { return false, nil }

pkg/sql/comment_on_index.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/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
 type commentOnIndexNode struct {
@@ -64,25 +65,20 @@ func (n *commentOnIndexNode) startExec(params runParams) error {
 		}
 	}
 
-	return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogCommentOnIndex,
-		int32(n.tableDesc.ID),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			TableName string
-			IndexName string
-			Statement string
-			User      string
-			Comment   *string
-		}{
-			n.tableDesc.Name,
-			string(n.n.Index.Index),
-			n.n.String(),
-			params.p.User().Normalized(),
-			n.n.Comment},
-	)
+	comment := ""
+	if n.n.Comment != nil {
+		comment = *n.n.Comment
+	}
+	return params.p.logEvent(params.ctx,
+		n.tableDesc.ID,
+		&eventpb.CommentOnIndex{
+			// TODO(knz): This table name is improperly qualified.
+			// See: https://github.com/cockroachdb/cockroach/issues/57740
+			TableName:   n.tableDesc.Name,
+			IndexName:   string(n.n.Index.Index),
+			Comment:     comment,
+			NullComment: n.n.Comment == nil,
+		})
 }
 
 func (p *planner) upsertIndexComment(

pkg/sql/comment_on_table.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
 type commentOnTableNode struct {
@@ -79,24 +80,17 @@ func (n *commentOnTableNode) startExec(params runParams) error {
 		}
 	}
 
-	return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogCommentOnTable,
-		int32(n.tableDesc.ID),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			TableName string
-			Statement string
-			User      string
-			Comment   *string
-		}{
-			params.p.ResolvedName(n.n.Table).FQString(),
-			n.n.String(),
-			params.p.User().Normalized(),
-			n.n.Comment,
-		},
-	)
+	comment := ""
+	if n.n.Comment != nil {
+		comment = *n.n.Comment
+	}
+	return params.p.logEvent(params.ctx,
+		n.tableDesc.ID,
+		&eventpb.CommentOnTable{
+			TableName:   params.p.ResolvedName(n.n.Table).FQString(),
+			Comment:     comment,
+			NullComment: n.n.Comment == nil,
+		})
 }
 
 func (n *commentOnTableNode) Next(runParams) (bool, error) { return false, nil }

pkg/sql/create_database.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
 	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
+	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
 type createDatabaseNode struct {
@@ -107,18 +108,10 @@ func (n *createDatabaseNode) startExec(params runParams) error {
 	if created {
 		// Log Create Database event. This is an auditable log event and is
 		// recorded in the same transaction as the table descriptor update.
-		if err := MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-			params.ctx,
-			params.p.txn,
-			EventLogCreateDatabase,
-			int32(desc.GetID()),
-			int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-			struct {
-				DatabaseName string
-				Statement    string
-				User         string
-			}{n.n.Name.String(), n.n.String(), params.p.User().Normalized()},
-		); err != nil {
+		if err := params.p.logEvent(params.ctx, desc.GetID(),
+			&eventpb.CreateDatabase{
+				DatabaseName: n.n.Name.String(),
+			}); err != nil {
 			return err
 		}
 	}

pkg/sql/create_index.go

@@ -31,6 +31,7 @@ import (
 	"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/log/eventpb"
 	"github.com/cockroachdb/errors"
 )
 
@@ -471,23 +472,13 @@ func (n *createIndexNode) startExec(params runParams) error {
 	// Record index creation in the event log. This is an auditable log
 	// event and is recorded in the same transaction as the table descriptor
 	// update.
-	return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
-		params.ctx,
-		params.p.txn,
-		EventLogCreateIndex,
-		int32(n.tableDesc.ID),
-		int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
-		struct {
-			TableName  string
-			IndexName  string
-			Statement  string
-			User       string
-			MutationID uint32
-		}{
-			n.n.Table.FQString(), indexName, n.n.String(),
-			params.p.User().Normalized(), uint32(mutationID),
-		},
-	)
+	return params.p.logEvent(params.ctx,
+		n.tableDesc.ID,
+		&eventpb.DropIndex{
+			TableName:  n.n.Table.FQString(),
+			IndexName:  indexName,
+			MutationID: uint32(mutationID),
+		})
 }
 
 func (*createIndexNode) Next(runParams) (bool, error) { return false, nil }