Skip to content

executor: fix some audit log error#26489

Closed
tiancaiamao wants to merge 2 commits intopingcap:release-3.0from
tiancaiamao:fix-audit-log-3.0
Closed

executor: fix some audit log error#26489
tiancaiamao wants to merge 2 commits intopingcap:release-3.0from
tiancaiamao:fix-audit-log-3.0

Conversation

@tiancaiamao
Copy link
Contributor

@tiancaiamao tiancaiamao commented Jul 22, 2021

What problem does this PR solve?

Problem Summary:

Some of the audit log are incorrect:
https://docs.google.com/spreadsheets/d/1E59z7busbRwjb2D20QLbmAGhLT75vec44HTB5vi2WrI/edit#gid=0

What is changed and how it works?

What's Changed:

Fix the following bugs:

  • 'drop review' is not logged as 'drop table'
  • 'create sesson binding' logged as 'explain'
  • prepare statement operation is not logged
  • 'trace' statement is logged as 'select'
  • no audit log for 'shutdown' statement

How it Works:

Some are caused by AST not registed in GetStmtLabel()
Some are caused by the statement execute some other SQL internally and change the statement context

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Insert (".", ".",".*","") into mysql.tidb_audit_table_access before executing

  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

  • Fix some audit log that do not work as expected, including 'drop view'/'create session binding'/'prepare'/'trace'/'shutdown' etc

@ti-chi-bot ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 22, 2021
@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 22, 2021
@ti-srebot
Copy link
Contributor

@github-actions github-actions bot added the sig/execution SIG execution label Jul 22, 2021
@ti-chi-bot
Copy link
Member

@tiancaiamao: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

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/test-infra repository.

@ti-chi-bot ti-chi-bot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Jul 30, 2021
@tiancaiamao tiancaiamao marked this pull request as ready for review July 30, 2021 09:14
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2021
@tiancaiamao tiancaiamao mentioned this pull request Jul 31, 2021
12 tasks
@zhouqiang-cl zhouqiang-cl removed the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Aug 4, 2021
@tiancaiamao
Copy link
Contributor Author

Not for 3.X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/execution SIG execution size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants