TiDB Cloud Serverless database audit logging #20526
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
missing-translation-status
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces documentation for TiDB Cloud Serverless database audit logging. The changes include adding a new documentation file and updating the table of contents. Overall, the documentation seems clear and well-structured. However, there are a few points that could be improved for better clarity and completeness.
Summary of Findings
- Note about audit log retention period: The note about the audit log retention period in
tidb-cloud/serverless-audit-logging.mdcurrently has a placeholder value (xx days). This should be updated with the actual retention period to provide accurate information to the user. - Incomplete CLI reference: The CLI reference in
tidb-cloud/serverless-audit-logging.mdlacks specific details about the audit-log commands. It would be beneficial to include links to the specific CLI command documentation pages forenable,disable,config, anddownloadto provide users with more detailed information.
Merge Readiness
The pull request is almost ready for merging. However, the placeholder value for the audit log retention period should be updated with the actual value. Additionally, providing direct links to the CLI command documentation would enhance the user experience. Once these issues are addressed, the pull request will be ready for merging. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
|
|
||
| ## Enable audit logging | ||
|
|
||
| To enable the audit logging for a TiDB Cloud Serverless cluster, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) |
There was a problem hiding this comment.
Consider adding a link to the specific CLI command documentation for ticloud serverless audit-log enable for more detailed information.
| To enable the audit logging for a TiDB Cloud Serverless cluster, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) | |
| To enable the audit logging for a TiDB Cloud Serverless cluster, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md#ticloud-serverless-audit-log-enable) |
| ticloud serverless audit-log enable --cluster-id <cluster-id> | ||
| ``` | ||
|
|
||
| To disable the audit logging for a TiDB Cloud Serverless cluster, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) |
There was a problem hiding this comment.
Consider adding a link to the specific CLI command documentation for ticloud serverless audit-log disable for more detailed information.
| To disable the audit logging for a TiDB Cloud Serverless cluster, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) | |
| To disable the audit logging for a TiDB Cloud Serverless cluster, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md#ticloud-serverless-audit-log-disable) |
| INSERT INTO `test`.`users` (`id`, `name`, `password`) VALUES ( ... ); | ||
| ``` | ||
|
|
||
| If you want to disable the redaction, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) |
There was a problem hiding this comment.
Consider adding a link to the specific CLI command documentation for ticloud serverless audit-log config for more detailed information.
| If you want to disable the redaction, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) | |
| If you want to disable the redaction, using the [TiDB Cloud CLI](/tidb-cloud/cli-reference.md#ticloud-serverless-audit-log-config) |
|
|
||
| ## View audit logs | ||
|
|
||
| TiDB Cloud Serverless audit logs are readable text files named `YYYY-MM-DD-<uuid>.log`. You can download the audit logs by [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) to view them. |
There was a problem hiding this comment.
Consider adding a link to the specific CLI command documentation for ticloud serverless audit-log download for more detailed information.
| TiDB Cloud Serverless audit logs are readable text files named `YYYY-MM-DD-<uuid>.log`. You can download the audit logs by [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) to view them. | |
| You can download the audit logs by [TiDB Cloud CLI](/tidb-cloud/cli-reference.md#ticloud-serverless-audit-log-download) to view them. |
|
@gemini-code-assist[bot]: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces audit logging for TiDB Cloud Serverless databases. The changes include adding a new documentation page for serverless audit logging and updating the table of contents. Overall, the documentation is clear and provides instructions on how to enable, configure, and view audit logs. However, there are a few areas where improvements can be made to enhance clarity and consistency.
Summary of Findings
- Incomplete Note: The note in the 'View audit logs' section of
tidb-cloud/serverless-audit-logging.mdis incomplete and needs to be filled in with the retention period for audit logs. - Inconsistent Terminology: The term 'database audit logging' is used in the TOC, while the individual pages use 'TiDB Cloud Dedicated Database Audit Logging' and 'TiDB Cloud Serverless Database Audit Logging'. Consistency in terminology is important for clarity.[^1]
- Missing Link: The link to the TiDB Cloud CLI in
tidb-cloud/serverless-audit-logging.mdshould be checked to ensure it is up to date.
Merge Readiness
The pull request is almost ready for merging. However, the incomplete note in tidb-cloud/serverless-audit-logging.md should be addressed before merging. Additionally, ensuring consistency in terminology and verifying the TiDB Cloud CLI link would improve the overall quality of the documentation. I am unable to approve this pull request, and recommend that others review and approve this code before merging.
| ``` | ||
|
|
||
| > **Note:** | ||
| > TiDB Cloud only save your audit logs xx days. |
There was a problem hiding this comment.
This note is incomplete. Please fill in the number of days that TiDB Cloud saves audit logs. For example: TiDB Cloud only saves your audit logs for 90 days.
| > TiDB Cloud only save your audit logs xx days. | |
| > TiDB Cloud only saves your audit logs for XX days. |
|
|
||
| ## View audit logs | ||
|
|
||
| TiDB Cloud Serverless audit logs are readable text files named `YYYY-MM-DD-<uuid>.log`. You can download the audit logs by [TiDB Cloud CLI](/tidb-cloud/cli-reference.md) to view them. |
There was a problem hiding this comment.
It might be helpful to verify that this link is up to date and points to the correct location for the TiDB Cloud CLI documentation.
|
@gemini-code-assist[bot]: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
First-time contributors' checklist
What is changed, added or deleted? (Required)
Which TiDB version(s) do your changes apply to? (Required)
Tips for choosing the affected version(s):
By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.
For details, see tips for choosing the affected versions.
What is the related PR or file link(s)?
Do your changes match any of the following descriptions?