Skip to content

lib: cmetrics: upgrade to v2.1.2#11685

Merged
edsiper merged 1 commit into
masterfrom
cmetrics-2.1.2
Apr 9, 2026
Merged

lib: cmetrics: upgrade to v2.1.2#11685
edsiper merged 1 commit into
masterfrom
cmetrics-2.1.2

Conversation

@edsiper

@edsiper edsiper commented Apr 8, 2026

Copy link
Copy Markdown
Member

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

  • Bug Fixes

    • Improved Prometheus histogram decoding: explicitly validates required bucket labels and reports clear syntax errors when labels or bucket values are missing, preventing silent failures.
  • Tests

    • Added a test covering histograms missing required bucket labels to ensure proper error handling.
  • Chores

    • Bumped package version.

@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR increments the CMetrics patch version and adds strict validation for Prometheus histogram buckets by requiring a non-empty "le" label value; a unit test was added to assert failure when "le" is missing.

Changes

Cohort / File(s) Summary
Version Update
lib/cmetrics/CMakeLists.txt
Bump CMT_VERSION_PATCH from 1 to 2, updating derived CMT_VERSION_STR/CPACK_PACKAGE_VERSION used for artifacts and tarball naming.
Histogram Label Validation
lib/cmetrics/src/cmt_decode_prometheus.c
In add_metric_histogram, detect presence of the "le" label by scanning labels, compute allocations excluding "le", error with CMT_DECODE_PROMETHEUS_SYNTAX_ERROR if "le" is missing, and report a syntax error if a bucket's "le" value is empty before parsing.
Tests
lib/cmetrics/tests/prometheus_parser.c
Add test_histogram_missing_le_label() and register it in TEST_LIST to verify decoder fails with CMT_DECODE_PROMETHEUS_SYNTAX_ERROR when histogram buckets lack the "le" label.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • cosmo0920

Poem

🐰 I hopped through labels, sniffed each one,
The missing "le" and I were done.
I stamped my paw, I cried "No parse!"
Then nudged the version bump with a little jazz.
Now tests hop by with smiles and cheer.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately reflects the main objective of upgrading the cmetrics library to version 2.1.2, which encompasses all the changes made across CMakeLists.txt, the decoder, and tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cmetrics-2.1.2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@edsiper edsiper added this to the Fluent Bit v5.0.3 milestone Apr 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/cmetrics/CMakeLists.txt`:
- Line 9: The CMake file sets CMT_VERSION_PATCH to 2 but the official
fluent/cmetrics latest release is v2.0.5, so update the patch constant to match
the real upstream tag (change CMT_VERSION_PATCH to 5) or, if you intended an
unreleased/dev snapshot, document that decision and set the variable to the
exact development identifier you will use; locate the CMT_VERSION_PATCH
definition in the CMakeLists.txt (symbol: CMT_VERSION_PATCH) and adjust it to
the correct patch number or agreed-upon dev version string.

In `@lib/cmetrics/src/cmt_decode_prometheus.c`:
- Around line 546-553: The code checks only for NULL le label values but not
empty strings, so an empty le ("") slips into parse_double and produces a
generic create error; update the bucket handling in cmt_decode_prometheus.c to
treat empty le values as a syntax error by verifying
sample->label_values[le_label_index] is not NULL and not an empty string (e.g.,
string length == 0) before calling parse_double, and call report_error with
CMT_DECODE_PROMETHEUS_SYNTAX_ERROR and the existing "missing histogram bucket
\"le\" value" message when empty to keep behavior consistent with
invalid/missing boundaries.
🪄 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

Run ID: a66f10c9-8f66-43ca-87d4-42d9a690bf46

📥 Commits

Reviewing files that changed from the base of the PR and between e0b99b6 and 563d477.

⛔ Files ignored due to path filters (1)
  • lib/cmetrics/tests/prometheus_remote_write_payload.bin is excluded by !**/*.bin
📒 Files selected for processing (3)
  • lib/cmetrics/CMakeLists.txt
  • lib/cmetrics/src/cmt_decode_prometheus.c
  • lib/cmetrics/tests/prometheus_parser.c

set(CMT_VERSION_MAJOR 2)
set(CMT_VERSION_MINOR 1)
set(CMT_VERSION_PATCH 1)
set(CMT_VERSION_PATCH 2)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

What is the latest version of cmetrics library? Is version 2.1.2 available?

💡 Result:

The latest CMetrics (fluent/cmetrics) release tag is v2.0.5 (dated March 22, 2026). The repository also has an unstable tag dated April 8, 2026. [1]

Version 2.1.2 is not available in the official fluent/cmetrics tags/releases list (there is no v2.1.2 tag/release). [1]

Sources
[1] fluent/cmetrics tags (GitHub) https://github.com/fluent/cmetrics/tags


🏁 Script executed:

#!/bin/bash
# Description: Search for any hardcoded references to the old version 2.1.1

echo "=== Searching for references to version 2.1.1 ==="
rg -n "2\.1\.1" --type-add 'cmake:*.cmake' --type-add 'cmake:CMakeLists.txt' --type cmake

echo ""
echo "=== Searching for cmetrics version references in documentation ==="
rg -n "cmetrics.*2\.1\.1" --glob '*.md' --glob '*.txt' --glob '*.rst'

echo ""
echo "=== Checking git submodule configuration ==="
if [ -f .gitmodules ]; then
  cat .gitmodules | grep -A 5 cmetrics
fi

echo ""
echo "=== Checking for any version pins in dependency files ==="
fd -e txt -e cmake -e conf | xargs rg -l "cmetrics" | head -20

Repository: fluent/fluent-bit

Length of output: 527


Version 2.1.2 does not exist in the official cmetrics repository.

The web search confirms that the latest fluent/cmetrics release is v2.0.5 (March 22, 2026), with only an unstable tag available (April 8, 2026). There is no v2.1.2 tag or release in the official fluent/cmetrics repository.

Verify the correct version you intended to upgrade to and update the patch version accordingly. If this is targeting an unreleased development version, clarify the versioning approach with the team and update the CMakeLists.txt to reflect the actual version being used.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/cmetrics/CMakeLists.txt` at line 9, The CMake file sets CMT_VERSION_PATCH
to 2 but the official fluent/cmetrics latest release is v2.0.5, so update the
patch constant to match the real upstream tag (change CMT_VERSION_PATCH to 5)
or, if you intended an unreleased/dev snapshot, document that decision and set
the variable to the exact development identifier you will use; locate the
CMT_VERSION_PATCH definition in the CMakeLists.txt (symbol: CMT_VERSION_PATCH)
and adjust it to the correct patch number or agreed-upon dev version string.

