statistics: Add 'store' label to metric pd_cluster_status. #9898
Conversation
Signed-off-by: Sergey Kolosov <[email protected]>
ti-chi-bot
bot
added
release-note
|
Hi @SerjKol80. Thanks for your PR. I'm waiting for a tikv 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. Instructions 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. |
ti-chi-bot
bot
added
the
size/L
|
/ok-to-test |
ti-chi-bot
bot
added
ok-to-test
| ObserveHotStat(store, storesStats) | ||
| } | ||
| stats := storeStats.stats | ||
| tikvStats := stats.engineStatistics[core.EngineTiKV] |
There was a problem hiding this comment.
Do we need to add a new test for it?
There was a problem hiding this comment.
@lhy1024
Since we don't have aggregation logic anymore, then the test was removed for that part.
So, the only thing that might be tested in a new logic is if actual metric is emitted. I looked through existing tests and didn't dint find any tests validating actual metric emission. Thus, no new tests.
| Name: "status", | ||
| Help: "Status of the cluster.", | ||
| }, []string{"type", "engine"}) | ||
| }, []string{"type", "engine", "store"}) |
There was a problem hiding this comment.
Do we need to update metrics/grafana/pd.json?
There was a problem hiding this comment.
@lhy1024
I don't think so. All queries for pd_cluster_status metrics already aggregate that metric with sum(). Thus, there should no be any changes there. The description of that metrics should be updated, but I wasn't able to find doc describing metrics in this repo.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lhy1024 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
ti-chi-bot
bot
added
needs-1-more-lgtm
|
@lhy1024 |
|
@bufferflies PTAL |
3 participants
What problem does this PR solve?
Issue Number: Close #9855
What is changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
N/A
Release note