Skip to content

Conversation

edsiper
Copy link
Member

@edsiper edsiper commented Aug 7, 2025

Fixes #10313


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

    • Improved handling of empty metrics chunks, ensuring empty metrics are skipped during processing and flushing.
    • Added new utility to detect when all metrics are empty.
  • Bug Fixes

    • Enhanced memory management to prevent potential double-free scenarios when processing metrics.
  • Documentation

    • Updated comments to clarify the distinction between legacy and current metrics interfaces.

Copy link

coderabbitai bot commented Aug 7, 2025

Walkthrough

The changes introduce stricter handling of empty metrics payloads throughout the metrics processing and output pipeline. They add a utility to check for empty metrics, update macro definitions to use C boolean types, and ensure that output plugins skip flushes when metrics data is empty, thus preventing attempts to send empty payloads.

Changes

Cohort / File(s) Change Summary
Boolean Macro Refactor
include/fluent-bit/flb_macros.h
Replaces integer-based FLB_FALSE/FLB_TRUE macros with false/true using <stdbool.h>.
Metrics Utility and Documentation
include/fluent-bit/flb_metrics.h, src/flb_metrics.c
Adds flb_metrics_is_empty function and declaration; improves comments clarifying v1 HTTP endpoint metrics.
Output Flush Handling
include/fluent-bit/flb_output.h
Adds logic to skip flushing empty metrics chunks; improves memory management and control flow for metrics event chunks.
Input Metrics Processing
src/flb_input_metric.c
Refactors encoding logic to centralize empty-check and destruction, using the new flb_metrics_is_empty utility.

Sequence Diagram(s)

sequenceDiagram
    participant Input as Input Plugin
    participant Processor as Metrics Processor
    participant Output as Output Plugin

    Input->>Processor: Provide metrics context
    Processor->>Processor: Process metrics (may filter out all data)
    Processor->>Output: Pass processed metrics context
    Output->>Output: Check if metrics context is empty (flb_metrics_is_empty)
    alt Metrics context is empty
        Output-->>Output: Log debug, skip flush, do not send payload
    else Metrics context is not empty
        Output->>Output: Serialize and send payload
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Assessment against linked issues

