Support mysql prepare statement command #8084
Conversation
adriansr
added
enhancement
in progress
|
I have checked these commits. Thanks for your help. |
There was a problem hiding this comment.
I left some suggestions.
In general, I think we should try to remove most or all binary shifts from mysql.go. The binary package should have most functionality and we can add anything that's missing.
Since I don't know much about the MySQL wire protocol I think @andrewkroh or @urso should have a look as well.
|
@ggg-geyi I am currently working in some problems I found with the mysql parser (not caused by your code), I will fix them and then take care of the changes requested in this PR and merging it. |
|
If it's possible I can do some help. |
|
I believe this addresses both of these issues and they can be closed by this PR. |
|
@cwurm I addressed all your comments, do you mind to give it another look? |
adriansr
added
review
and removed
in progress
| self.render_config_template( | ||
| mysql_ports=[3307] | ||
| ) | ||
| self.run_packetbeat(pcap="mysql_prepare_statement.pcap", |
| @@ -140,6 +152,10 @@ type mysqlPlugin struct { | |||
| transactions *common.Cache | |||
| transactionTimeout time.Duration | |||
|
|
|||
| // prepare statements cache | |||
| prepareStatements *common.Cache | |||
| prepareStatementTimeout time.Duration | |||
There was a problem hiding this comment.
Prepared statements are accessed via address tuple. A mysql connection is allowed to only have this many prepared statements (applications requiring more or dynamic set of prepared statements tend to implement a Cache of active statements).
Why store the statements globally and time them out globally? I wonder if we want to move the statements to the TCP connection state.
Advantage is: if the connection is gone all known prepared statements are gone as well
Disadvantage: if connection is dropped (internal timeout or parse error after gaps), then we loose the current state.
Alternatively bind to connections + timeout only if connection is dropped/errored:
- If TCP connection is properly closed we can can drop the state
- If connection is dropped we put the connection it's statements on timeout
- If new connection without TCP handshake is found we check if set of prepared statements is available for address pair and assign it to the current connection (stopping timeout).
LGTM. Sorry I took so long to get to this. See @urso's comment though - sounds like a good idea to me, but not sure if it's worth putting into this already very long running PR or if it should rather be handled as a follow-up. Anything is good with me. |
This might also need some improvements in signaling a plugin about connection state. +1 for changes/improvements to come later. Let's open an issue, though. |
|
Thanks @urso I will follow up with a new PR. |
This is a cleanup of #5663 (Support mysql prepare statement command by @ggg-geyi)