alias: handle input, output, filter, and processor plugin aliases#11656
Open
cosmo0920 wants to merge 11 commits into
Open
alias: handle input, output, filter, and processor plugin aliases#11656cosmo0920 wants to merge 11 commits into
cosmo0920 wants to merge 11 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a plugin-alias subsystem: public header and implementation for alias lookup and URI rewriting, integrates alias handling into output creation and network parsing, updates build/test files, and adds unit tests for alias lookup, rewrite, parsing, and instantiation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OutputLayer as Output Layer
participant AliasResolver as Alias Resolver
participant NetworkLayer as Network Layer
participant Plugin as Plugin
User->>OutputLayer: flb_output_new(config, "elasticsearch")
OutputLayer->>AliasResolver: flb_plugin_alias_rewrite(FLB_PLUGIN_OUTPUT, "elasticsearch://host:port")
AliasResolver->>AliasResolver: extract scheme/alias, lookup "elasticsearch"
AliasResolver-->>OutputLayer: "es://host:port" or FLB_PLUGIN_ALIAS_ERR or NULL
alt rewritten URI returned
OutputLayer->>NetworkLayer: flb_net_host_set("es://host:port")
else no rewrite (NULL)
OutputLayer->>NetworkLayer: flb_net_host_set("elasticsearch://host:port")
else FLB_PLUGIN_ALIAS_ERR
OutputLayer-->>User: return error (cleanup and NULL)
end
NetworkLayer->>NetworkLayer: detect "://", parse host and port
NetworkLayer-->>OutputLayer: host/port populated
OutputLayer->>Plugin: instantiate plugin with canonical name "es"
Plugin-->>User: plugin instance returned
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
cosmo0920
force-pushed
the
comso0920-handle-plugin-aliases
branch
from
f3f7908 to
7394a1c
Compare
cosmo0920
force-pushed
the
comso0920-handle-plugin-aliases
branch
from
7394a1c to
4efcaf7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@include/fluent-bit/flb_plugin_alias.h`:
- Around line 25-27: flb_plugin_alias_rewrite currently returns NULL for both
“no rewrite needed” and allocation/build failures; change its API and
implementation so callers can distinguish errors: return NULL only for “no
rewrite”, return a distinct sentinel pointer (e.g. (char *)-1 or a named macro
like FLB_PLUGIN_ALIAS_ERR) to indicate allocation/build failure, and continue
returning a real allocated string on success; update callers (e.g. the code that
calls flb_plugin_alias_rewrite in flb_output.c) to check for the sentinel and
treat it as an error (fail cleanly) instead of falling back to the original
plugin_reference, and document the new return contract next to the
flb_plugin_alias_rewrite and flb_plugin_alias_get declarations.
In `@src/flb_network.c`:
- Around line 178-184: In flb_net_host_set make scheme detection strict: do not
use strstr(address, "://") which can match "://" anywhere; instead find the
first ':' with strchr(address, ':'), verify it is not at address (non-empty
scheme) and that the following two chars are '/' and '/' (i.e. separator[1]=='/'
&& separator[2]=='/'), and only then set s = separator + 3; otherwise fall back
to s = address + len. Update the code around variables separator, address, and s
in flb_net_host_set accordingly so only a proper leading "scheme://" is
accepted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c181e7c-7d06-484f-933f-359991ea075c
📒 Files selected for processing (7)
include/fluent-bit/flb_plugin_alias.hsrc/CMakeLists.txtsrc/flb_network.csrc/flb_output.csrc/flb_plugin_alias.ctests/internal/CMakeLists.txttests/internal/plugin_alias.c
cosmo0920
force-pushed
the
comso0920-handle-plugin-aliases
branch
from
4efcaf7 to
dd91460
Compare
Contributor
Author
Currently, I implemented only for out_es plugin like: struct flb_plugin_alias_entry {
int plugin_type;
const char *alias_name;
const char *plugin_name;
};
/*
* Table that maps user-facing aliases to plugin short names.
*
* Keep this table focused on backwards/forwards compatibility names where the
* historical short name is still used internally by the plugin implementation.
*/
static struct flb_plugin_alias_entry plugin_aliases[] = {
{
FLB_PLUGIN_OUTPUT,
"elasticsearch",
"es"
},
{
0,
NULL,
NULL
}
};Just interpreted as es to be elasticsearch on loading output plugins. |
Contributor
But ultimately as long as we provide the aliases struct we are ok to use for any plugin? |
Contributor
Author
On paper, we'll be able to provide any type of the plugins of aliases but in this implementation we only provide a mechanism of output plugin alias. |
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
cosmo0920
force-pushed
the
comso0920-handle-plugin-aliases
branch
from
b175d14 to
ba60204
Compare
cosmo0920
changed the title
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We can instantiate out_es as output elasticsearch plugin but the input of elasticsearch plugin is in_elasticsearch.
To maintain naming coherence, we need to handle plugin alias at least of out_es as out_elasticsearch.
With this patch, we can instantiate out_elasticsearch as out_es.
Closes #9261.
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:
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
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
Improvements
Bug Fixes
Tests