engine: expose internal logging call counts as internal metrics #10326

alecholmes · 2025-05-13T00:04:37Z

This PR adds a new v2 runtime metric that exposes the number of logger calls by message type. A fluent-bit process consistently logging errors can be indicative of significant configuration or infrastructure problems. A common pattern for observing failures across many instances of software is to expose failures as metric counters that can then be observed and alerted on.

The implementation piggybacks on the src/flb_log.c logging library already extracting a worker context from the current thread.

Here is the example output of curling a fluent-bit with a service http_server enabled:

> curl localhost:5432/api/v2/metrics/prometheus 2>&1 | grep logger

fluentbit_logger_logs_total{severity="error"} 2
fluentbit_logger_logs_total{severity="warn"} 0
fluentbit_logger_logs_total{severity="info"} 10
fluentbit_logger_logs_total{severity="debug"} 0
fluentbit_logger_logs_total{severity="trace"} 0
fluentbit_logger_logs_total{severity="help"} 0

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:

[N/A] Example configuration file for the change
Debug log output from testing the change (see example from curling above)
Attached Valgrind output that shows no leaks or memory corruption was found

> valgrind -s bin/flb-rt-core_internal_logger

SUCCESS: All unit tests have passed.
==118424==
==118424== HEAP SUMMARY:
==118424==     in use at exit: 0 bytes in 0 blocks
==118424==   total heap usage: 2,098 allocs, 2,098 frees, 720,751 bytes allocated
==118424==
==118424== All heap blocks were freed -- no leaks are possible
==118424==
==118424== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

[N/A] Run local packaging test showing all targets (including any new ones) build.
[N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

Documentation required for this feature (I will create a docs PR to update the metric name table once this PR is approved)

Backporting

[N/A] Backport to latest stable release.

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.

src/flb_sds.c

alecholmes · 2025-05-13T00:06:24Z

tests/runtime/core_internal_logger.c

+        return NULL;
+    }
+
+    ret_ctx->u = flb_upstream_create(ret_ctx->config, "127.0.0.1", 2020, 0, NULL);


Is there any pattern or prior art for picking random free ports in tests?

There's not but we can talk about it if you're interested in making that improvement, I'd appreciate it.

alecholmes · 2025-05-14T03:37:20Z

I'm not convinced this PR introduced the fuzzer failures since I'm able to repro them on master.

The signv4_fuzzer failure, for example, seems to have been introduced at some point between 352bb31 and 6899dc1 -- it's hard to bisect because the commits in the middle of that range do not compile.

leonardo-albertovich · 2025-05-14T08:18:15Z

include/fluent-bit/flb_config.h

