Skip to content

out_forward: fix retain_metadata_in_forward_mode default value (true)#11756

Open
edsiper wants to merge 2 commits intomasterfrom
forward-metadata-fix
Open

out_forward: fix retain_metadata_in_forward_mode default value (true)#11756
edsiper wants to merge 2 commits intomasterfrom
forward-metadata-fix

Conversation

@edsiper
Copy link
Copy Markdown
Member

@edsiper edsiper commented Apr 28, 2026

This fixes a gap in Fluent Bit-to-Fluent Bit forwarding where log event metadata could be silently dropped by default in out_forward forward mode.

Fluent Bit’s current log event model includes metadata, so forwarding between Fluent Bit instances should preserve that metadata unless the user explicitly opts out for strict Forward-compatible records. This change updates retain_metadata_in_forward_mode to default to true while preserving the existing opt-out behavior with retain_metadata_in_forward_mode false.

The integration coverage adds a Fluent Bit sender and receiver chain and verifies that metadata is preserved by default, preserved when explicitly enabled, and dropped when explicitly disabled.

Tests:

  • tests/integration/.venv/bin/python -m pytest tests/integration/scenarios/out_forward/tests/
    test_out_forward_metadata_001.py -q
  • VALGRIND=1 VALGRIND_STRICT=1 tests/integration/.venv/bin/python -m pytest tests/integration/
    scenarios/out_forward/tests/test_out_forward_metadata_001.py -q

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

  • Configuration Changes

    • Forward output plugin default behavior changed to retain metadata in Forward mode. This affects how records are forwarded to downstream systems by default. Users can explicitly disable metadata retention in their configurations to restore the previous behavior if needed.
  • Tests

    • Added comprehensive integration test scenarios for metadata handling in Forward mode.

edsiper added 2 commits April 28, 2026 10:12
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@edsiper edsiper requested a review from cosmo0920 as a code owner April 28, 2026 16:15
@edsiper edsiper added this to the Fluent Bit v5.0.4 milestone Apr 28, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

The default value of the retain_metadata_in_forward_mode option in the forward output plugin is changed from disabled to enabled. Comprehensive integration tests are added to validate metadata preservation and dropout behavior across different configurations.

Changes

Cohort / File(s) Summary
Plugin configuration default
plugins/out_forward/forward.c
Flips default value of retain_metadata_in_forward_mode from disabled to enabled, changing initial runtime behavior in Forward mode.
Test configurations
tests/integration/scenarios/out_forward/config/out_forward_metadata_receiver.yaml, out_forward_metadata_sender_default.yaml, out_forward_metadata_sender_false.yaml, out_forward_metadata_sender_true.yaml
Adds four new YAML configurations for integration testing: receiver/capture endpoint, and three sender variants with different metadata retention settings (default, explicitly disabled, explicitly enabled).
Integration test
tests/integration/scenarios/out_forward/tests/test_out_forward_metadata_001.py
Comprehensive integration test that coordinates sender and receiver instances, constructs Forward protocol messages with metadata payloads, validates message capture, and asserts metadata is preserved or dropped based on configuration.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Integration Test
    participant Sender as FluentBit Sender
    participant Receiver as FluentBit Receiver
    participant Capture as Forward Server
    
    Test->>Sender: Start (default/true/false config)
    Test->>Receiver: Start
    Test->>Capture: Start capture server
    
    Note over Test: Construct Forward message<br/>with TEST_METADATA
    Test->>Sender: Send message to sender port
    
    Sender->>Receiver: Forward message<br/>(metadata retained or dropped)
    Receiver->>Capture: Send to capture endpoint
    
    Note over Capture: Store message in data_storage
    
    Test->>Capture: Poll for forwarded messages
    Capture-->>Test: Return captured messages
    
    alt Metadata Preserved Config
        Test->>Test: Assert record[metadata]<br/>contains TEST_METADATA
    else Metadata Dropped Config
        Test->>Test: Assert metadata<br/>is empty/absent
    end
    
    Test->>Sender: Stop
    Test->>Receiver: Stop
    Test->>Capture: Stop & cleanup
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

docs-required

Poem

🐰 With whiskers twitching, metadata hops,
Now forward mode keeps what it shops,
From sender to receiver, true and bright,
The rabbit's default shines just right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: enabling the retain_metadata_in_forward_mode default value to true in the out_forward plugin, which is the primary focus of this changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch forward-metadata-fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a 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: cf58f73f7b

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/out_forward/forward.c
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
tests/integration/scenarios/out_forward/tests/test_out_forward_metadata_001.py (2)

