Repository navigation
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Config contract: struct fields and config_map entry plugins/in_podman_metrics/podman_metrics_config.h, plugins/in_podman_metrics/podman_metrics.h |
flb_in_metrics gains fields for the regex text and compiled regex. The config map connects exclude_name_regex to the text field. |
Regex initialization, filtering, and cleanup plugins/in_podman_metrics/podman_metrics.c |
The plugin compiles the configured regex during initialization and returns an error if compilation fails. Container collection skips names that match the regex. The plugin destroys the compiled regex if collector setup fails or when the plugin exits. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Feature
Merge Risk: 
6e797
-
A rare initialization failure can leak plugin context memory; free the context on collector setup failure before merging.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "in_podman_metrics: add exclude_name_regex option" names the main change. The PR adds an optional exclude_name_regex setting that skips containers whose names match the regex before their me… Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "in_podman_metrics: add exclude_name_regex option" names the main change. The PR adds an optional exclude_name_regex setting that skips containers whose names match the regex before their metrics are collected. The title is short, specific, and uses the plugin name and option name the changeset uses. It does not describe the compile, cleanup, or documentation details, which is acceptable.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
Companion documentation PR: fluent/fluent-bit-docs#2604 Testing evidenceBuilt via Yocto (Fluent Bit 5.0.2 base) and run on an aarch64 Linux device pipeline:
inputs:
- name: podman_metrics
scrape_interval: 60
scrape_on_start: true
exclude_name_regex: '-(infra|service)$'With 9 containers running (3 application + 6 pod No I can add Valgrind output from a dedicated local build if that is required for (This PR supersedes #11968, which was accidentally orphaned by a force-push from |
Document the new exclude_name_regex configuration option for the in_podman_metrics input plugin, added in fluent/fluent-bit#11970. The option skips containers whose name matches a user-supplied regular expression before their metrics are collected (for example pod infra and service containers). Signed-off-by: Stefano Tondo <stondo@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@plugins/in_podman_metrics/podman_metrics.c`:
- Around line 463-470: The error handling path for the collector setup failure
(when coll_fd_runtime equals -1) destroys the exclude_name_regex but fails to
free the ctx structure itself before returning. Add a call to free the ctx
structure after destroying the regex to ensure proper cleanup and prevent memory
leaks. Check other error paths in the same function to match the cleanup pattern
used elsewhere when releasing ctx on initialization failures.
🪄 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: 9b161edf-6e8e-421c-96d0-17db3d76e49b
📒 Files selected for processing (3)
plugins/in_podman_metrics/podman_metrics.cplugins/in_podman_metrics/podman_metrics.hplugins/in_podman_metrics/podman_metrics_config.h
| flb_input_set_context(in, ctx); | ||
| coll_fd_runtime = flb_input_set_collector_time(in, cb_metrics_collect_runtime, ctx->scrape_interval, 0, config); | ||
| if (coll_fd_runtime == -1) { | ||
| flb_plg_error(ctx->ins, "Could not set collector for podman metrics plugin"); | ||
| if (ctx->exclude_name_regex) { | ||
| flb_regex_destroy(ctx->exclude_name_regex); | ||
| } | ||
| return -1; |
There was a problem hiding this comment.
Free plugin context on collector setup failure.
Line 465-470 returns after destroying the compiled regex, but ctx is not released on this error path. That makes init cleanup inconsistent with other failure branches and can leak initialization state.
Suggested fix
- flb_input_set_context(in, ctx);
coll_fd_runtime = flb_input_set_collector_time(in, cb_metrics_collect_runtime, ctx->scrape_interval, 0, config);
if (coll_fd_runtime == -1) {
flb_plg_error(ctx->ins, "Could not set collector for podman metrics plugin");
if (ctx->exclude_name_regex) {
flb_regex_destroy(ctx->exclude_name_regex);
}
+ flb_free(ctx);
return -1;
}
ctx->coll_fd_runtime = coll_fd_runtime;
+ flb_input_set_context(in, ctx);🤖 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 `@plugins/in_podman_metrics/podman_metrics.c` around lines 463 - 470, The error
handling path for the collector setup failure (when coll_fd_runtime equals -1)
destroys the exclude_name_regex but fails to free the ctx structure itself
before returning. Add a call to free the ctx structure after destroying the
regex to ensure proper cleanup and prevent memory leaks. Check other error paths
in the same function to match the cleanup pattern used elsewhere when releasing
ctx on initialization failures.
|
CodeRabbit flagged on the previous iteration (#11968) that the compiled |
|
Friendly ping. This has been approved by @bachp since early July and CI is green. Is anything else needed from my side before it can be merged? The companion docs change is fluent/fluent-bit-docs#2604. |
|
Second gentle ping. This has been approved by @bachp since early July and CI is green; the companion docs change is fluent/fluent-bit-docs#2604, which is waiting on this one to merge. Is anything still needed from my side? Related, in case it helps a reviewer batch the plugin work: #11719 is still open, and I filed #12391 today for a separate issue where a failing initial scrape with |
|
@cosmo0920 This one adds an |
Add an optional 'exclude_name_regex' configuration option to skip containers whose name matches a user-supplied regular expression before their metrics are collected and emitted. This mirrors the regex filtering model already used by the node exporter (netdev.ignore_device_regex) and the diskstats/filesystem collectors. A typical use is dropping Podman pod infrastructure containers, e.g. exclude_name_regex '-(infra|service)$' to skip pause and catatonit containers that carry no useful application metrics. The option defaults to unset, so existing behaviour (collect every container) is unchanged. Signed-off-by: Stefano Tondo <stondo@gmail.com>
5781050 to
6e797ed
Compare
Summary
Add an optional
exclude_name_regexconfiguration option toin_podman_metricsthat skips containers whose name matches a user-supplied regular expression,
before their metrics are collected and emitted.
This mirrors the regex-filtering model already used elsewhere in Fluent Bit,
e.g.
netdev.ignore_device_regexinin_node_exporter_metrics. A typical useis dropping Podman pod infrastructure containers, for example
exclude_name_regex '-(infra|service)$'to skip the pause and catatonitcontainers that carry no useful application metrics.
The option defaults to unset, so existing behaviour (collect every container)
is unchanged and the feature is fully opt-in.
Example configuration
Implementation
FLB_CONFIG_MAP_STRoptionexclude_name_regex(defaultNULL).in_metrics_initviaflb_regex_create.cleanly before
flb_input_set_context/flb_input_set_collector_time, sono collector is ever armed against an uninitialised context. The compiled
regex is also released on the later init error paths (collector setup and
initial scrape failures) so it never leaks on a failed startup.
collect_container_data, a container is skipped when its name matchesthe compiled regex; otherwise it is added and counted as before.
in_metrics_exit.Testing
exclude_name_regex '-(infra|service)$': only application containersemit metrics; pod
-infra/-service(pause/catatonit) containers areexcluded at the source. Verified via the prometheus exporter output.
container emitted).
could not compile exclude_name_regex '<value>',no crash and no armed collector.
Documentation
fluent-bit-docsdocuments the newexclude_name_regexoption.Signed-off-by: Stefano Tondo stondo@gmail.com
Summary by CodeRabbit
Release Notes
exclude_name_regexconfiguration option to omit Podman containers with matching names from metrics collection. Containers that do not match the configured pattern continue to be collected.