-
Notifications
You must be signed in to change notification settings - Fork 1.9k
add gzip compression option for prometheus exporter #11321
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
base: master
Are you sure you want to change the base?
Changes from all commits
117bc4b
4eae4dc
9c95cb4
7006e17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,9 @@ | |
|
|
||
| #include <fluent-bit/flb_output_plugin.h> | ||
| #include <fluent-bit/flb_http_server.h> | ||
| #include <fluent-bit/flb_gzip.h> | ||
| #include <monkey/mk_http_parser.h> | ||
| #include <strings.h> | ||
| #include "prom.h" | ||
| #include "prom_http.h" | ||
|
|
||
|
|
@@ -157,10 +160,70 @@ static int http_server_mq_create(struct prom_http *ph) | |
| return 0; | ||
| } | ||
|
|
||
| /* 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 (strncasecmp(p, "gzip", 4) == 0) { | ||
| /* 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; | ||
| } | ||
|
Comment on lines
163
to
217
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 Consider parsing and respecting the quality parameter or at minimum checking for 🔎 Example logic to reject gzip when q=0After 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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,
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 If issues arise in the wild (which I doubt), you can always revisit it later. 🐰 🧠 Learnings used |
||
|
|
||
| /* HTTP endpoint: /metrics */ | ||
| static void cb_metrics(mk_request_t *request, void *data) | ||
| { | ||
| struct prom_http_buf *buf; | ||
| void *compressed_data = NULL; | ||
| size_t compressed_size = 0; | ||
| int use_gzip = 0; | ||
| int ret; | ||
| (void) data; | ||
|
|
||
| buf = metrics_get_latest(); | ||
|
|
@@ -172,9 +235,36 @@ static void cb_metrics(mk_request_t *request, void *data) | |
|
|
||
| buf->users++; | ||
|
|
||
| /* Check if client accepts gzip encoding */ | ||
| use_gzip = client_accepts_gzip(request); | ||
|
|
||
| /* Compress data if client supports gzip */ | ||
| if (use_gzip) { | ||
| ret = flb_gzip_compress(buf->buf_data, buf->buf_size, | ||
| &compressed_data, &compressed_size); | ||
| if (ret != 0 || !compressed_data) { | ||
| /* Compression failed, fall back to uncompressed */ | ||
| use_gzip = 0; | ||
| if (compressed_data) { | ||
| flb_free(compressed_data); | ||
| compressed_data = NULL; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| mk_http_status(request, 200); | ||
| flb_hs_add_content_type_to_req(request, FLB_HS_CONTENT_TYPE_PROMETHEUS); | ||
| mk_http_send(request, buf->buf_data, buf->buf_size, NULL); | ||
|
|
||
| /* Add Content-Encoding header if compressed */ | ||
| if (use_gzip && compressed_data) { | ||
| mk_http_header(request, "Content-Encoding", 16, "gzip", 4); | ||
| mk_http_send(request, compressed_data, compressed_size, NULL); | ||
| flb_free(compressed_data); | ||
| } | ||
| else { | ||
| mk_http_send(request, buf->buf_data, buf->buf_size, NULL); | ||
| } | ||
|
|
||
| mk_http_done(request); | ||
|
|
||
| buf->users--; | ||
|
|
||
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.
In
client_accepts_gzipthe code returns true as soon as it finds agziptoken in theAccept-Encodingheader, but it never inspects theqquality 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 👍 / 👎.