-
Notifications
You must be signed in to change notification settings - Fork 1.9k
aws: Update compression support for AWS plugins #11400
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds SNAPPY compression support across AWS outputs: new compression constant, snappy wiring in AWS compression dispatch, Kinesis config + runtime compression paths, S3 header handling, Firehose docs update, and expanded unit/runtime tests for snappy (and other compressions). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant KinesisOutput as Kinesis Output Plugin
participant CompressDispatch as AWS Compression Dispatch
participant RecordBuffer as Record/Event Buffer
Client->>KinesisOutput: Submit record(s)
KinesisOutput->>KinesisOutput: Read ctx->compression
alt compression != NONE
KinesisOutput->>CompressDispatch: flb_aws_compression_b64_truncate_compress(in_data)
CompressDispatch-->>KinesisOutput: compressed_b64_data + size
KinesisOutput->>RecordBuffer: Replace buffer with compressed data
else compression == NONE
KinesisOutput->>RecordBuffer: Base64-encode data (existing path)
RecordBuffer-->>KinesisOutput: base64_data + size
end
KinesisOutput->>Client: send record(s) to AWS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@plugins/out_kinesis_streams/kinesis_api.h`:
- Line 26: The macro MAX_B64_EVENT_SIZE currently defines 1398075 but the
comment formula ceil(1048556 / 3) * 4 evaluates to 1398076; update the macro or
the comment to be consistent: either change the macro MAX_B64_EVENT_SIZE to
1398076 to match the formula, or if the -1 is intentional (e.g., to
reserve/exclude a NUL), update the comment next to MAX_B64_EVENT_SIZE to clearly
state that the value intentionally subtracts one (explain reason). Ensure you
edit the definition of MAX_B64_EVENT_SIZE and its inline comment together so the
code and comment match.
In `@src/aws/flb_aws_compress.c`:
- Around line 35-41: The wrapper flb_snappy_compress_wrapper currently
propagates negative error codes from flb_snappy_compress; change it to call
flb_snappy_compress into an int variable (e.g., ret) and then return -1 if ret <
0, otherwise return ret, so all negative error codes are normalized to -1 to
match the AWS compression interface contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5e4ce37e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
Signed-off-by: Shelby Hagman <shelbyzh@amazon.com>
f5e4ce3 to
fbbe38d
Compare
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Summary view
https://gist.github.com/ShelbyZ/6a325762003785ba1804faeff958a1c7
Debug Logs
https://gist.github.com/ShelbyZ/9d0036c46600b44cb9a4cea99795230a
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
fluent/fluent-bit-docs#2359
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.