-
Notifications
You must be signed in to change notification settings - Fork 1.9k
in_event_type: implement pause/resume to stop timed collector. #10843
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
Signed-off-by: Phillip Whelan <phillip.whelan@chronosphere.io>
WalkthroughAdds an input-instance pointer to the event_type context, stores it during init, and implements pause/resume callbacks to control the collector using the stored collector FD and input instance. Updates the plugin descriptor to register these callbacks. Other collection logic remains unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant InputEngine
participant EventTypePlugin as EventType Plugin
participant Collector
rect rgb(235, 245, 255)
note over InputEngine,EventTypePlugin: Initialization
InputEngine->>EventTypePlugin: init(ins)
EventTypePlugin->>EventTypePlugin: ctx->ins = ins
EventTypePlugin->>Collector: register collector -> coll_fd
end
rect rgb(245, 235, 255)
note over InputEngine,EventTypePlugin: Pause lifecycle
InputEngine->>EventTypePlugin: cb_pause()
EventTypePlugin->>Collector: pause(coll_fd, ctx->ins)
Collector-->>EventTypePlugin: paused
end
rect rgb(235, 255, 235)
note over InputEngine,EventTypePlugin: Resume lifecycle
InputEngine->>EventTypePlugin: cb_resume()
EventTypePlugin->>Collector: resume(coll_fd, ctx->ins)
Collector-->>EventTypePlugin: resumed
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugins/in_event_type/event_type.c (3)
456-462: Log and guard pause errors.Currently ignores failures from flb_input_collector_pause; add minimal checks and a warning for observability.
Apply:
-static void cb_event_type_pause(void *data, struct flb_config *config) -{ - struct event_type *ctx = data; - - flb_input_collector_pause(ctx->coll_fd, ctx->ins); -} +static void cb_event_type_pause(void *data, struct flb_config *config) +{ + int ret; + struct event_type *ctx = data; + + if (!ctx) { + return; + } + + ret = flb_input_collector_pause(ctx->coll_fd, ctx->ins); + if (ret != 0 && ctx->ins) { + flb_plg_warn(ctx->ins, "failed to pause collector id=%i", ctx->coll_fd); + } +}
463-469: Mirror resume-side error handling.Same rationale as pause: check return and log failures to aid shutdown diagnostics.
Apply:
-static void cb_event_type_resume(void *data, struct flb_config *config) -{ - struct event_type *ctx = data; - - flb_input_collector_resume(ctx->coll_fd, ctx->ins); -} +static void cb_event_type_resume(void *data, struct flb_config *config) +{ + int ret; + struct event_type *ctx = data; + + if (!ctx) { + return; + } + + ret = flb_input_collector_resume(ctx->coll_fd, ctx->ins); + if (ret != 0 && ctx->ins) { + flb_plg_warn(ctx->ins, "failed to resume collector id=%i", ctx->coll_fd); + } +}
473-476: Fix minor typo in config help text.“optionsa” → “options”.
- "Set the type of event to deliver, optionsa are: logs, metrics or traces" + "Set the type of event to deliver, options are: logs, metrics or traces"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_event_type/event_type.c(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/in_event_type/event_type.c (1)
src/flb_input.c (2)
flb_input_collector_pause(1951-1991)flb_input_collector_resume(2014-2070)
🔇 Additional comments (3)
plugins/in_event_type/event_type.c (3)
46-46: Storing input instance in context looks good.This enables safe pause/resume without depending on external state.
409-409: Init correctly persists the input instance.Assignment is early enough and before collector setup; no issues.
499-501: Approve code changes: only the time collector (flb_input_set_collector_time) is used and the.cb_pause/.cb_resumecallbacks are correctly wired.
Summary
Add pause/resume callbacks for
in_event_typethat stops the interval collector.Description
When stopping fluent-bit it firsts pauses all input plugins. This is usually meant to stop plugins from creating more tasks. For this to work correctly plugins need to pause their timed collectors at this point.
The plugin
in_event_typedoes not have these callbacks. This leads to situations occasionally where a task is created right as fluent-bit is shutting down leading to a memory leak:Here is a run of the
flb-rt-processor_labelshash_label test, which is where I originally encountered the bug: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