@@ -164,6 +164,7 @@ struct flb_config {
    /* Logging */
    char *log_file;
    struct flb_log *log;
+    struct flb_log_metrics *log_metrics_ctx; /* Global metrics for logging calls */


Isn't this metrics context supposed to be limited to the logger instance? If that's the case then it should be declared inside if flb_log instead of flb_config?

I've moved the metrics to flb_log and the lifecycle of them is now managed by flb_log_create and flb_log_destroy.

leonardo-albertovich · 2025-05-14T08:19:37Z

include/fluent-bit/flb_log.h

@@ -232,6 +242,8 @@ static inline int flb_log_suppress_check(int log_suppress_interval, const char *
 int flb_log_worker_init(struct flb_worker *worker);
 int flb_log_worker_destroy(struct flb_worker *worker);
 int flb_errno_print(int errnum, const char *file, int line);
+struct flb_log_metrics *flb_log_metrics_create();


These two shouldn't be public as they should be only invoked by flb_log_create and flb_log_destroy.

Removed from the header file.

leonardo-albertovich · 2025-05-14T08:19:55Z

src/flb_config.c

@@ -391,6 +391,14 @@ struct flb_config *flb_config_init()
    flb_regex_init();
 #endif

+    /* Create internal logger metrics */


This should be moved to flb_log_create

leonardo-albertovich · 2025-05-14T08:20:51Z

src/flb_log.c

@@ -573,39 +597,32 @@ int flb_log_construct(struct log_message *msg, int *ret_len,
    int total;
    time_t now;
    const char *header_color = NULL;
-    const char *header_title = NULL;
+    const char *header_title = flb_log_message_type_str(type);


Please don't make function calls in the middle of the declarations, initialize it to NULL and move the function call to line 606.

leonardo-albertovich · 2025-05-14T08:21:16Z

src/flb_log.c

@@ -564,6 +566,28 @@ struct flb_log *flb_log_create(struct flb_config *config, int type,
    return log;
 }

+static inline char *flb_log_message_type_str(int type)


Please change the return type to const char * to match the current usage.

leonardo-albertovich · 2025-05-14T08:27:49Z

src/flb_log.c

+    return lm;
+
+error:
+    if (lm && lm->logs_total_counter) {


If this code was to stay this would be a much cleaner structure which I'd ask you to follow:

if (lm != NULL) { if (lm->logs_total_counter != NULL) { cmt_counter_destroy(lm->logs_total_counter); } if (lm->cmt != NULL) { cmt_destroy(lm->cmt); } flb_free(lm); }

I reverted to not using goto. The pattern of using goto to jump to a cleanup section that effectively unwinds the allocations was something Phil and I had talked a bit about ahead of the PR, but I can bring that up as a discussion separately.

leonardo-albertovich · 2025-05-14T08:29:57Z

src/flb_log.c

+            break;
+        }
+
+        if (cmt_counter_set(lm->logs_total_counter, ts, 0, 1, (char *[]) {message_type_str}) == -1) {


Please don't put function calls inside of the conditions, it's ok to use simpler functions such as strlen sometimes there but this is not the case, declare a variable, store the result value there and then compare it.

The compiler will optimize it anyway and the result code will remain in a register in 99% of the cases, especially in the modern fastcall-ish conventions (amd64 & arm).

leonardo-albertovich · 2025-05-14T08:30:54Z

src/flb_log.c

+     * Initialize counters for all log message types to 0.
+     * This assumes types are contiguous starting at 1 (FLB_LOG_ERROR).
+     */
+    i = 1;


i is already initialized by the for, remove this.

src/flb_sds.c

leonardo-albertovich · 2025-05-14T08:39:53Z

tests/runtime/core_internal_logger.c

+        return NULL;
+    }
+
+    ret_ctx->u = flb_upstream_create(ret_ctx->config, "127.0.0.1", 2020, 0, NULL);


There's not but we can talk about it if you're interested in making that improvement, I'd appreciate it.

edsiper · 2025-05-28T20:15:24Z

src/flb_log.c

+/* Frees the metrics instance and its associated resources. */
+void flb_log_metrics_destroy(struct flb_log_metrics *metrics)
+{
+    if (metrics != NULL && metrics->cmt != NULL) {


metrics != NULL should be checked only once at the beginning

I had gotten earlier feedback to have this function to clean up partially initialized flb_log_metrics* instances. Here's what I just pushed to do the safe minimum number of null checks:

void flb_log_metrics_destroy(struct flb_log_metrics *metrics) { if (metrics == NULL) { return; } if (metrics->cmt != NULL) { cmt_destroy(metrics->cmt); } flb_free(metrics); }

edsiper · 2025-05-28T20:17:15Z

src/flb_sds.c

@@ -33,7 +33,7 @@
 #include <stdarg.h>
 #include <ctype.h>

-static flb_sds_t sds_alloc(size_t size)
+static flb_sds_t flb_sds_alloc_internal(size_t size)


since CFL was already updated, it seems these changes in flb_sds are not necessary

👍 Just rebased and reverted this rename.

Internal logger calls increment a new v2 metric exposed by the HTTP server Prometheus scrape endpoint. There is one time series per log message type. Signed-off-by: Alec Holmes <[email protected]>

Signed-off-by: Alec Holmes <[email protected]>

alecholmes · 2025-05-28T20:31:55Z

@edsiper Thanks for taking a look. Pushed changes addressing your feedback in a new temporary commit.

edsiper · 2025-05-28T22:37:57Z

by mistake I merged before squashing commits.. fixing that up

alecholmes requested review from edsiper, leonardo-albertovich, fujimotos and koleini as code owners May 13, 2025 00:04

github-actions bot added the docs-required label May 13, 2025

alecholmes commented May 13, 2025

View reviewed changes

src/flb_sds.c Outdated Show resolved Hide resolved

alecholmes commented May 13, 2025

View reviewed changes

alecholmes temporarily deployed to pr May 13, 2025 04:00 — with GitHub Actions Inactive

alecholmes temporarily deployed to pr May 13, 2025 04:28 — with GitHub Actions Inactive

alecholmes force-pushed the alec/2025-05-08-expose-log-counts branch from d83dbeb to 68ede1c Compare May 13, 2025 15:52

alecholmes requested a review from niedbalski as a code owner May 13, 2025 15:52

alecholmes temporarily deployed to pr May 13, 2025 16:21 — with GitHub Actions Inactive

alecholmes temporarily deployed to pr May 13, 2025 16:45 — with GitHub Actions Inactive

alecholmes force-pushed the alec/2025-05-08-expose-log-counts branch from 68ede1c to e61c2c0 Compare May 13, 2025 17:11

alecholmes temporarily deployed to pr May 13, 2025 19:21 — with GitHub Actions Inactive

alecholmes temporarily deployed to pr May 13, 2025 19:45 — with GitHub Actions Inactive

alecholmes had a problem deploying to pr May 13, 2025 19:46 — with GitHub Actions Failure

alecholmes temporarily deployed to pr May 13, 2025 19:46 — with GitHub Actions Inactive

leonardo-albertovich requested changes May 14, 2025

View reviewed changes

alecholmes force-pushed the alec/2025-05-08-expose-log-counts branch from 818b18c to 6388e14 Compare May 23, 2025 16:48

alecholmes temporarily deployed to pr May 23, 2025 16:48 — with GitHub Actions Inactive

alecholmes temporarily deployed to pr May 23, 2025 17:04 — with GitHub Actions Inactive

alecholmes temporarily deployed to pr May 23, 2025 17:05 — with GitHub Actions Inactive

alecholmes requested review from niedbalski and leonardo-albertovich May 27, 2025 17:03

edsiper requested changes May 28, 2025

View reviewed changes

alecholmes added 6 commits May 28, 2025 13:21

engine: expose internal logging call counts as internal metrics

4ed5dbe

Internal logger calls increment a new v2 metric exposed by the HTTP server Prometheus scrape endpoint. There is one time series per log message type. Signed-off-by: Alec Holmes <[email protected]>

Feedback (temporary -- will squash into above)

12bc6e8

Signed-off-by: Alec Holmes <[email protected]>

Feedback (temporary -- will squash into above)

91c0611

Signed-off-by: Alec Holmes <[email protected]>

Feedback (temporary -- will squash into above)

d42cb3e

Signed-off-by: Alec Holmes <[email protected]>

Feedback (temporary -- will squash into above)

22f8ce8

Signed-off-by: Alec Holmes <[email protected]>

Feedback (temporary -- will squash into above)

b45c0b5

Signed-off-by: Alec Holmes <[email protected]>

alecholmes force-pushed the alec/2025-05-08-expose-log-counts branch from 6388e14 to b45c0b5 Compare May 28, 2025 20:28

alecholmes temporarily deployed to pr May 28, 2025 20:29 — with GitHub Actions Inactive

alecholmes requested a review from edsiper May 28, 2025 20:31

alecholmes temporarily deployed to pr May 28, 2025 20:46 — with GitHub Actions Inactive

edsiper approved these changes May 28, 2025

View reviewed changes

edsiper added this to the Fluent bit v4.0.3 milestone May 28, 2025

edsiper merged commit 967c48f into fluent:master May 28, 2025
49 checks passed

edsiper mentioned this pull request May 28, 2025

log: expose internal logging call counts as internal metrics #10397

Merged

engine: expose internal logging call counts as internal metrics #10326

engine: expose internal logging call counts as internal metrics #10326

Conversation

alecholmes commented May 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alecholmes commented May 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alecholmes commented May 28, 2025

Uh oh!

Uh oh!

edsiper commented May 28, 2025

Uh oh!

Uh oh!