-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Packetbeat, mysql proto, trim SQLs captured from app running #5572
Conversation
Can one of the admins verify this patch? |
Hi @timesking, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile? |
@karmi I added that email now. |
@timesking Thanks for the contribution. It would be great to have some tests to verify this change. |
@ruflin I saw mysql_test.go including some pcap binary data. Do you mean add a test in that file, also use pcap as input data? |
@timesking Whatever works best. But I think @urso is probably better to comment here. |
Change looks reasonable. Please add a system test to packetbeat/tests/system. Also add a changelog entry to Changelog.asciidoc |
@urso Sure, I will learn how to add that python test case. |
@urso All passed except that triavis-ci.
|
jenkins, test it |
@timesking The tests are fine. I checked failures and non are related to your PR. can you also add a Changelog entry (to Changelog.asciidoc) about fixing newlines on windows? Also rebase before doing so, so not to have merge conflicts on the changelog file. |
b84d3f1
to
5be5c95
Compare
@urso the changeslog has been added, and all changes rebased on master |
8657ce2
to
f6a5081
Compare
@timesking I can't see the changelog entry. Did it get lost during rebasing? Can you rebase on master again? |
…g on Windows server. Otherwise method extracted including \r, which is problem. e.g. "SELECT\r\n\t1"
f6a5081
to
002f128
Compare
@ruflin Sorry for that, I rebased again, fixed missing changelog |
Merged. Thanks for contributing the fix! |
Can one of the admins verify this patch? |
elastic#5572) * Packetbeat, mysql proto, add \r to trim SQLs captured from app running on Windows server. Otherwise method extracted including \r, which is problem. e.g. "SELECT\r\n\t1" * Packetbeat, test case for windows lineending * Packetbeat, changelog for windows lineending
#5572) * Packetbeat, mysql proto, add \r to trim SQLs captured from app running on Windows server. Otherwise method extracted including \r, which is problem. e.g. "SELECT\r\n\t1" * Packetbeat, test case for windows lineending * Packetbeat, changelog for windows lineending
Packetbeat
Mysql proto
add \r to trim SQLs captured from app running on Windows server. Otherwise method extracted including \r, which is problem. e.g. "SELECT\r\n\t1".