Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support tidb-binlog metrics scraping for TidbMonitor #2750

Merged
merged 10 commits into from
Jun 19, 2020
Merged

Support tidb-binlog metrics scraping for TidbMonitor #2750

merged 10 commits into from
Jun 19, 2020

Conversation

Yisaer
Copy link
Contributor

@Yisaer Yisaer commented Jun 17, 2020 •
edited
Loading

What problem does this PR solve?

Support tidb-binlog metrics scraping for TidbMonitor like below:

image

Related changes

  • Need to cherry-pick to the release branch

Does this PR introduce a user-facing change?:

Support scraping `Pump` and `Drainer` metrics in TidbMonitor

@Yisaer
Copy link
Contributor Author

Yisaer commented Jun 17, 2020

@tirsen PTAL

DanielZhangQD
DanielZhangQD reviewed Jun 17, 2020
View reviewed changes
pkg/monitor/monitor/template.go Outdated
@@ -126,6 +138,8 @@ func newPrometheusConfig(cmodel *MonitorConfigModel) *config.Config {
scrapeJob("tikv", tikvPattern, cmodel, buildAddressRelabelConfig(portLabel, tikvReplacement, true)),
scrapeJob("tiflash", tiflashPattern, cmodel, buildAddressRelabelConfig(portLabel, tiflashReplacement, true)),
scrapeJob("tiflash-proxy", tiflashPattern, cmodel, buildAddressRelabelConfig(tiflashProxyPortLabel, tiflashProxyReplacement, true)),
scrapeJob("pump", pumpPattern, cmodel, buildAddressRelabelConfig(portLabel, pumpReplacement, true)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the function in L286 for TLS need to be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

DanielZhangQD
DanielZhangQD previously approved these changes Jun 18, 2020
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DanielZhangQD
Copy link
Contributor

Please make sure the monitoring dashboard works as expected for clusters with or without TLS enabled.

@cofyc cofyc mentioned this pull request Jun 19, 2020
Closed
6 tasks
DanielZhangQD
DanielZhangQD previously approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zjj2wry
zjj2wry previously approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@zjj2wry zjj2wry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Yisaer Yisaer dismissed stale reviews from zjj2wry and DanielZhangQD via 1e55a5a June 19, 2020 06:32
@Yisaer Yisaer requested review from zjj2wry and DanielZhangQD June 19, 2020 06:32
DanielZhangQD
DanielZhangQD previously approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zjj2wry
zjj2wry previously approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@zjj2wry zjj2wry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Yisaer
Copy link
Contributor Author

Yisaer commented Jun 19, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@Yisaer Yisaer dismissed stale reviews from zjj2wry and DanielZhangQD via 71af806 June 19, 2020 08:08
@Yisaer Yisaer requested review from DanielZhangQD and zjj2wry June 19, 2020 08:10
DanielZhangQD
DanielZhangQD approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zjj2wry
zjj2wry approved these changes Jun 19, 2020
View reviewed changes
Copy link
Contributor

@zjj2wry zjj2wry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Yisaer Yisaer merged commit beb7c19 into pingcap:master Jun 19, 2020
ti-srebot pushed a commit to ti-srebot/tidb-operator that referenced this pull request Jun 19, 2020
@Yisaer @ti-srebot
cherry pick pingcap#2750 to release-1.1
f178d4f 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Jun 19, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-1.1 in PR #2782

cofyc pushed a commit that referenced this pull request Jun 19, 2020
@ti-srebot @Yisaer
cherry pick #2750 to release-1.1 (#2782)
80d29c6 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: Song Gao <disxiaofei@163.com>
This was referenced Jun 19, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanielZhangQD DanielZhangQD DanielZhangQD approved these changes

@zjj2wry zjj2wry zjj2wry approved these changes

@weekface weekface Awaiting requested review from weekface

@cofyc cofyc Awaiting requested review from cofyc

Assignees
No one assigned
Labels
area/monitor monitoring CanMerge needs-cherry-pick-1.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Yisaer @DanielZhangQD @ti-srebot @zjj2wry