logging: standardize the event log and bring structured details to external logs #57629
Closed
26 tasks done
Assignees
Labels
A-logging
In and around the logging infrastructure.
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Comments
|
Hi @knz, I've guessed the C-ategory of your issue and suitably labeled it. Please re-label if inaccurate. While you're here, please consider adding an A- label to help keep our repository tidy. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
blathers-crl
bot
added
the
C-enhancement
|
cc @thtruo |
|
Finding during the investigation:
So we will need different metadata for them. |
This comment has been minimized.
This comment has been minimized.
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]>
|
lifting the remaining to-do list to the top of the issue. |
|
All the items have been addressed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Today we have an
EventLogmechanism inside the SQL package that writes JSON blobs tosystem.eventlog. These use structured payloads that are undocumented, unstable and irregular/inconsistent.Then, separately, we also have the logging infrastructure that writes unstructured strings to external files.
We want instead to have structured payloads that are documented, stable and consistent, and have them into external files (or network collector when that is supported - see #57170 ), on a dedicated logging channel (see #57171 ).
Steps:
pkg/ui/src/utilaccordingly sql,log: productionize the event logging #57737sql.plannerthat everything that touches descriptor has at least 1 event logged - NOT DONE, instead filed sql: insert an assertion in planner / table collection that requires event logging for all DDL #57823 for SQL Schema teamutil/logand treat them separately sql,log: productionize the event logging #57737system.eventlogconditional #57879The text was updated successfully, but these errors were encountered: