sql: event log entries for DDL for user-defined schemas do not include fully qualified names #57738
Comments
knz
added
C-bug
knz
added a commit
to knz/cockroach
that referenced
this issue
Dec 9, 2020
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
|
cc @thtruo this is needed for full DDL logging |
knz
added a commit
to knz/cockroach
that referenced
this issue
Dec 10, 2020
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.
knz
added a commit
to knz/cockroach
that referenced
this issue
Dec 11, 2020
CockroachDB needs to report structured events for
"important" (notable) 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.
- events could be omitted entirely from external logs, e.g.
if the system.eventlog table was not available during a node
restart.
- 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 groups events into categories and associates logging
channels to each event category.
- 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.
- it ensures that events are sent to external logs unconditionally.
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.
Release note (cli change): The events that were previously only stored
in `system.eventlog` are now also directed unconditionally to an
external logging channel using a JSON format. Refer to the
configuration to see how to customize how events are directed to
external sinks. Note that the exact external output format (and thus
how to detect/parse the events from e.g. log files) is not yet
stabilized and remains subject to change.
Release note (doc change): The cluster event logging system has been
standardized. A reference documentation is now
available (auto-generated from source code); changes to non-reserved
payloads will now be announced at least one release version in
advance. The event types are organized into broad categories: SQL
Logical Schema Changes, SQL Privilege Changes, SQL User Management,
CLuster-level events and SQL Miscellaneous operations.
Release note (backward-incompatible change): The payload fields for
certain event types in `system.eventlog` have been changed and/or
renamed. Note that the payloads in `system.eventlog` were an
undocumented, reserved feature so no guarantee was made about
cross-version compatibility to this point. The list of changes
includes (but is not limited to):
- `TargetID` has been renamed to `NodeID` for `node_join`
- `TargetID` has been renamed to `TargetNodeID` for
`node_decommissioning` / `node_decommissioned` /
`node_recommissioned`.
- `NewDatabaseName` has been renamed to `NewDatabaseParent` for
`convert_to_schema`.
- `grant_privilege` and `revoke_privilege` have been removed;
they are replaced by `change_database_privilege`,
`change_schema_privilege`, `change_type_privilege` and
`change_table_privilege`.
Each event only reports change for one user/role, so the
`Grantees` field was renamed to `Grantee`.
- Each `drop_role` event now pertains to a single user/role.
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.
craig bot
pushed a commit
that referenced
this issue
Dec 11, 2020
57737: sql,log: productionize the event logging r=itsbilal a=knz Informs #57629 CockroachDB needs to report structured events for "important" (notable) 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. - events could be omitted entirely from external logs, e.g. if the system.eventlog table was not available during a node restart. - 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 groups events into categories and associates logging channels to each event category. - 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. - it ensures that events are sent to external logs unconditionally. 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: #57573 #57576 #57739 #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: #57734 #57735 #57738 #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. Release note (cli change): The events that were previously only stored in `system.eventlog` are now also directed unconditionally to an external logging channel using a JSON format. Refer to the configuration to see how to customize how events are directed to external sinks. Note that the exact external output format (and thus how to detect/parse the events from e.g. log files) is not yet stabilized and remains subject to change. Release note (doc change): The cluster event logging system has been standardized. A reference documentation is now available (auto-generated from source code); changes to non-reserved payloads will now be announced at least one release version in advance. The event types are organized into broad categories: SQL Logical Schema Changes, SQL Privilege Changes, SQL User Management, CLuster-level events and SQL Miscellaneous operations. Release note (backward-incompatible change): The payload fields for certain event types in `system.eventlog` have been changed and/or renamed. Note that the payloads in `system.eventlog` were an undocumented, reserved feature so no guarantee was made about cross-version compatibility to this point. The list of changes includes (but is not limited to): - `TargetID` has been renamed to `NodeID` for `node_join` - `TargetID` has been renamed to `TargetNodeID` for `node_decommissioning` / `node_decommissioned` / `node_recommissioned`. - `NewDatabaseName` has been renamed to `NewDatabaseParent` for `convert_to_schema`. - `grant_privilege` and `revoke_privilege` have been removed; they are replaced by `change_database_privilege`, `change_schema_privilege`, `change_type_privilege` and `change_table_privilege`. Each event only reports change for one user/role, so the `Grantees` field was renamed to `Grantee`. - Each `drop_role` event now pertains to a single user/role. 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. 57752: kvserver: only accept raw engines for WriteInitialClusterVersion r=irfansharif a=irfansharif We have an invariant to maintain around ensuring that all writes to this key are durably persisted. Let's update our signature to reflect as much. Release note: None Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: irfan sharif <[email protected]>
|
Discussed with @aaron-crl : lower priority. |
craig bot
pushed a commit
that referenced
this issue
Jan 25, 2021
58617: sql: add qualification prefix for user-defined schema names r=the-ericwang35 a=the-ericwang35 Fixes #57738. Previously, event logs were not capturing the qualified schema names for create_schema, drop_schema, rename_schema and alter_schema_owner events. This PR changes the event logs to use the qualified schema names. Tests were also updated to reflect these changes. Release note (bug fix): add qualification prefix for user-defined schema names. 59139: streamclient: add random stream client r=pbardea a=pbardea See individual commits, but this PR does some cleanup while introducing a stream client implementation that randomly generates rows. I broke down each change into its own commit for reviews, but let me know if splitting this into separate PRs would be helpful. 59276: opt: add opttester command to check rule applications and memo size r=rytaft a=DrewKimball Previously, it was difficult to test that the number of rule applications and/or memo groups remains reasonable during optimization of a query. This patch adds the `check-size` command, which outputs the number of rules applied and memo groups created during optimization. The `rule-limit` and `group-limit` flags can be used to throw an error if the number of rule applications or memo groups exceed the given limit. Release note: None Fixes #59192 Co-authored-by: Eric Wang <[email protected]> Co-authored-by: Paul Bardea <[email protected]> Co-authored-by: Andrew Kimball <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
See in system.eventlog:
The schema name does not contain a qualification prefix, which means it's not possible to determine which database the schema is in.
The text was updated successfully, but these errors were encountered: