out_firehose: out_kinesis: add simple_aggregation config parameter and cleanup #2317
Conversation
📝 WalkthroughWalkthroughDocumentation updates for Kinesis Firehose and Kinesis Streams outputs: new and revised configuration keys (including Changes
Sequence Diagram(s)(omitted — changes are documentation/configuration updates without new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-12-12T14:30:18.461ZApplied to files:
🔇 Additional comments (2)
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 |
|
@esmerel review request for docs update needed to match 4.2.2 release. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
pipeline/outputs/firehose.md (2)
28-28: Avoid "simple" terminology in description per style guidelines.The vale FluentBit.Simplicity rule flags words like "simple" that may imply ease to users when the feature could be complex. The parameter name cannot be changed, but the description can be reworded to avoid this term.
🔎 Proposed revision
- | `simple_aggregation` | Enable simple aggregation to combine multiple records into single API calls. This reduces the number of requests and can improve throughput. | `false` | + | `simple_aggregation` | Enable record aggregation to combine multiple records into single API calls. This reduces the number of requests and can improve throughput. | `false` |
28-28: Rephrase description to avoid "simple" terminology per style guidelines.The vale FluentBit.Simplicity rule flags words like "simple" that may imply the feature is easy when users might find it complex. Reword the description to avoid this term while keeping the meaning clear.
🔎 Proposed revision
- | `simple_aggregation` | Enable simple aggregation to combine multiple records into single API calls. This reduces the number of requests and can improve throughput. | `false` | + | `simple_aggregation` | Enable record aggregation to combine multiple records into single API calls. This reduces the number of requests and can improve throughput. | `false` |
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pipeline/outputs/firehose.mdpipeline/outputs/kinesis.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T14:30:18.461Z
Learnt from: kalavt
Repo: fluent/fluent-bit-docs PR: 2294
File: pipeline/inputs/kafka.md:147-168
Timestamp: 2025-12-12T14:30:18.461Z
Learning: In Fluent Bit v4.0.4+, when using AWS MSK IAM authentication (rdkafka.sasl.mechanism: aws_msk_iam), the rdkafka.security.protocol is automatically set to SASL_SSL and the AWS region is auto-detected from the broker hostname for standard MSK endpoints. The aws_msk_iam_cluster_arn parameter was removed - users only need to set rdkafka.sasl.mechanism: aws_msk_iam (and optionally aws_region for custom DNS/PrivateLink).
Applied to files:
pipeline/outputs/kinesis.mdpipeline/outputs/firehose.md
🪛 GitHub Actions: Lint PRs
pipeline/outputs/kinesis.md
[warning] 27-27: Vale: FluentBit.Simplicity - Avoid words like 'simple' that imply ease of use.
pipeline/outputs/firehose.md
[warning] 28-28: Vale: FluentBit.Simplicity - Avoid words like 'simple' that imply ease of use.
🪛 GitHub Check: runner / vale
pipeline/outputs/kinesis.md
[warning] 27-27:
[vale] reported by reviewdog 🐶
[FluentBit.Simplicity] Avoid words like "simple" that imply ease of use, because the user may find this action difficult.
Raw Output:
{"message": "[FluentBit.Simplicity] Avoid words like "simple" that imply ease of use, because the user may find this action difficult.", "location": {"path": "pipeline/outputs/kinesis.md", "range": {"start": {"line": 27, "column": 33}}}, "severity": "WARNING"}
pipeline/outputs/firehose.md
[warning] 28-28:
[vale] reported by reviewdog 🐶
[FluentBit.Simplicity] Avoid words like "simple" that imply ease of use, because the user may find this action difficult.
Raw Output:
{"message": "[FluentBit.Simplicity] Avoid words like "simple" that imply ease of use, because the user may find this action difficult.", "location": {"path": "pipeline/outputs/firehose.md", "range": {"start": {"line": 28, "column": 33}}}, "severity": "WARNING"}
🪛 LanguageTool
pipeline/outputs/kinesis.md
[grammar] ~26-~26: Use a hyphen to join words.
Context: ... ARN of an IAM role to assume (for cross account access). | none | | `simple_ag...
(QB_NEW_EN_HYPHEN)
pipeline/outputs/firehose.md
[style] ~23-~23: To form a complete sentence, be sure to include a subject.
Context: ...Specify an external ID for the STS API. Can be used with the role_arn parameter i...
(MISSING_IT_THERE)
[grammar] ~27-~27: Use a hyphen to join words.
Context: ... ARN of an IAM role to assume (for cross account access). | none | | `simple_ag...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (7)
pipeline/outputs/kinesis.md (3)
27-27: Address Vale style warning onsimple_aggregationparameter.The word "simple" in the parameter description triggers the FluentBit.Simplicity style rule, which suggests avoiding language that implies ease of use. Consider rephrasing to be more neutral and descriptive of the actual behavior (e.g., "Aggregate" or "Batch aggregation").
This is flagged in the pipeline (Vale warning), and while it's a style preference rather than a functional issue, you may want to rephrase to improve compliance with project style guidelines.
15-32: Configuration table changes look good.The parameter table correctly reflects:
- Alphabetical ordering per project rules
- New
simple_aggregation,endpoint,external_id,log_key,port,sts_endpoint, andtime_key_formatparameters documented- Correct default value for
time_keyupdated to_none_(from 'false')- Appropriate descriptions for each parameter
68-71: fluent-bit.conf example properly uses Title_Case.The configuration example has been correctly updated to use Title_Case for parameter names (
Region,Stream), aligning with the PR objectives and Fluent Bit configuration file conventions.pipeline/outputs/firehose.md (4)
66-72: Configuration examples properly use Title_Case and formatting.Both .conf configuration examples correctly use Title_Case for parameter names (Name, Match, Region, Delivery_Stream, Workers) and maintain proper alignment and consistency.
Also applies to: 118-124
17-32: Configuration table meets PR objectives.The configuration parameter table successfully:
- Adds the
simple_aggregationparameter as per issue #2316- Maintains alphabetical ordering of all entries
- Documents new parameters (auto_retry_requests, compression, log_key, role_arn, sts_endpoint, time_key_format, workers) with clear descriptions and defaults
- Corrects
time_keydefault to_none_as intendedAside from the grammar/style refinements flagged separately, the parameter documentation is comprehensive and well-structured.
17-32: Configuration table successfully addresses all PR objectives.The parameter table:
- ✓ Adds
simple_aggregationas primary objective (line 28, defaultfalse)- ✓ Maintains alphabetical ordering across all entries
- ✓ Documents new parameters comprehensively (auto_retry_requests, compression, log_key, role_arn, sts_endpoint, time_key_format, workers)
- ✓ Corrects
time_keydefault value to_none_Structure and descriptions are clear; aside from the grammar refinements flagged separately, the documentation is well-formatted.
66-72: Configuration examples correctly apply Title_Case formatting.Both
.confconfiguration blocks properly use Title_Case for parameter names (Name, Match, Region, Delivery_Stream, Workers) with correct alignment. YAML examples use lowercase appropriately for that format. Formatting is consistent across examples.Also applies to: 118-124
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
pipeline/outputs/firehose.md (2)
23-23: Reword fragment sentence to form a complete sentence.The description for
external_idcontains an incomplete sentence: "Can be used with therole_arnparameter..." lacks a subject. This was flagged in a previous review and remains unresolved.🔎 Proposed fix
- | `external_id` | Specify an external ID for the STS API. Can be used with the `role_arn` parameter if your role requires an external ID. | _none_ | + | `external_id` | Specify an external ID for the STS API. It can be used with the `role_arn` parameter if your role requires an external ID. | _none_ |
27-27: Hyphenate compound adjective "cross-account".The phrase "cross account access" should use a hyphen to form the compound adjective. This was flagged in a previous review and remains unresolved.
🔎 Proposed fix
- | `role_arn` | ARN of an IAM role to assume (for cross account access). | _none_ | + | `role_arn` | ARN of an IAM role to assume (for cross-account access). | _none_ |pipeline/outputs/kinesis.md (1)
26-26: Hyphenate compound adjective "cross-account".The phrase "cross account access" should use a hyphen to form the compound adjective. This was flagged in a previous review and remains unresolved.
🔎 Proposed fix
- | `role_arn` | ARN of an IAM role to assume (for cross account access). | _none_ | + | `role_arn` | ARN of an IAM role to assume (for cross-account access). | _none_ |
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pipeline/outputs/firehose.mdpipeline/outputs/kinesis.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T14:30:18.461Z
Learnt from: kalavt
Repo: fluent/fluent-bit-docs PR: 2294
File: pipeline/inputs/kafka.md:147-168
Timestamp: 2025-12-12T14:30:18.461Z
Learning: In Fluent Bit v4.0.4+, when using AWS MSK IAM authentication (rdkafka.sasl.mechanism: aws_msk_iam), the rdkafka.security.protocol is automatically set to SASL_SSL and the AWS region is auto-detected from the broker hostname for standard MSK endpoints. The aws_msk_iam_cluster_arn parameter was removed - users only need to set rdkafka.sasl.mechanism: aws_msk_iam (and optionally aws_region for custom DNS/PrivateLink).
Applied to files:
pipeline/outputs/kinesis.mdpipeline/outputs/firehose.md
🪛 LanguageTool
pipeline/outputs/kinesis.md
[grammar] ~26-~26: Use a hyphen to join words.
Context: ... ARN of an IAM role to assume (for cross account access). | none | | `simple_ag...
(QB_NEW_EN_HYPHEN)
pipeline/outputs/firehose.md
[style] ~23-~23: To form a complete sentence, be sure to include a subject.
Context: ...Specify an external ID for the STS API. Can be used with the role_arn parameter i...
(MISSING_IT_THERE)
[grammar] ~27-~27: Use a hyphen to join words.
Context: ... ARN of an IAM role to assume (for cross account access). | none | | `simple_ag...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (4)
pipeline/outputs/firehose.md (2)
19-32: Configuration parameters are properly alphabetized and documented.The expanded parameter table includes the newly added
simple_aggregation,compression,auto_retry_requests,log_key,sts_endpoint,time_key_format, andworkersparameters, all correctly sorted alphabetically. Descriptions and defaults are clear.
68-71: Configuration examples use consistent Title_Case formatting.Both
.confexamples in the "Get started" and "Worker support" sections correctly use Title_Case for parameter names (Name,Match,Region,Delivery_Stream,Workers), aligning with project conventions.Also applies to: 119-123
pipeline/outputs/kinesis.md (2)
19-32: Configuration parameters are properly alphabetized and documented.The expanded parameter table includes the newly added
simple_aggregation,endpoint,external_id,log_key,port,sts_endpoint,time_key,time_key_format, andworkersparameters, all correctly sorted alphabetically. Thetime_keydefault is correctly set to_none_per PR objectives.
68-71: Configuration example uses consistent Title_Case formatting.The
.confexample in the "Get started" section correctly uses Title_Case for parameter names (Name,Match,Region,Stream), aligning with project conventions.
eschabell
force-pushed
the
erics_kinesis_update_422
branch
2 times, most recently
from
c3bbbb1 to
59b2a58
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pipeline/outputs/kinesis.md (1)
26-26: Fix hyphenation in compound adjective for role_arn description.The phrase "cross account access" should use a hyphen to form the compound adjective "cross-account access". This was flagged in a previous review and needs to be applied here as it was in firehose.md.
🔎 Proposed fix
-| `role_arn` | ARN of an IAM role to assume (for cross account access). | _none_ | +| `role_arn` | ARN of an IAM role to assume (for cross-account access). | _none_ |
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pipeline/outputs/firehose.mdpipeline/outputs/kinesis.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T14:30:18.461Z
Learnt from: kalavt
Repo: fluent/fluent-bit-docs PR: 2294
File: pipeline/inputs/kafka.md:147-168
Timestamp: 2025-12-12T14:30:18.461Z
Learning: In Fluent Bit v4.0.4+, when using AWS MSK IAM authentication (rdkafka.sasl.mechanism: aws_msk_iam), the rdkafka.security.protocol is automatically set to SASL_SSL and the AWS region is auto-detected from the broker hostname for standard MSK endpoints. The aws_msk_iam_cluster_arn parameter was removed - users only need to set rdkafka.sasl.mechanism: aws_msk_iam (and optionally aws_region for custom DNS/PrivateLink).
Applied to files:
pipeline/outputs/firehose.mdpipeline/outputs/kinesis.md
🪛 LanguageTool
pipeline/outputs/kinesis.md
[grammar] ~26-~26: Use a hyphen to join words.
Context: ... ARN of an IAM role to assume (for cross account access). | none | | `simple_ag...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (3)
pipeline/outputs/kinesis.md (1)
27-27: Approve simple_aggregation parameter addition.The
simple_aggregationparameter is correctly positioned in alphabetical order with clear description and default value.pipeline/outputs/firehose.md (2)
20-20: Verify compression parameter documentation matches implementation.The
compressionparameter is newly documented with support forgzipandarrow. Ensure the parameter names and compile-time availability notes match the actual Fluent Bit implementation for this 4.2.2 release.
28-28: Approve simple_aggregation parameter addition.The
simple_aggregationparameter is correctly positioned in alphabetical order with clear description and default value, addressing the PR objective for the 4.2.2 release.
…d cleanup - Add missing simple_aggregation configuration parameter to both kinesis_streams and kinesis_firehose output plugin documentation - Sort all configuration parameters alphabetically per project rules - Update .conf examples to use Title_Case for parameters - Fix minor typos: extra space in region, stray backtick in role_arn, period instead of comma in compression values list - Fix time_key default value from 'false' to '_none_' in kinesis.md Fixes fluent#2316. Signed-off-by: Eric D. Schabell <eric@schabell.org>
eschabell
force-pushed
the
erics_kinesis_update_422
branch
from
59b2a58 to
4630cb7
Compare
1 participant
Fixes #2316.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.