Fix max_message_length truncation for multibyte messages - #285
Conversation
| return message unless @max_message_length | ||
|
|
||
| message.byteslice(0, @max_message_length).scrub("") |
There was a problem hiding this comment.
Please add an early return when no truncation is needed.
| return message unless @max_message_length | |
| message.byteslice(0, @max_message_length).scrub("") | |
| return message unless @max_message_length | |
| return message if message.bytesize <= @max_message_length | |
| message.byteslice(0, @max_message_length).scrub("") |
As written, every event pays for a full string copy plus a full scrub scan even when the message already fits. max_message_length is typically configured with a fairly large value, so the common case is that nothing needs to be truncated.
Besides the performance win, this keeps scrub("") scoped to the messages we actually cut. scrub("") removes invalid byte sequences anywhere in the string rather than only at the boundary, so without the early return, setting max_message_length would silently alter messages that need no truncation at all. In practice scrub_record! already replaces invalid bytes in record values with U+FFFD before formatting, so this only matters for a custom <format> section that emits invalid UTF-8, but there is no reason to touch those messages when we are not truncating them.
Watson1978
left a comment
There was a problem hiding this comment.
Please update README.
This is a behavior change for users with multibyte messages, since their messages will now be truncated earlier than before. The current wording is ambiguous about the unit:
* `max_message_length`: maximum length of the message
Could you update it to state that the limit is measured in UTF-8 bytes, and mention that a character split across the boundary is dropped rather than corrupted? Something like maximum length of the message in UTF-8 bytes (a multibyte character split at the boundary is dropped) would work.
What
Why
CloudWatch validates event size in bytes. Character-based truncation could therefore leave multibyte messages larger than
max_message_length, while a raw byte slice could produce invalid UTF-8.Fixes #260
Tests
bundle exec ruby -Itest test/plugin/test_out_cloudwatch_logs.rb --name test_truncate_message_uses_byte_limit_and_preserves_utf8(1 test, 5 assertions)CI=true bundle exec ruby -Itest -e 'Dir["test/plugin/*.rb"].sort.each { |file| require File.expand_path(file) }'(58 tests, 57 assertions, 46 expected omissions)gem build fluent-plugin-cloudwatch-logs.gemspec --output /tmp/fluent-plugin-cloudwatch-logs-worker5.gemgit diff --check