Comment thread lib/cmetrics/src/cmt_decode_prometheus.c Outdated
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
lib/cmetrics/tests/prometheus_parser.c (1)

695-713: Also validate empty le value in this regression test.

This case verifies missing le, but not le="". Adding the empty-value case would fully protect the new “non-empty le” rule.

✅ Suggested test extension
 void test_histogram_missing_le_label()
 {
     int status;
     struct cmt *cmt;
     struct cmt_decode_prometheus_parse_opts opts;

     cmt = NULL;
     memset(&opts, 0, sizeof(opts));

     status = cmt_decode_prometheus_create(&cmt,
             "# HELP test_histogram A histogram missing the le label.\n"
             "# TYPE test_histogram histogram\n"
             "test_histogram_bucket{foo=\"bar\"} 1\n"
             "test_histogram_bucket{foo=\"baz\"} 2\n"
             "test_histogram_sum 3.5\n"
             "test_histogram_count 2\n", 0, &opts);

     TEST_CHECK(status == CMT_DECODE_PROMETHEUS_SYNTAX_ERROR);
+
+    if (cmt != NULL) {
+        cmt_decode_prometheus_destroy(cmt);
+    }
+
+    cmt = NULL;
+    status = cmt_decode_prometheus_create(&cmt,
+            "# HELP test_histogram A histogram with empty le label.\n"
+            "# TYPE test_histogram histogram\n"
+            "test_histogram_bucket{le=\"\"} 1\n"
+            "test_histogram_sum 1\n"
+            "test_histogram_count 1\n", 0, &opts);
+
+    TEST_CHECK(status == CMT_DECODE_PROMETHEUS_SYNTAX_ERROR);
+
+    if (cmt != NULL) {
+        cmt_decode_prometheus_destroy(cmt);
+    }
 }
Based on learnings: Applies to tests/** : Add or update tests for behavior changes, especially protocol parsing and encoder/decoder paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/cmetrics/tests/prometheus_parser.c` around lines 695 - 713, Extend the
test_histogram_missing_le_label regression test to also cover an empty le value
by invoking cmt_decode_prometheus_create with an input where the bucket lines
include le="" (e.g. test_histogram_bucket{le="" ,foo="bar"} ...) and assert the
same failure status CMT_DECODE_PROMETHEUS_SYNTAX_ERROR; ensure you modify the
test's input string(s) or add a separate call within the same function to
validate both the missing `le` and the `le=""` cases while keeping the use of
opts and the final TEST_CHECK on status.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/cmetrics/tests/prometheus_parser.c`:
- Around line 695-713: Extend the test_histogram_missing_le_label regression
test to also cover an empty le value by invoking cmt_decode_prometheus_create
with an input where the bucket lines include le="" (e.g.
test_histogram_bucket{le="" ,foo="bar"} ...) and assert the same failure status
CMT_DECODE_PROMETHEUS_SYNTAX_ERROR; ensure you modify the test's input string(s)
or add a separate call within the same function to validate both the missing
`le` and the `le=""` cases while keeping the use of opts and the final
TEST_CHECK on status.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bc18b6c7-39c7-4499-abcc-7cced0b5846e

📥 Commits

Reviewing files that changed from the base of the PR and between 563d477 and 8da6dc2.

⛔ Files ignored due to path filters (1)
  • lib/cmetrics/tests/prometheus_remote_write_payload.bin is excluded by !**/*.bin
📒 Files selected for processing (3)
  • lib/cmetrics/CMakeLists.txt
  • lib/cmetrics/src/cmt_decode_prometheus.c
  • lib/cmetrics/tests/prometheus_parser.c
🚧 Files skipped from review as they are similar to previous changes (2)
  • lib/cmetrics/src/cmt_decode_prometheus.c
  • lib/cmetrics/CMakeLists.txt

@edsiper edsiper merged commit 40a6d4a into master Apr 9, 2026
53 checks passed
@edsiper edsiper deleted the cmetrics-2.1.2 branch April 9, 2026 03:43
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.

2 participants