BE: fix isolate JMX metrics by label schema to prevent dropped broker me…#1824
BE: fix isolate JMX metrics by label schema to prevent dropped broker me…#1824bantnd wants to merge 1 commit into
Conversation
…trics Modified RawMetric.groupIntoSnapshot to use a composite key of name + labels. This prevents topic-level metrics (2 labels) from overwriting and dropping broker-level metrics (1 label) when they share the same metric name, fixing the 0 Bytes issue on the dashboard.
kapybro
Bot
added
status/triage
📝 WalkthroughWalkthroughThe metric snapshot grouping now uses a composite key combining sanitized metric name and label schema to index Prometheus Gauge instances. The label-schema mismatch check has been removed, allowing label-value assignment to proceed unconditionally for distinct gauges per metric name and label schema combination. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi bantnd! 👋
Welcome, and thank you for opening your first PR in the repo!
Please wait for triaging by our maintainers.
Please take a look at our contributing guide.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/src/main/java/io/kafbat/ui/service/metrics/RawMetric.java (1)
29-36: ⚡ Quick winMake label schema derivation deterministic before building
mapKey.On Line 40, the schema key depends on
m.labels()iteration order. Two logically identical label sets with different map order can produce differentmapKeyvalues and get split into separate gauges. Consider canonicalizing labels (e.g., sort by sanitized label name) and deriving bothlblsandlblValsfrom that same ordered sequence.Proposed refactor
+import java.util.Map.Entry; +import java.util.Comparator; ... - var lbls = m.labels().keySet() - .stream() - .map(PrometheusNaming::sanitizeLabelName) - .toArray(String[]::new); - var lblVals = m.labels().keySet() - .stream() - .map(l -> m.labels().get(l)) - .toArray(String[]::new); + var orderedLabels = m.labels().entrySet().stream() + .map(e -> Map.entry(PrometheusNaming.sanitizeLabelName(e.getKey()), e.getValue())) + .sorted(Comparator.comparing(Entry::getKey)) + .toList(); + var lbls = orderedLabels.stream().map(Entry::getKey).toArray(String[]::new); + var lblVals = orderedLabels.stream().map(Entry::getValue).toArray(String[]::new); var sanitizedName = PrometheusNaming.sanitizeMetricName(m.name());Also applies to: 40-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/src/main/java/io/kafbat/ui/service/metrics/RawMetric.java` around lines 29 - 36, The label schema derivation for building mapKey is non-deterministic because it uses m.labels() iteration order; change the logic in the block that produces lbls and lblVals so you first derive a single ordered list of label keys sorted by PrometheusNaming::sanitizeLabelName (e.g., collect m.labels().keySet(), map to sanitized names paired with original keys, sort by sanitized name), then build both lbls and lblVals from that same ordered sequence so that mapKey is stable across different map iteration orders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/src/main/java/io/kafbat/ui/service/metrics/RawMetric.java`:
- Around line 29-36: The label schema derivation for building mapKey is
non-deterministic because it uses m.labels() iteration order; change the logic
in the block that produces lbls and lblVals so you first derive a single ordered
list of label keys sorted by PrometheusNaming::sanitizeLabelName (e.g., collect
m.labels().keySet(), map to sanitized names paired with original keys, sort by
sanitized name), then build both lbls and lblVals from that same ordered
sequence so that mapKey is stable across different map iteration orders.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9c66a22-7ffc-4ae4-81a3-87ed2ef7c166
📒 Files selected for processing (1)
api/src/main/java/io/kafbat/ui/service/metrics/RawMetric.java
1 participant
What changes did you make? (Give an overview)
Modified
RawMetric.groupIntoSnapshotto group metrics using a composite key ofsanitizedName + ":" + labels. By doing this, metrics with the same name but different label dimensions (e.g., broker-level vs. topic-level) are placed into separate, dedicatedGaugeinstances. This prevents topic-level metrics from overwriting and dropping broker-level metrics when they share the same metric name, successfully fixing the 0 Bytes issue on the dashboard.Is there anything you'd like reviewers to focus on?
Just the mapping logic in
RawMetric.java.How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Check out Contributing and Code of Conduct
A picture of a cute animal (not mandatory but encouraged)
Summary by CodeRabbit
Bug Fixes