42-52: Reset capture storage at test-run start to enforce isolation.

forward_data_storage["messages"] is shared/global; clearing it in start() avoids stale messages from prior runs affecting assertions.

♻️ Suggested patch
 def start(self):
+        forward_data_storage["messages"].clear()
         self.sender_port = find_available_port()
         self.receiver_port = find_available_port()
         self.capture_port = find_available_port()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@tests/integration/scenarios/out_forward/tests/test_out_forward_metadata_001.py`
around lines 42 - 52, The test setup's start() does not clear shared
forward_data_storage["messages"], risking test pollution; modify start() to
reset forward_data_storage["messages"] (e.g., set to empty list or appropriate
empty structure) before calling forward_server_run(self.capture_port) so each
test run starts with an isolated storage state; locate this in the start method
alongside the existing _set_env calls and ensure the reset happens prior to
starting the server.

56-56: Replace fixed sleep with readiness-based synchronization.

A hardcoded time.sleep(1) can make this test flaky under slower environments; prefer waiting for receiver readiness instead of elapsed time.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@tests/integration/scenarios/out_forward/tests/test_out_forward_metadata_001.py`
at line 56, Replace the hardcoded time.sleep(1) in test_out_forward_metadata_001
with readiness-based synchronization: remove time.sleep(1) and instead poll a
readiness predicate (for example call receiver.is_ready() or use a helper like
wait_for_port(host, port, timeout=5) or receiver.wait_until_ready(timeout=5)) in
a short loop until success or timeout; this keeps the test stable across slow
environments and uses explicit readiness semantics rather than a fixed sleep.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@tests/integration/scenarios/out_forward/tests/test_out_forward_metadata_001.py`:
- Around line 42-52: The test setup's start() does not clear shared
forward_data_storage["messages"], risking test pollution; modify start() to
reset forward_data_storage["messages"] (e.g., set to empty list or appropriate
empty structure) before calling forward_server_run(self.capture_port) so each
test run starts with an isolated storage state; locate this in the start method
alongside the existing _set_env calls and ensure the reset happens prior to
starting the server.
- Line 56: Replace the hardcoded time.sleep(1) in test_out_forward_metadata_001
with readiness-based synchronization: remove time.sleep(1) and instead poll a
readiness predicate (for example call receiver.is_ready() or use a helper like
wait_for_port(host, port, timeout=5) or receiver.wait_until_ready(timeout=5)) in
a short loop until success or timeout; this keeps the test stable across slow
environments and uses explicit readiness semantics rather than a fixed sleep.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9b7592b-64ce-4de7-94c0-0bffe5d13bb9

📥 Commits

Reviewing files that changed from the base of the PR and between 230eb3c and cf58f73.

📒 Files selected for processing (6)
  • plugins/out_forward/forward.c
  • tests/integration/scenarios/out_forward/config/out_forward_metadata_receiver.yaml
  • tests/integration/scenarios/out_forward/config/out_forward_metadata_sender_default.yaml
  • tests/integration/scenarios/out_forward/config/out_forward_metadata_sender_false.yaml
  • tests/integration/scenarios/out_forward/config/out_forward_metadata_sender_true.yaml
  • tests/integration/scenarios/out_forward/tests/test_out_forward_metadata_001.py

Copy link
Copy Markdown
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, out_forward tests are good but group_semantics is failing. So, we need to fix it:

[731](https://github.com/fluent/fluent-bit/actions/runs/25064403993/job/73427376837?pr=11756#step:8:4732)
        Start  82: flb-rt-group_counter_semantics
 79/185 Test  #82: flb-rt-group_counter_semantics ...................***Failed   12.00 sec
Test forward_group_size_default...              [ FAILED ]
  group_counter_semantics.c:661: Check get_forward_size() == 1... failed
Test forward_group_size_retain_metadata...      [ OK ]
Test forward_group_size_retain_metadata_upstream_node... [ OK ]
Test loki_group_values_count...                 [ OK ]
Test stackdriver_group_entries_count...         [ OK ]
Test forward_output_processor_mixed_payload_smoke... [ OK ]
FAILED: 1 of 6 unit tests has failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants