Skip to content

Commit

Permalink
sql,log: productionize the event logging
Browse files Browse the repository at this point in the history
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.
- it automates the generation of a reference documentation for all
  event types.

(FIXME - remaining to be done:)

- 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): The cluster event logging system has been
modernized. In particular, the schema of the entries for the `info`
column in `system.eventlog` has been stabilized. See the documentation
for details.

Release note (sql change): The `targetID` and `reportingID` columns in
`system.eventlog` are now deprecated. Their values, for relevant event
types, can be found as fields inside the `info` column instead.
  • Loading branch information
knz committed Dec 10, 2020
1 parent dc4eaf0 commit d4186bc
Show file tree
Hide file tree
Showing 69 changed files with 20,511 additions and 1,036 deletions.
14 changes: 13 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,8 @@ DOCGEN_TARGETS := \
bin/.docgen_functions \
docs/generated/redact_safe.md \
bin/.docgen_http \
docs/generated/logging.md
docs/generated/logging.md \
docs/generated/eventlog.md

EXECGEN_TARGETS = \
pkg/col/coldata/vec.eg.go \
Expand Down Expand Up @@ -1552,6 +1553,17 @@ docs/generated/redact_safe.md:
sed -E -e 's/^([^:]*):[0-9]+:.*redact\.RegisterSafeType\((.*)\).*/\1 | \`\2\`/g' >>$@.tmp || { rm -f $@.tmp; exit 1; }
@mv -f $@.tmp $@

EVENTLOG_PROTOS = \
pkg/util/log/eventpb/events.proto \
pkg/util/log/eventpb/ddl_events.proto \
pkg/util/log/eventpb/misc_sql_events.proto \
pkg/util/log/eventpb/privilege_events.proto \
pkg/util/log/eventpb/cluster_events.proto

docs/generated/eventlog.md: pkg/util/log/eventpb/gen.go $(EVENTLOG_PROTOS)
$(GO) run $(GOFLAGS) $(GOMODVENDORFLAGS) $< eventlog.md $(EVENTLOG_PROTOS) >$@.tmp || { rm -f $@.tmp; exit 1; }
mv -f $@.tmp $@

docs/generated/logging.md: pkg/util/log/gen.go pkg/util/log/logpb/log.proto
$(GO) run $(GOFLAGS) $(GOMODVENDORFLAGS) $^ logging.md $@.tmp || { rm -f $@.tmp; exit 1; }
mv -f $@.tmp $@
Expand Down
1 change: 1 addition & 0 deletions build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ define VALID_VARS
ENABLE_LIBROACH_ASSERTIONS
ERRORS_PATH
ERRORS_PROTO
EVENTLOG_PROTOS
EXECGEN_TARGETS
EXTRA_XCMAKE_FLAGS
EXTRA_XCONFIGURE_FLAGS
Expand Down
Loading

0 comments on commit d4186bc

Please sign in to comment.