out_loki: Add descriptions about tenant_id and its around behavior#2581
out_loki: Add descriptions about tenant_id and its around behavior#2581cosmo0920 wants to merge 2 commits into
Conversation
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR updates the Loki output documentation to add ChangesTenant ID Routing Feature Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pipeline/outputs/loki.md (2)
314-320: 💤 Low valueConsider explaining the
remove_keysusage in examples.Both configuration examples include
remove_keys: tenant_idwithout explanation. Adding a brief note would help users understand that removing the tenant ID key after routing is a recommended practice to avoid storing redundant data in Loki.📝 Suggested addition
Add after line 320:
Use `remove_keys` to remove the tenant ID field from the record after routing to avoid storing redundant data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/outputs/loki.md` around lines 314 - 320, The examples use remove_keys: tenant_id but don't explain it; add a brief sentence after the tenant routing section clarifying that remove_keys is used to strip the tenant ID field from records after routing to avoid storing redundant tenant metadata in Loki. Reference the existing config options tenant_id_key and remove_keys so readers know this is applied after tenant routing; keep the note short and place it near the tenant_id_key / tenant_id_key_error_handling explanation.
324-349: ⚡ Quick winAdd
labelsparameter to configuration examples.The examples should include the
labelsparameter for clarity and consistency with other examples in this documentation. While the default (job=fluent-bit) may apply, explicit configuration helps users understand the complete setup and ensures records are properly grouped into Loki streams.📝 Suggested enhancement
pipeline: outputs: - name: loki match: '*' host: 127.0.0.1 port: 3100 + labels: job=fluentbit tenant_id_key: tenant_id tenant_id_key_error_handling: partial_error remove_keys: tenant_id[OUTPUT] Name loki Match * Host 127.0.0.1 Port 3100 + Labels job=fluentbit Tenant_ID_Key tenant_id Tenant_ID_Key_Error_Handling partial_error Remove_Keys tenant_id🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/outputs/loki.md` around lines 324 - 349, Add an explicit labels parameter to both example configurations so records are grouped into Loki streams; in the YAML pipeline.outputs.loki example add a labels mapping (e.g., labels: { job: fluent-bit } or labels: job: fluent-bit) and in the fluent-bit [OUTPUT] loki block add the corresponding Labels entry (e.g., Labels job=fluent-bit) alongside Tenant_ID_Key, Tenant_ID_Key_Error_Handling, and Remove_Keys to make the examples complete and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pipeline/outputs/loki.md`:
- Line 32: The setting description for tenant_id_key_error_handling needs
explicit data-consistency warnings: update the doc text for
tenant_id_key_error_handling to state that choosing `partial_success` will drop
records for failed tenants (potential data loss) and that choosing
`partial_error` will retry the whole chunk which may resend records for
successful tenants (potential duplication); ensure the entries for
`partial_success` and `partial_error` include these two short warnings so users
can weigh loss vs. duplication when selecting a mode.
- Around line 314-320: Add a short note in the Loki output docs explaining the
behavior when tenant_id_key is missing/empty and how it interacts with the
static tenant_id: specify that if tenant_id_key resolves to missing/empty for a
record, Fluent Bit will fall back to the static tenant_id (if set) and send
X-Scope-OrgID=tenant_id; if tenant_id_key is missing/empty and tenant_id is also
omitted/empty, the record is treated as single-tenant with no X-Scope-OrgID
header (no tenant) rather than failing/dropping; also mention this behavior
applies when using tenant_id_key_error_handling (partial_success/partial_error)
so readers know fallback records count toward the per-tenant requests.
---
Nitpick comments:
In `@pipeline/outputs/loki.md`:
- Around line 314-320: The examples use remove_keys: tenant_id but don't explain
it; add a brief sentence after the tenant routing section clarifying that
remove_keys is used to strip the tenant ID field from records after routing to
avoid storing redundant tenant metadata in Loki. Reference the existing config
options tenant_id_key and remove_keys so readers know this is applied after
tenant routing; keep the note short and place it near the tenant_id_key /
tenant_id_key_error_handling explanation.
- Around line 324-349: Add an explicit labels parameter to both example
configurations so records are grouped into Loki streams; in the YAML
pipeline.outputs.loki example add a labels mapping (e.g., labels: { job:
fluent-bit } or labels: job: fluent-bit) and in the fluent-bit [OUTPUT] loki
block add the corresponding Labels entry (e.g., Labels job=fluent-bit)
alongside Tenant_ID_Key, Tenant_ID_Key_Error_Handling, and Remove_Keys to make
the examples complete and consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
fb8b4f5 to
28e0df7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pipeline/outputs/loki.md`:
- Line 32: Update the wording in the description for the
`tenant_id_key_error_handling` option: replace "may result in duplicate records
for successful tenants" with either "might result in duplicate records for
successful tenants" or "can result in duplicate records for successful tenants"
to satisfy the Vale linter; ensure you edit the table cell that contains the
`tenant_id_key_error_handling` entry so the rest of the sentence and formatting
remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
eschabell
left a comment
There was a problem hiding this comment.
Good to go once code change merged.
The corresponding PR is fluent/fluent-bit#11832.
Summary by CodeRabbit
tenant_id_key_error_handlingconfig description outliningpartial_successandpartial_errorbehaviors.tenant_id_key" guide explaining dynamic tenant routing, fallback behavior whentenant_id_keyis missing/empty, and how fallback interacts with error handling.