-
Notifications
You must be signed in to change notification settings - Fork 544
pipeline: router: Only logs are currently supported #2263
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
Conversation
WalkthroughA documentation update to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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.
Signed-off-by: lecaros <[email protected]>
2cdb6bb to
885c5e2
Compare
| Conditional routing supports different signal types to route logs, metrics, and traces separately: | ||
|
|
||
| | Signal | Description | | ||
| | --- | --- | | ||
| | `logs` | Routes log records | | ||
| | `metrics` | Routes metric records | | ||
| | `traces` | Routes trace records | | ||
| | `any` | Routes all signal types | | ||
| | Signal | Description | Supported | | ||
| | --- | --- |---| | ||
| | `logs` | Routes log records | Yes | | ||
| | `metrics` | Routes metric records | No | | ||
| | `traces` | Routes trace records | No | | ||
| | `any` | Routes all signal types | No | |
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.
I'm still a little bit confused by this--would it help to change the sentence before the table to say something like "Not all signal types support conditional routing"? (right now the way it's phrased makes it sound like something else entirely)
| Conditional routing supports different signal types to route logs, metrics, and traces separately: | ||
|
|
||
| | Signal | Description | | ||
| | --- | --- | | ||
| | `logs` | Routes log records | | ||
| | `metrics` | Routes metric records | | ||
| | `traces` | Routes trace records | | ||
| | `any` | Routes all signal types | | ||
| | Signal | Description | Supported | | ||
| | --- | --- |---| | ||
| | `logs` | Routes log records | Yes | | ||
| | `metrics` | Routes metric records | No | | ||
| | `traces` | Routes trace records | No | | ||
| | `any` | Routes all signal types | No | |
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.
should any be part of this table? (does any refer to a specific tag/label/name of something? if not, I think it makes more sense to remove it)
|
@lecaros are you sure about this, when I have our tooling parse the code base to validate I get this: Based on review of the Fluent Bit source code, the Signal types table is correct. Looking at And the header file So conditional routing supports: The documentation table in https://docs.fluentbit.io/manual/data-pipeline/router#route-multiple-signal-types |
|
@lecaros I dug a bit deeper and see now that they are STUBBED features, so metrics and traces always returns FALSE. The ANY listing supports only LOGS. Good catch.... this needs a bigger update. I'm going to adjust the entire doc as examples need fixing too. I'm going to merge this and then fix the rest of the doc. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.