add gzip compression option for prometheus exporter #11321
Conversation
📝 WalkthroughWalkthroughPer-request gzip negotiation was added to the Prometheus exporter: the handler checks Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server as cb_metrics()
participant Parser as client_accepts_gzip()
participant Compressor as flb_gzip_compress()
participant Response
Client->>Server: GET /metrics (Accept-Encoding: ...)
Server->>Server: Check metrics buffer exists
alt no metrics
Server->>Response: Respond 404
Response-->>Client: 404
else metrics present
Server->>Parser: Inspect Accept-Encoding header
Parser-->>Server: gzip accepted? (true/false)
alt gzip accepted
Server->>Compressor: Compress metrics payload
alt compression success
Compressor-->>Server: compressed payload
Note right of Server `#D5F5E3`: Set Content-Encoding: gzip
Server->>Response: Send compressed payload
else compression fails
Compressor-->>Server: error
Server->>Response: Send uncompressed payload
end
else gzip not accepted
Server->>Response: Send uncompressed payload
end
Response-->>Client: 200 + headers + body
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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 |
Lusitaniae
force-pushed
the
feature/prom_gzip
branch
from
b68c4ee to
833ab79
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: 0
🧹 Nitpick comments (2)
plugins/out_prometheus_exporter/prom_http.c (2)
22-24: Consider removing unusedstring.hinclude.The
flb_gzip.handmk_http_parser.hincludes are necessary for the new functionality. However,string.hdoesn't appear to be used by the new code—no new calls tostrcpy,strcmp,strlen, or similar functions are introduced.🔎 Proposed fix
#include <fluent-bit/flb_gzip.h> #include <monkey/mk_http_parser.h> -#include <string.h>
163-220: Token parsing looks correct; consider handlingq=0quality value (optional).The token boundary detection logic is sound. However, per RFC 7231, a quality value of
q=0explicitly means the encoding is not acceptable (e.g.,gzip;q=0, deflate). The current implementation would still return1for such cases.This is a minor RFC compliance gap that's unlikely to affect real Prometheus scrapers, but worth noting.
If you want full compliance, you could extend the check to parse the quality value after detecting "gzip":
/* After finding "gzip" token, check for q=0 */ const char *semi = p + 4; while (semi < end && (*semi == ' ' || *semi == '\t')) semi++; if (semi < end && *semi == ';') { /* Parse quality value - skip if q=0 */ // ... (parse "q=" and value) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_prometheus_exporter/prom_http.c
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/out_prometheus_exporter/prom_http.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/out_prometheus_exporter/prom_http.c
🧬 Code graph analysis (1)
plugins/out_prometheus_exporter/prom_http.c (3)
lib/monkey/mk_server/mk_http.c (1)
mk_http_header_get(1499-1528)src/flb_gzip.c (1)
flb_gzip_compress(157-252)include/fluent-bit/flb_mem.h (1)
flb_free(126-128)
🔇 Additional comments (1)
plugins/out_prometheus_exporter/prom_http.c (1)
241-269: Compression logic and memory handling look correct.The fallback to uncompressed on compression failure is a good resilience pattern. Memory management is correct:
flb_gzip_compressallocates internally and the caller frees withflb_free().Note: The check on lines 251-254 is defensive—
flb_gzip_compressdoesn't set*out_dataon failure paths, socompressed_datashould remainNULLwhenret != 0. The defensive check is fine to keep.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/out_prometheus_exporter/prom_http.c (1)
163-220: Consider simplifying token parsing with standard library functions.The manual case-insensitive search and boundary validation work correctly but could be more maintainable. Consider using
strncasecmp()or similar standard functions to simplify the logic, though the current implementation handles edge cases properly (e.g., preventing matches within words like "notgzip").Alternative approach using standard functions
/* Check if client accepts gzip encoding */ static int client_accepts_gzip(mk_request_t *request) { struct mk_http_header *header; char *p, *token; char *saveptr = NULL; char buf[256]; /* Get Accept-Encoding header */ header = mk_http_header_get(MK_HEADER_ACCEPT_ENCODING, request, NULL, 0); if (!header || header->val.len == 0 || header->val.len >= sizeof(buf)) { return 0; } /* Copy header value for tokenization */ memcpy(buf, header->val.data, header->val.len); buf[header->val.len] = '\0'; /* Parse comma-separated encoding list */ token = strtok_r(buf, ",", &saveptr); while (token) { /* Skip leading whitespace */ while (*token == ' ' || *token == '\t') token++; /* Check for gzip (case-insensitive) */ if (strncasecmp(token, "gzip", 4) == 0) { char c = token[4]; if (c == '\0' || c == ' ' || c == '\t' || c == ';') { return 1; } } token = strtok_r(NULL, ",", &saveptr); } return 0; }Note: This assumes a reasonable header length limit. Adjust if needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_prometheus_exporter/prom_http.c
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/out_prometheus_exporter/prom_http.c
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/out_prometheus_exporter/prom_http.c
🔇 Additional comments (3)
plugins/out_prometheus_exporter/prom_http.c (3)
226-256: LGTM - Solid compression implementation with proper fallback.The compression logic is well-structured:
- Proper initialization of
compressed_datato NULL enables safe cleanup- Client negotiation via
client_accepts_gzip()respects HTTP standards- Graceful fallback to uncompressed response on compression failure
- Correct memory management in error paths
The per-request compression is appropriate given this is an opt-in feature controlled by the client.
22-22: No changes needed to the gzip includes.GZIP compression is unconditionally available in Fluent Bit and included without guards throughout the codebase (e.g., src/flb_http_common.c, src/http_server/flb_http_server.c, tests/runtime/in_forward.c). The direct include of
flb_gzip.his correct and consistent with existing patterns.Likely an incorrect or invalid review comment.
258-273: No changes needed. The code at lines 258–273 is correct.Lines 260 and 269 do not have trailing whitespace—these are clean blank lines. Additionally, the
buf->usersreference counting is thread-safe by design: buffers are stored in thread-local storage (viapthread_getspecific), so each Monkey HTTP worker thread maintains its own separate metrics list. The counter is only modified and read within the same thread context, eliminating race conditions.Likely an incorrect or invalid review comment.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e71090557d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| /* Found valid "gzip" token */ | ||
| return 1; |
There was a problem hiding this comment.
In client_accepts_gzip the code returns true as soon as it finds a gzip token in the Accept-Encoding header, but it never inspects the q quality parameter. A client that explicitly disables gzip (e.g. Accept-Encoding: gzip;q=0) will still receive a compressed metrics response and be unable to read it. Consider parsing the quality value and skipping gzip when it is zero.
Useful? React with 👍 / 👎.
…orter Signed-off-by: Pedro <[email protected]>
Signed-off-by: Pedro <[email protected]>
Lusitaniae
force-pushed
the
feature/prom_gzip
branch
from
e710905 to
4eae4dc
Compare
|
force pushed to add |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/out_prometheus_exporter/prom_http.c (1)
257-268: LGTM! Conditional Content-Encoding header and memory cleanup.The response handling correctly:
- Sets
Content-Encoding: gziponly when compressed data is sent.- Sends either compressed or uncompressed payload as appropriate.
- Frees
compressed_dataimmediately after sending (line 264), preventing leaks.Minor: trailing whitespace on lines 259 and 268 (after the semicolon and closing brace, respectively). This is a formatting nit and may be caught by linters.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/out_prometheus_exporter/prom_http.c
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components, ZSTD support is always available and doesn't need build-time conditionals.
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
📚 Learning: 2025-08-29T06:25:02.561Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:7-7
Timestamp: 2025-08-29T06:25:02.561Z
Learning: In Fluent Bit, ZSTD (zstandard) compression library is bundled directly in the source tree at `lib/zstd-1.5.7` and is built unconditionally as a static library. Unlike optional external dependencies, ZSTD does not use conditional compilation guards like `FLB_HAVE_ZSTD` and is always available. Headers like `<fluent-bit/flb_zstd.h>` can be included directly without guards.
Applied to files:
plugins/out_prometheus_exporter/prom_http.c
📚 Learning: 2025-08-29T06:25:27.250Z
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
Applied to files:
plugins/out_prometheus_exporter/prom_http.c
🧬 Code graph analysis (1)
plugins/out_prometheus_exporter/prom_http.c (2)
lib/monkey/mk_server/mk_http.c (1)
mk_http_header_get(1499-1528)src/flb_gzip.c (1)
flb_gzip_compress(157-252)
🔇 Additional comments (3)
plugins/out_prometheus_exporter/prom_http.c (3)
22-23: LGTM! Necessary includes for gzip compression.The includes are appropriate for the new functionality:
flb_gzip.hprovides the compression API andmk_http_parser.hprovides the HTTP header access.
231-236: Good: Return 404 when no metrics available.Returning a 404 status when the buffer is unavailable is the correct behavior for this endpoint.
240-255: LGTM! Compression with proper fallback.The compression flow is well-designed:
- Checks client capability before attempting compression.
- Calls
flb_gzip_compress()which allocates the output buffer.- Falls back gracefully to uncompressed output if compression fails.
- Frees allocated memory if compression fails partway.
Memory management is correct based on the
flb_gzip_compresscontract (caller owns the allocated buffer).
| /* Check if client accepts gzip encoding */ | ||
| static int client_accepts_gzip(mk_request_t *request) | ||
| { | ||
| struct mk_http_header *header; | ||
| const char *accept_encoding; | ||
| size_t len; | ||
| const char *p; | ||
| const char *end; | ||
|
|
||
| /* Get Accept-Encoding header */ | ||
| header = mk_http_header_get(MK_HEADER_ACCEPT_ENCODING, request, NULL, 0); | ||
| if (!header || header->val.len == 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| accept_encoding = header->val.data; | ||
| len = header->val.len; | ||
|
|
||
| /* Check if "gzip" is present in Accept-Encoding header (case-insensitive) */ | ||
| if (len < 4) { | ||
| return 0; | ||
| } | ||
|
|
||
| p = accept_encoding; | ||
| end = accept_encoding + len; | ||
|
|
||
| /* Search for "gzip" token */ | ||
| while (p < end - 3) { | ||
| /* Case-insensitive comparison for "gzip" */ | ||
| if ((p[0] == 'g' || p[0] == 'G') && | ||
| (p[1] == 'z' || p[1] == 'Z') && | ||
| (p[2] == 'i' || p[2] == 'I') && | ||
| (p[3] == 'p' || p[3] == 'P')) { | ||
| /* Check if it's a complete token (not part of another word) */ | ||
| /* Check character before "gzip" */ | ||
| if (p > accept_encoding) { | ||
| char prev = p[-1]; | ||
| if (prev != ',' && prev != ' ' && prev != '\t') { | ||
| p++; | ||
| continue; | ||
| } | ||
| } | ||
| /* Check character after "gzip" */ | ||
| if (p + 4 < end) { | ||
| char next = p[4]; | ||
| if (next != ',' && next != ' ' && next != '\t' && next != ';') { | ||
| p++; | ||
| continue; | ||
| } | ||
| } | ||
| /* Found valid "gzip" token */ | ||
| return 1; | ||
| } | ||
| p++; | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Quality values (q=0) still not respected in Accept-Encoding.
The function correctly identifies the presence of "gzip" with token-boundary checks, but it does not parse quality values. A client that sends Accept-Encoding: gzip;q=0 (explicitly disabling gzip) will still receive compressed content. Per RFC 7231, q=0 means "not acceptable."
Consider parsing and respecting the quality parameter or at minimum checking for q=0 after detecting the gzip token.
🔎 Example logic to reject gzip when q=0
After finding the gzip token (line 213), scan forward for a semicolon and parse the q-value:
}
/* Found valid "gzip" token */
+ /* Check for q=0 (explicitly disabled) */
+ const char *q_check = p + 4;
+ while (q_check < end && (*q_check == ' ' || *q_check == '\t')) {
+ q_check++;
+ }
+ if (q_check < end && *q_check == ';') {
+ q_check++;
+ while (q_check < end - 2 && (*q_check == ' ' || *q_check == '\t')) {
+ q_check++;
+ }
+ if (q_check < end - 2 && q_check[0] == 'q' && q_check[1] == '=' && q_check[2] == '0') {
+ return 0; /* q=0 means not acceptable */
+ }
+ }
return 1;
}Note: A full RFC 7231-compliant parser would be more robust, but this addresses the most critical case.
There was a problem hiding this comment.
How much do we care about this?
I'm guessing there's very limited number of prom metric clients, which wouldn't require handling edge cases
There was a problem hiding this comment.
@Lusitaniae, that's a fair point. You're absolutely right that the Prometheus ecosystem has a relatively small and well-defined set of clients—mostly Prometheus servers scraping endpoints, Grafana, and similar monitoring tools.
In practice, q=0 for gzip is extremely rare. HTTP clients typically either:
- Send
Accept-Encoding: gzip(or include it in a list) - Don't send it at all
- Very rarely explicitly disable it with
q=0
The current implementation handles the common cases correctly and will work fine with all mainstream Prometheus clients. The q=0 edge case is more of a theoretical RFC compliance issue than a practical concern.
My take: Given the specific use case (Prometheus metrics), the current implementation is pragmatic and sufficient. If you're comfortable with the trade-off of not handling q=0, I'd say ship it as-is. The benefit of perfect RFC compliance is minimal compared to the added complexity and maintenance burden.
If issues arise in the wild (which I doubt), you can always revisit it later.
🐰
🧠 Learnings used
Learnt from: shadowshot-x
Repo: fluent/fluent-bit PR: 10794
File: tests/internal/aws_compress.c:93-107
Timestamp: 2025-08-29T06:25:27.250Z
Learning: In Fluent Bit, ZSTD compression is enabled by default and is treated as a core dependency, not requiring conditional compilation guards like `#ifdef FLB_HAVE_ZSTD`. Unlike some other optional components such as ARROW/PARQUET (which use `#ifdef FLB_HAVE_ARROW` guards), ZSTD support is always available and doesn't need build-time conditionals. ZSTD headers are included directly without guards across multiple plugins and core components.
2 participants
It allows the prometheus exporter to serve compressed responses when the appropriate Accept Encoding header is set by the client.
Closes #11320
Testing
fluent-bit.conf
test requests
valgrind
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
Bug Fixes / Reliability
✏️ Tip: You can customize this high-level summary in your review settings.