lib: cmetrics: upgrade to v2.0.6#11681
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
📝 WalkthroughWalkthroughThe PR updates CMetrics library version from 2.1.1 to 2.0.6, removes unit field serialization from msgpack codec operations, removes aggregation type assignment during exponential histogram concatenation, and adds explicit validation for the required Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 0dd3a4e162
ℹ️ 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".
| /* 'opts' */ | ||
| mpack_write_cstr(writer, "opts"); | ||
| mpack_start_map(writer, 5); | ||
| mpack_start_map(writer, 4); |
There was a problem hiding this comment.
Reducing the opts map to 4 fields here drops unit from msgpack output, so units are lost across the encode/decode path. Fluent Bit persists metrics through cmt_encode_msgpack_create/cmt_decode_msgpack_create, and exporters later consume map->unit (for example OTLP and Prometheus remote write metadata), so inputs that set units will now emit empty units after passing through a chunk. Please keep unit in the serialized header to preserve metric metadata.
Useful? React with 👍 / 👎.
| if (filtered_map != NULL) { | ||
| ret = cmt_cat_copy_map(&eh->opts, eh->map, filtered_map); |
There was a problem hiding this comment.
When concatenating exponential histograms, this path no longer copies exp_histogram->aggregation_type into the destination object. A newly created eh is zero-initialized, so DELTA/CUMULATIVE inputs become UNSPECIFIED after cmt_cat(), and OTLP encoding then derives the wrong aggregation temporality from that field. Any workflow that merges contexts via cmt_cat can therefore emit incorrect exponential histogram semantics.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/cmetrics/src/cmt_encode_msgpack.c (1)
96-115:⚠️ Potential issue | 🔴 CriticalBreaking msgpack schema change: opts map reduced from 5 to 4 fields removes backward compatibility with v2.1.0 payloads.
The
optsmap was extended in v2.1.0 to include aunitfield (5 fields total), but the current version reverts to 4 fields (ns, ss, name, desc). The decoder uses strict field validation: it returnsCMT_MPACK_UNEXPECTED_KEY_ERRORwhen encountering unknown keys. This means payloads from v2.1.0 containing theunitfield will fail to decode, causing data processing failures. There is no fallback mechanism or version handling to support both formats.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/cmetrics/src/cmt_encode_msgpack.c` around lines 96 - 115, The opts map was reduced to 4 entries breaking compatibility with v2.1.0 which included a unit field; restore the original 5-field schema by changing mpack_start_map(writer, 4) to mpack_start_map(writer, 5) and insert code to write the unit key/value (use mpack_write_cstr(writer, "unit"); mpack_write_cstr(writer, opts->unit);) between the other opts fields before mpack_finish_map(writer) so encoders produce the expected 'unit' field that the strict decoder requires.
🤖 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`:
- Around line 7-9: The CMake variables CMT_VERSION_MAJOR, CMT_VERSION_MINOR, and
CMT_VERSION_PATCH were changed from 2.1.1 to 2.0.6 but the commit message says
"upgrade", creating an inconsistency; either correct the version numbers back to
the intended 2.1.1 values in the CMakeLists
(CMT_VERSION_MAJOR/CMT_VERSION_MINOR/CMT_VERSION_PATCH) or, if the downgrade to
2.0.6 is intentional, update the commit message to clearly state this is a
revert/downgrade to v2.0.6 (or add a note explaining why) so version change and
commit message align.
---
Outside diff comments:
In `@lib/cmetrics/src/cmt_encode_msgpack.c`:
- Around line 96-115: The opts map was reduced to 4 entries breaking
compatibility with v2.1.0 which included a unit field; restore the original
5-field schema by changing mpack_start_map(writer, 4) to mpack_start_map(writer,
5) and insert code to write the unit key/value (use mpack_write_cstr(writer,
"unit"); mpack_write_cstr(writer, opts->unit);) between the other opts fields
before mpack_finish_map(writer) so encoders produce the expected 'unit' field
that the strict decoder requires.
🪄 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: 05825072-e75d-4bea-8907-61572b1a87e9
⛔ Files ignored due to path filters (1)
lib/cmetrics/tests/prometheus_remote_write_payload.binis excluded by!**/*.bin
📒 Files selected for processing (9)
lib/cmetrics/CMakeLists.txtlib/cmetrics/src/cmt_cat.clib/cmetrics/src/cmt_decode_msgpack.clib/cmetrics/src/cmt_decode_prometheus.clib/cmetrics/src/cmt_encode_msgpack.clib/cmetrics/tests/cat.clib/cmetrics/tests/encoding.clib/cmetrics/tests/opentelemetry.clib/cmetrics/tests/prometheus_parser.c
💤 Files with no reviewable changes (4)
- lib/cmetrics/src/cmt_cat.c
- lib/cmetrics/tests/cat.c
- lib/cmetrics/tests/opentelemetry.c
- lib/cmetrics/tests/encoding.c
| set(CMT_VERSION_MAJOR 2) | ||
| set(CMT_VERSION_MINOR 1) | ||
| set(CMT_VERSION_PATCH 1) | ||
| set(CMT_VERSION_MINOR 0) | ||
| set(CMT_VERSION_PATCH 6) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are any references to the expected version in the repository
echo "Checking for version references in the cmetrics library..."
rg -n "2\.1\.1|2\.0\.6" lib/cmetrics/ --type-add 'cmake:*.cmake' --type cmake --type-add 'txt:*.txt' --type txt -g '!build*'Repository: fluent/fluent-bit
Length of output: 119
🏁 Script executed:
# Search for CMT_VERSION in all CMakeLists.txt files to understand version management
echo "=== Searching for CMT_VERSION definitions across the repo ==="
find . -name "CMakeLists.txt" -type f | head -20 | xargs grep -n "CMT_VERSION" 2>/dev/null || echo "No matches found"
echo ""
echo "=== Checking for version files or headers ==="
fd -e h -e c -e txt "version" --max-results 20 | head -20
echo ""
echo "=== Looking at the specific file context ==="
head -20 lib/cmetrics/CMakeLists.txtRepository: fluent/fluent-bit
Length of output: 2001
🏁 Script executed:
# Check git history for this specific file to see version changes
echo "=== Git history for CMakeLists.txt version changes ==="
git log -p --follow -S "CMT_VERSION" -- lib/cmetrics/CMakeLists.txt 2>/dev/null | head -100
echo ""
echo "=== Alternative: Check recent git log for this file ==="
git log --oneline -10 -- lib/cmetrics/CMakeLists.txt 2>/dev/null
echo ""
echo "=== Check git diff for staged/recent changes to this file ==="
git diff -- lib/cmetrics/CMakeLists.txt 2>/dev/null || echo "No unstaged changes"Repository: fluent/fluent-bit
Length of output: 277
🏁 Script executed:
# Get more detailed git history to see the version before the upgrade
echo "=== Show the commit content ==="
git show 0dd3a4e1:lib/cmetrics/CMakeLists.txt 2>/dev/null | head -15
echo ""
echo "=== Show previous version by checking parent commit ==="
git show 0dd3a4e1^:lib/cmetrics/CMakeLists.txt 2>/dev/null | head -15 || git log --reverse --all --oneline -- lib/cmetrics/CMakeLists.txt 2>/dev/null | head -5Repository: fluent/fluent-bit
Length of output: 569
🏁 Script executed:
# Get the commit before 0dd3a4e1 to see the previous version
echo "=== Show parent commit version ==="
git show 0dd3a4e1~1:lib/cmetrics/CMakeLists.txt 2>/dev/null | head -15
echo ""
echo "=== Check if there's a version tag or release info ==="
git tag -l "*2.0.6*" "*2.1.1*" 2>/dev/null
echo ""
echo "=== Look at commit message ==="
git log -1 --format=fuller 0dd3a4e1 2>/dev/nullRepository: fluent/fluent-bit
Length of output: 521
🏁 Script executed:
# Try to get the actual diff to see what changed
echo "=== Get the actual diff for the version lines ==="
git show 0dd3a4e1 -- lib/cmetrics/CMakeLists.txt 2>/dev/null | grep -A 5 -B 5 "CMT_VERSION"
echo ""
echo "=== Check all commits that touched this file ==="
git log --oneline --all -- lib/cmetrics/CMakeLists.txt 2>/dev/null | head -10
echo ""
echo "=== Get previous commit's full CMakeLists.txt ==="
git log --all --oneline -p -- lib/cmetrics/CMakeLists.txt 2>/dev/null | head -80Repository: fluent/fluent-bit
Length of output: 5231
Commit message inconsistency: says "upgrade" but version downgrade is from 2.1.1 to 2.0.6.
Git history shows the previous version was 2.1.1 (commit 83d7619), and this commit (0dd3a4e) sets version 2.0.6, which is a downgrade. The commit message states "upgrade to v2.0.6", which is misleading. If this is intentional (e.g., reverting to a stable release), the commit message should be clarified to reflect that. Otherwise, verify that the version numbers are correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/cmetrics/CMakeLists.txt` around lines 7 - 9, The CMake variables
CMT_VERSION_MAJOR, CMT_VERSION_MINOR, and CMT_VERSION_PATCH were changed from
2.1.1 to 2.0.6 but the commit message says "upgrade", creating an inconsistency;
either correct the version numbers back to the intended 2.1.1 values in the
CMakeLists (CMT_VERSION_MAJOR/CMT_VERSION_MINOR/CMT_VERSION_PATCH) or, if the
downgrade to 2.0.6 is intentional, update the commit message to clearly state
this is a revert/downgrade to v2.0.6 (or add a note explaining why) so version
change and commit message align.
|
PR replaced by #11685 |
1 participant
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
Chores