Objective Addressed Explanation
Prevent out_splunk (and outputs in general) from sending empty payloads (#10313)
Ensure metrics event chunks with zero size or empty metrics are skipped in output flushes (#10313)
Add utility to check for empty metrics contexts (#10313)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes were found.

Poem

A rabbit hops through metrics land,
With boolean flags now close at hand.
Empty chunks, we skip with glee,
No more payloads sent for free!
With careful checks and tidy code,
Our Splunk output’s in safe mode.
🐇✨

Note

🔌 MCP (Model Context Protocol) integration is now available in Early Access!

Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch empty-cmetrics

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

edsiper added 4 commits August 8, 2025 11:15
When ingesting a CMetrics context into the pipeline, it may contain metadata
but no actual data points (e.g., counters, gauges, histograms). Such metrics
should be dropped and not ingested.

Signed-off-by: Eduardo Silva <[email protected]>
When a metrics context is about to be flushed, it may pass through one or more
processors before reaching the output plugin. Processing can result in a valid
metrics context with no time series data, which is useless for output plugins.
This change detects that case, skips the flush, and returns FLB_OK to the engine.

Signed-off-by: Eduardo Silva <[email protected]>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
include/fluent-bit/flb_metrics.h (1)

95-99: Missing <stdbool.h> include for public header

flb_metrics_is_empty() returns bool, but this header itself doesn’t pull in <stdbool.h>.
It currently works only because downstream headers happen to include flb_macros.h, which now adds it.
Adding the include here removes the hidden dependency chain.

 #include <cmetrics/cmt_filter.h>
+#include <stdbool.h>
src/flb_input_metric.c (1)

72-79: Tiny optimisation – avoid double check

You’ve already selected encode_context; encode_context == NULL cannot be true because both branches set it to either out_context or cmt.
The explicit NULL test can be dropped leaving only the emptiness check and saving one conditional branch.

Not critical – feel free to ignore.

include/fluent-bit/flb_output.h (2)

673-679: Edge-case skip is good – but keep symmetry with logs

Nice addition. Consider also checking total_events == 0 for metrics chunks (mirroring the logs check) – some future encoder may set size==0 but non-zero events or vice-versa.


736-740: Unused variable in non-metrics builds

encode_context is declared at function scope but is only referenced inside the METRICS branch.
With FLB_HAVE_METRICS disabled some compilers will warn.

Either wrap the declaration in the same #ifdef or mark it FLB_UNUSED.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e62817 and 1bbf67c.

📒 Files selected for processing (5)
  • include/fluent-bit/flb_macros.h (1 hunks)
  • include/fluent-bit/flb_metrics.h (2 hunks)
  • include/fluent-bit/flb_output.h (9 hunks)
  • src/flb_input_metric.c (3 hunks)
  • src/flb_metrics.c (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-centos-7
  • GitHub Check: PR - fuzzing test
🔇 Additional comments (2)
include/fluent-bit/flb_macros.h (1)

23-27: Boolean macro switch may leak into pre-C99 translation units

Replacing the integer literals with true/false is cleaner, but it silently promotes FLB_TRUE/FLB_FALSE to the _Bool type.
Any compilation unit that (a) still builds in C90 mode or (b) relies on var-arg promotion differences might now get a surprise warning/error.

No change requested, just verify all modules are being compiled with -std=c99 (or later) and check for new warnings in CI.

include/fluent-bit/flb_output.h (1)

898-905: flb_event_chunk_create() with NULL data may assert

When serialization_buffer_offset == 0 we free p_buf then create a chunk with data=NULL and size=0.
If the helper insists on a non-NULL buffer even for zero length this will abort at runtime.

Please verify the helper tolerates (data==NULL && size==0) across all builds, or allocate a 1-byte dummy buffer instead.

Comment on lines +380 to +387
bool flb_metrics_is_empty(struct cmt *cmt)
{
return cfl_list_is_empty(&cmt->counters) &&
cfl_list_is_empty(&cmt->gauges) &&
cfl_list_is_empty(&cmt->histograms) &&
cfl_list_is_empty(&cmt->summaries) &&
cfl_list_is_empty(&cmt->untypeds);
}
Copy link

@coderabbitai coderabbitai bot Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Guard against NULL input in flb_metrics_is_empty()

If a caller ever passes a NULL struct cmt *, the function will seg-fault.
A defensive early-return costs nothing and prevents a crash path that’s very hard to diagnose at runtime.

 bool flb_metrics_is_empty(struct cmt *cmt)
 {
-    return cfl_list_is_empty(&cmt->counters) &&
+    if (cmt == NULL) {
+        return true;
+    }
+
+    return cfl_list_is_empty(&cmt->counters) &&
            ...
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bool flb_metrics_is_empty(struct cmt *cmt)
{
return cfl_list_is_empty(&cmt->counters) &&
cfl_list_is_empty(&cmt->gauges) &&
cfl_list_is_empty(&cmt->histograms) &&
cfl_list_is_empty(&cmt->summaries) &&
cfl_list_is_empty(&cmt->untypeds);
}
bool flb_metrics_is_empty(struct cmt *cmt)
{
if (cmt == NULL) {
return true;
}
return cfl_list_is_empty(&cmt->counters) &&
cfl_list_is_empty(&cmt->gauges) &&
cfl_list_is_empty(&cmt->histograms) &&
cfl_list_is_empty(&cmt->summaries) &&
cfl_list_is_empty(&cmt->untypeds);
}
🤖 Prompt for AI Agents
In src/flb_metrics.c around lines 380 to 387, the function flb_metrics_is_empty
does not check if the input pointer cmt is NULL, which can cause a segmentation
fault. Add a defensive check at the start of the function to return true (or an
appropriate value) immediately if cmt is NULL to prevent crashes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use FLB_TRUE instead true

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the current definition of FLB_TRUE and FLB_FALSE
cat include/fluent-bit/flb_macros.h | grep -A2 -B2 "FLB_TRUE\|FLB_FALSE"

Length of output: 173


🏁 Script executed:

#!/bin/bash
# Check how other functions handle NULL checks with boolean returns
rg -A3 "if \(.*== NULL\)" src/flb_metrics.c | head -20

Length of output: 56


🏁 Script executed:

#!/bin/bash
# Look for other bool-returning functions that use FLB_TRUE
rg "bool.*\(" include/fluent-bit/ -A5 | grep -B3 "FLB_TRUE\|return true"

Length of output: 74


@edsiper, you're right! For consistency with the rest of the codebase, here's the updated suggestion using FLB_TRUE:

bool flb_metrics_is_empty(struct cmt *cmt)
{
+    if (cmt == NULL) {
+        return FLB_TRUE;
+    }
+
    return cfl_list_is_empty(&cmt->counters) &&
           cfl_list_is_empty(&cmt->gauges) &&
           cfl_list_is_empty(&cmt->histograms) &&
           cfl_list_is_empty(&cmt->summaries) &&
           cfl_list_is_empty(&cmt->untypeds);
}

This maintains consistency with the project's macro usage pattern, even though FLB_TRUE is now defined as the boolean literal true.


🧬 Code Graph Analysis Results

Relevant snippets from src/flb_metrics.c (lines 22-35)

static int id_exists(int id, struct flb_metrics *metrics)
{
    struct mk_list *head;
    struct flb_metric *metric;

    mk_list_foreach(head, &metrics->list) {
        metric = mk_list_entry(head, struct flb_metric, _head);
        if (metric->id == id) {
            return FLB_TRUE;
        }
    }

    return FLB_FALSE;
}
  • This function checks if a metric ID already exists in the metrics list.
  • It returns FLB_TRUE if the ID exists, otherwise FLB_FALSE.
  • The user comment about using FLB_TRUE instead of true is directly relevant here since this function returns FLB_TRUE and FLB_FALSE macros instead of standard C true/false.

Relevant snippets from src/flb_metrics.c (lines 37-51)

static int id_get(struct flb_metrics *metrics)
{
    int id;
    int ret = FLB_FALSE;

    /* Try to use 'count' as an id */
    id = metrics->count;

    while ((ret = id_exists(id, metrics)) == FLB_TRUE) {
        id++;
    }

    return id;
}
  • This function generates a new unique metric ID.
  • It uses FLB_TRUE and FLB_FALSE macros for boolean logic.
  • This is another place where the user comment about using FLB_TRUE instead of true is relevant.

Relevant snippet from include/fluent-bit/flb_mem.h (lines 84-96)

void *flb_calloc(size_t n, const size_t size) {
    if (size == 0) {
        return NULL;
    }
#ifdef FLB_HAVE_TESTS_OSSFUZZ
   // Add chance of failure. Used by fuzzing to test error-handling code.
   if (flb_fuzz_get_probability(1)) {
     return NULL;
   }
#endif

    return calloc(n, size);
}
  • This snippet shows the flb_calloc function used in flb_metrics_create for memory allocation.
  • It is relevant for understanding memory allocation and error handling in the metrics code.

Relevant snippet from src/flb_sds.c (lines 389-399)

void flb_sds_destroy(flb_sds_t s)
{
    struct flb_sds *head;

    if (!s) {
        return;
    }

    head = FLB_SDS_HEADER(s);
    flb_free(head);
}
  • This snippet shows how flb_sds_destroy frees string data structures used for metric titles.
  • Relevant for understanding resource cleanup in flb_metrics_destroy.

Relevant snippet from src/flb_sds.c (lines 58-76)

flb_sds_t flb_sds_create_len(const char *str, int len)
{
    flb_sds_t s;
    struct flb_sds *head;

    s = sds_alloc(len);
    if (!s) {
        return NULL;
    }

    if (str) {
        memcpy(s, str, len);
        s[len] = '\0';

        head = FLB_SDS_HEADER(s);
        head->len = len;
    }
    return s;
}
  • This snippet shows how metric titles are created with a length limit.
  • Relevant for understanding how titles are handled in flb_metrics_add and flb_metrics_title.

Relevant snippet from lib/cmetrics/src/cmt_counter.c (lines 26-81)

struct cmt_counter *cmt_counter_create(struct cmt *cmt,
                                       char *ns, char *subsystem,
                                       char *name, char *help,
                                       int label_count, char **label_keys)
{
    int ret;
    struct cmt_counter *counter;

    if (!ns) {
        cmt_log_error(cmt, "null ns not allowed");
        return NULL;
    }

    if (!subsystem) {
        cmt_log_error(cmt, "null subsystem not allowed");
        return NULL;
    }

    if (!name || strlen(name) == 0) {
        cmt_log_error(cmt, "undefined name");
        return NULL;
    }

    if (!help || strlen(help) == 0) {
        cmt_log_error(cmt, "undefined help");
        return NULL;
    }

    counter = calloc(1, sizeof(struct cmt_counter));
    if (!counter) {
        cmt_errno();
        return NULL;
    }
    cfl_list_add(&counter->_head, &cmt->counters);

    ret = cmt_opts_init(&counter->opts, ns, subsystem, name, help);
    if (ret == -1) {
        cmt_log_error(cmt, "unable to initialize options for counter");
        cmt_counter_destroy(counter);
        return NULL;
    }

    /* Create the map */
    counter->map = cmt_map_create(CMT_COUNTER, &counter->opts, label_count, label_keys,
                                  (void *) counter);
    if (!counter->map) {
        cmt_log_error(cmt, "unable to allocate map for counter");
        cmt_counter_destroy(counter);
        return NULL;
    }
    /* set default counter aggregation type to cumulative */
    counter->aggregation_type = CMT_AGGREGATION_TYPE_CUMULATIVE;

    counter->cmt = cmt;
    return counter;
}
  • This snippet shows how counters are created in the cmetrics library, which is used in flb_metrics.c for attaching metrics.
  • Relevant for understanding the creation of counters in functions like attach_uptime.

Relevant snippet from lib/cmetrics/src/cmt_gauge.c (lines 27-81)

struct cmt_gauge *cmt_gauge_create(struct cmt *cmt,
                                   char *ns, char *subsystem, char *name,
                                   char *help, int label_count, char **label_keys)
{
    int ret;
    struct cmt_gauge *gauge;

    if (!ns) {
        cmt_log_error(cmt, "null ns not allowed");
        return NULL;
    }

    if (!subsystem) {
        cmt_log_error(cmt, "null subsystem not allowed");
        return NULL;
    }

    if (!name || strlen(name) == 0) {
        cmt_log_error(cmt, "undefined name");
        return NULL;
    }

    if (!help || strlen(help) == 0) {
        cmt_log_error(cmt, "undefined help");
        return NULL;
    }

    gauge = calloc(1, sizeof(struct cmt_gauge));
    if (!gauge) {
        cmt_errno();
        return NULL;
    }
    cfl_list_add(&gauge->_head, &cmt->gauges);

    /* Initialize options */
    ret = cmt_opts_init(&gauge->opts, ns, subsystem, name, help);
    if (ret == -1) {
        cmt_log_error(cmt, "unable to initialize options for gauge");
        cmt_gauge_destroy(gauge);
        return NULL;
    }

    /* Create the map */
    gauge->map = cmt_map_create(CMT_GAUGE, &gauge->opts, label_count, label_keys,
                                (void *) gauge);
    if (!gauge->map) {
        cmt_log_error(cmt, "unable to allocate map for gauge");
        cmt_gauge_destroy(gauge);
        return NULL;
    }

    gauge->cmt = cmt;

    return gauge;
}
  • This snippet shows how gauges are created in the cmetrics library, used in flb_metrics.c for attaching metrics like process start time and build info.
  • Relevant for understanding the metric attachment functions.

Relevant snippet from lib/cmetrics/src/cmt_gauge.c (lines 94-109)

int cmt_gauge_set(struct cmt_gauge *gauge, uint64_t timestamp, double val,
                  int labels_count, char **label_vals)
{
    struct cmt_metric *metric;

    metric = cmt_map_metric_get(&gauge->opts, gauge->map, labels_count, label_vals,
                                CMT_TRUE);
    if (!metric) {
        cmt_log_error(gauge->cmt, "unable to retrieve metric for gauge %s_%s_%s",
                      gauge->opts.ns, gauge->opts.subsystem,
                      gauge->opts.name);
        return -1;
    }
    cmt_metric_set(metric, timestamp, val);
    return 0;
}
  • This snippet shows how gauge values are set, using CMT_TRUE macro for boolean.
  • Relevant for understanding the boolean macro usage in metric setting functions.

[End of relevant snippets.]

Copy link

@coderabbitai coderabbitai bot left a 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

🔭 Outside diff range comments (1)
include/fluent-bit/flb_output.h (1)

49-58: Missing declaration for flb_metrics_is_empty

flb_metrics_is_empty() is used later in this header, but fluent-bit/flb_metrics.h is not included.
Relying on transitive includes risks an implicit-declaration warning or build break if the order changes.

+#include <fluent-bit/flb_metrics.h>   /* for flb_metrics_is_empty */
🧹 Nitpick comments (1)
include/fluent-bit/flb_output.h (1)

667-679: Early-return guard looks good but consider symmetry with logs

The new guard skips a flush when the metrics event-chunk has size == 0, mirroring the logs-path that checks total_events == 0.
Two suggestions:

  1. Use the same helper you added (flb_metrics_is_empty) for clarity and to future-proof against a non-zero buffer that still encodes zero series.
  2. Document in the comment why size is the chosen criterion so future readers don’t wonder about total_events.

Otherwise the logic correctly avoids the unnecessary network call.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1bbf67c and c9ced09.

📒 Files selected for processing (1)
  • include/fluent-bit/flb_output.h (9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
  • GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
  • GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
  • GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
  • GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
  • GitHub Check: pr-compile-centos-7
  • GitHub Check: PR - fuzzing test
🔇 Additional comments (1)
include/fluent-bit/flb_output.h (1)

903-916: Zero-metrics path may rely on flb_event_chunk_create() accepting NULL data

flb_event_chunk_create() is invoked with data = NULL and size = 0.
If that helper does not explicitly allow a NULL payload the call will seg-fault, and CI won’t catch it until a metrics filter removes every series.

Please confirm the helper’s contract or add a defensive fallback, e.g.

void *safe_buf = size ? buf : "";
flb_event_chunk_create(..., safe_buf, size);

@lecaros
Copy link
Contributor

lecaros commented Aug 12, 2025

thanks @edsiper, this solves the issue. I can see this message logged and no attempts to deliver empty chunks.

[2025/08/12 15:17:13.233752000] [debug] [output] skipping flush for metrics event chunk with zero metrics after processing.

It also deletes the unwanted chunks

@edsiper edsiper merged commit fb0b394 into master Aug 12, 2025
54 checks passed
@edsiper edsiper deleted the empty-cmetrics branch August 12, 2025 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

out_splunk tries to send data with empty payload
2 participants