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.

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

op-guide, media: update monitoring docs #965

Merged
merged 8 commits into from
Mar 27, 2019

Conversation

lilin90
Copy link
Member

@lilin90 lilin90 commented Mar 21, 2019

  • Updated TiDB monitoring documents based on the user acceptance testing
    • Updated wording
    • Replaced outdated content with the new content
  • Reorganized TiDB monitoring documents based on Morgan's comment

Two points:

  1. I checked with the DBA and got that in TiDB v3.0-beta and v2.1, Pushgateway is not used anymore (not supported) in PD and TiDB, while it's optional in TiKV.

    In this PR, I add a statement and keep the Pushgateway section. I didn't use Pushgateway in my testing. Removing it might make the document/process much clearer and simpler. Do we need to remove this Pushgateway section in v3.0 docs?

  2. I learned that the JSON file link (in the current document) is not up-to-date. Could we use the files in the tidb-ansible repo? @liubo0127

Related PR: #944

@liubo0127 @morgo @c4pt0r @dcalvin PTAL

Copy link
Contributor

@dcalvin dcalvin left a comment

Choose a reason for hiding this comment

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

See my edits and comments in line.

op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
@morgo morgo self-requested a review March 25, 2019 02:06
@lilin90
Copy link
Member Author

lilin90 commented Mar 25, 2019

@dcalvin Thanks! Updated. PTAL again~

@liubo0127 PTAL

@liubo0127
Copy link
Contributor

  • Both 2.1 and master (3.0) support the pull mode, so you can remove pushgateway
  • Can import grafana dashboard using json in tidb-ansible/scripts

op-guide/monitor.md Outdated Show resolved Hide resolved
@morgo
Copy link
Contributor

morgo commented Mar 25, 2019

There was one pre-existing sentence I improved for clarity. Otherwise LGTM

Co-Authored-By: lilin90 <lilin@pingcap.com>
@lilin90
Copy link
Member Author

lilin90 commented Mar 27, 2019

@liubo0127 Then I'll remove the Pushgateway section and update the JSON file link.

@lilin90
Copy link
Member Author

lilin90 commented Mar 27, 2019

@liubo0127 @morgo @dcalvin Thanks! Updated. PTAL again~

op-guide/monitor.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dcalvin dcalvin left a comment

Choose a reason for hiding this comment

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

Some minor edits for consistency. And also, we should note that there is still statement in the Monitor Overview (#About Prometheus in TiDB) saying TiDB uses Pushgateway.

op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Outdated Show resolved Hide resolved
op-guide/monitor.md Show resolved Hide resolved
lilin90 and others added 2 commits March 27, 2019 20:13
Copy link
Contributor

@dcalvin dcalvin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@liubo0127 liubo0127 left a comment

Choose a reason for hiding this comment

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

LGTM

@lilin90 lilin90 merged commit fda58e4 into pingcap:master Mar 27, 2019
@lilin90 lilin90 deleted the monitor-update branch March 28, 2019 02:46
lilin90 added a commit to lilin90/docs-cn that referenced this pull request Apr 3, 2019
lilin90 added a commit to pingcap/docs-cn that referenced this pull request Jun 27, 2019
* op-guide: udpate TiDB monitoring docs

Via: pingcap/docs#965

* op-guide: update punctuation

* op-guide: add a "&" character

* media: update the image size

* v2.1-legacy: update content for v2.1-legacy

* *: update content in dev, v3.0, v2.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants