-
Notifications
You must be signed in to change notification settings - Fork 293
DM: Add $k8s_cluster and $tidb_cluster as potential filter to Grafana panels. #12154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
varaible filters on TiDBCloud.
/hold |
/retest |
/cc @D3Hunter @GMHDBJD @lance6716 Please take a look and help review this PR. |
@OliverS929: GitHub didn't allow me to request PR reviews from the following users: joyjoyhong. Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you paste the manual test result? I'm not sure if there are other problems
Sure can do. I don't think these filters can be utilized in self-managed DM cluster though, since we don't have those two filters available. Are you referring to some manual testing in a self-managed cluster just in case we break anything with these modifications? |
I see the issue #12153 is opened because of TiDB cloud monitor problems. I think you can open the grafana of TiDB cloud and manually edit some panel expression to see if $k8s_cluster and $tidb_cluster can work as expected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest lgtm
@@ -208,7 +208,7 @@ | |||
"targets": [ | |||
{ | |||
"exemplar": true, | |||
"expr": "dm_worker_task_state{job_id=~\"$job_id\",task=~\"$task\",source_id=~\"$source\"}", | |||
"expr": "dm_worker_task_state{k8s_cluster=~\"$k8s_cluster\",tidb_cluster=~\"$tidb_cluster\", job_id=~\"$job_id\",task=~\"$task\",source_id=~\"$source\"}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tidb is using k8s_cluster="$k8s_cluster", tidb_cluster="$tidb_cluster"
, maybe unify it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I am aware of that practice, and I took this k8s_cluster=~\"$k8s_cluster\"
approach on purpose. Because it is much more flexible than k8s_cluster="$k8s_cluster"
. For scenarios where k8s_cluster is absent, under the current approach, Prometheus will include all series that do NOT have k8s_cluster, and ignore the filter for series without that label, whereas "$k8s_cluster" will lead to the query returning nothing.
Sure I will turn to @kaaaaaaang and see we can have some screenshots. Any particular panel you would like us to verify? |
/cc @kennytm |
@kaaaaaaang: adding LGTM is restricted to approvers and reviewers in OWNERS files. In response to this: 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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaaaaaaang, kennytm, lance6716 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 |
|
/unhold |
What problem does this PR solve?
Issue Number: close #12153
What is changed and how it works?
Add $k8s_cluster and $tidb_cluster as potential filter to DM grafana panel setups.
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
No it should not be.
Do you need to update user documentation, design documentation or monitoring documentation?
No obvious relation to user documentation or designs.
Release note