docs: administration: monitoring: on-demand-flush: Add on-demand flush to the HTTP server - #2642
docs: administration: monitoring: on-demand-flush: Add on-demand flush to the HTTP server#2642RaJiska wants to merge 4 commits into
Conversation
…h to the HTTP server Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe documentation now describes the on-demand flush API, including configuration, requests, responses, counters, retry handling, concurrency, and errors. Administration documentation links to the new guide. ChangesOn-demand flush documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 3
🤖 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 `@administration/on-demand-flush.md`:
- Around line 16-20: Update both YAML examples in the on-demand flush
documentation to avoid exposing the HTTP control endpoint by default: prefer
binding http_listen to a loopback address, and add an explicit warning that
authentication plus firewall or reverse-proxy restrictions are required when
using a non-loopback address.
- Around line 47-50: Update the on-demand flush documentation to describe -d
'{}' as an empty JSON object, not an empty request body, and revise the curl
example to include the application/json Content-Type header and the explicit
http://localhost URL.
- Around line 65-70: Update the on-demand flush documentation around
flush_now_count to define it as a process-wide counter, including whether it
increments on request acceptance, scheduling, or completion and whether timeout
responses report the value before or after the failed request’s potential
increment. Clarify that the {"flush":"timeout","flush_now_count":0} example is
valid only when the counter remains at 0.
🪄 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 Plus
Run ID: 2cd8d15f-0e33-4e8b-809b-aa84c6640ea4
📒 Files selected for processing (3)
SUMMARY.mdadministration/monitoring.mdadministration/on-demand-flush.md
Signed-off-by: Ra'Jiska <dodo.lasticot@gmail.com>
|
@RaJiska thanks for the doc PR! I'm going to fix a few small issues and then push them here for you to review too. |
… document engine behavior Validated the page against the implementation in fluent/fluent-bit#12192 and corrected four inaccuracies: - The timeout example reported flush_now_count as 0 and described the counter as unchanged, contradicting the process-wide semantics described directly above it. The 503 path packs the current global counter, which reflects other flushes. Also clarify that a 503 is a missed acknowledgement, not a cancellation: the request stays queued on the manager channel and can still be processed afterwards. - Document the previously unmentioned 500 responses. A failed dispatch to the engine triggers no flush and leaves the counter alone, while a response encoding failure occurs after the flush was already requested, so the counter can still advance. Both return an empty body rather than JSON. - Replace "must specify an empty request body as -d '{}'" with an accurate description. The handler dispatches on the request method and never reads the body, and the HTTP/1 parser accepts a bodyless POST, so no payload is required. Use an explicit http:// URL. - Add a section describing what a flush does. Pending retries are invalidated and rescheduled to run immediately before buffered chunks are dispatched, so chunks in retry backoff are sent without waiting out their timer. Signed-off-by: Eric D. Schabell <eric@schabell.org>
|
Thank you for reviewing and adding doc coverage @eschabell ! This looks good for most parts. The part regarding the curl commands needs to be reverted as the documented command won't work. Based on my previous message, I found out omitting the Through some additional tests I could pin down the issue to the This issue happens on every single routes of the HTTP server leading me to think this may be an upstream issue from Monkey lib (unfortunately I wasn't able to get concrete evidence through tracing on either FB or Monkey). You should be able to reproduce this issue easily with the following FB configuration: Following the tests confirming the above and isolating the issue to the # GET on non-existing route: 404 as expected (request processed)
$ time curl http://localhost:2020/dsadas -v
* Host localhost:2020 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:2020...
* Connected to localhost (::1) port 2020
> GET /dsadas HTTP/1.1
> Host: localhost:2020
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/1.1 404
< server: Fluent Bit
< x-http-engine: Monkey heritage
< content-length: 0
<
* Connection #0 to host localhost left intact
real 0m0.011s
user 0m0.005s
sys 0m0.005s
# POST w/ -d param on non-existing route: 404 as expected (request processed)
$ time curl -X POST http://localhost:2020/dsadas -d '{}' -v
# 404 as expected
Note: Unnecessary use of -X or --request, POST is already inferred.
* Host localhost:2020 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:2020...
* Connected to localhost (::1) port 2020
> POST /dsadas HTTP/1.1
> Host: localhost:2020
> User-Agent: curl/8.5.0
> Accept: */*
> Content-Length: 2
> Content-Type: application/x-www-form-urlencoded
>
< HTTP/1.1 404
< server: Fluent Bit
< x-http-engine: Monkey heritage
< content-length: 0
<
* Connection #0 to host localhost left intact
real 0m0.010s
user 0m0.005s
sys 0m0.004s
# POST w/o -d param on non-existing route: empty response from server-side timeout (request unprocessed)
time curl -X POST http://localhost:2020/dsadas -v
* Host localhost:2020 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:2020...
* Connected to localhost (::1) port 2020
> POST /dsadas HTTP/1.1
> Host: localhost:2020
> User-Agent: curl/8.5.0
> Accept: */*
>
* Empty reply from server
* Closing connection
curl: (52) Empty reply from server
real 0m9.274s
user 0m0.005s
sys 0m0.007s
# POST w/o -d param on non-existing route but manually adding Content-Length header: 404 as expected (request processed)
$ time curl -X POST http://localhost:2020/dsadas -H 'Content-Length: 0' -v
* Host localhost:2020 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:2020...
* Connected to localhost (::1) port 2020
> POST /dsadas HTTP/1.1
> Host: localhost:2020
> User-Agent: curl/8.5.0
> Accept: */*
> Content-Length: 0
>
< HTTP/1.1 404
< server: Fluent Bit
< x-http-engine: Monkey heritage
< content-length: 0
<
* Connection #0 to host localhost left intact
real 0m0.008s
user 0m0.003s
sys 0m0.004sI ran my tests on an non-existing route to highlight the issue may not originate from a route in particular but either from the interfacing with Monkey, or the Monkey lib itself. Those results are consistent across any routes (except 404 when testing against actually existing routes of course). |
Documents a new feature adding a flush route to the HTTP server in order to force a flush independently of the Fluentbit timer: fluent/fluent-bit#12192.
Summary by CodeRabbit
POSTorPUTto trigger flushes andGETto view the flush count.