grafana: render stacked nulls as zero#5008
Conversation
|
Hi @nizheming. Thanks for your PR. I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Welcome @nizheming! |
📝 WalkthroughWalkthroughThe PR updates the Memory Usage graph panel configuration in two Grafana dashboards. The ChangesGrafana Dashboard Null Point Mode Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the nullPointMode configuration from null to null as zero in two Grafana dashboard JSON files. The reviewer suggests adopting a templating tool like Grafonnet to manage these dashboards more efficiently and reduce the risk of inconsistencies during manual updates.
| "links": [], | ||
| "maxPerRow": 3, | ||
| "nullPointMode": "null", | ||
| "nullPointMode": "null as zero", |
There was a problem hiding this comment.
I notice this same change is applied to metrics/nextgengrafana/ticdc_new_arch_next_gen.json. Having to update multiple dashboard JSON files manually for the same change can be error-prone and increases maintenance overhead.
To improve long-term maintainability, it might be worth exploring generating these Grafana dashboards from a template using a tool like Grafonnet or a custom script. This would reduce duplication and ensure consistency between the dashboard files.
|
Issue Number: ref #5009 |
Update the source Grafana dashboard and regenerate next-gen dashboards with make generate-next-gen-grafana. This keeps metrics/nextgengrafana output generated from metrics/grafana/ticdc_new_arch.json instead of editing the generated file by hand.
093e85d to
cf6d8eb
Compare
|
Thanks for pointing this out. I reset the generated next-gen JSON, kept the source change in Verification:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3AceShowHand, tenfyzhong The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
|
/ok-to-test |
|
@nizheming: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary
Issue Number: close #5009
Test
make generate-next-gen-grafanastack: truegraph panel hasnullPointMode: "null as zero"