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

Bug 1881082: Expose etcd raft term as a metric #444

Merged
merged 1 commit into from Sep 21, 2020
Merged

Bug 1881082: Expose etcd raft term as a metric #444

merged 1 commit into from Sep 21, 2020

Conversation

ironcladlou
Copy link
Contributor

Piggyback on the member health checking facility to expose the current
raft term per member as a Prometheus metric to enable more granular
performance analysis.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2020
@ironcladlou
Copy link
Contributor Author

Here's what it looks like in a cluster I've got which has experienced massive churn:

# HELP etcd_raft_terms_total Number of etcd raft terms as observed by each member.
# TYPE etcd_raft_terms_total counter
etcd_raft_terms_total{member="dmace-71ad-nwdm7-master-0.c.openshift-gce-devel.internal"} 22
etcd_raft_terms_total{member="dmace-71ad-nwdm7-master-1.c.openshift-gce-devel.internal"} 22
etcd_raft_terms_total{member="dmace-71ad-nwdm7-master-2.c.openshift-gce-devel.internal"} 22

@ironcladlou
Copy link
Contributor Author

Although I'm not seeing them being scraped by Prometheus in my cluster, so I might be missing something in terms of registration, etc.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 14, 2020
@ironcladlou ironcladlou force-pushed the raft-metrics branch 5 times, most recently from 2cb09a2 to 505f28f Compare September 15, 2020 13:38
@ironcladlou
Copy link
Contributor Author

Already spent way too much time trying to figure out why modules are screwy in the CI build — switched to the deprecated RawMustRegister() function which bypasses the need to import semver since we have no use for the weird k8s metrics wrapper junk anyway.

@ironcladlou
Copy link
Contributor Author

From a cluster-bot cluster running this PR:

Screen Shot 2020-09-15 at 10 59 24 AM

@ironcladlou
Copy link
Contributor Author

/retest

@ironcladlou
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 15, 2020
@ironcladlou
Copy link
Contributor Author

/retest

2 similar comments
@ironcladlou
Copy link
Contributor Author

/retest

@ironcladlou
Copy link
Contributor Author

/retest

@hexfusion
Copy link
Contributor

I think in general this is a net gain in the context of historical reference. Raft terms are persisted to state thus the counter is immutable so to speak vs the current metric. As a followup, I would like to see the term added to events where reasonable as raft terms are included in all API responses thus very available.. Another benefit of adding the term to events is we can track before prom starts.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2020
@hexfusion
Copy link
Contributor

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 17, 2020
hexfusion
hexfusion reviewed Sep 17, 2020
View reviewed changes
pkg/etcdcli/health.go Outdated Show resolved Hide resolved
@ironcladlou
Expose etcd raft term as a metric
9d5cd75 
Piggyback on the member health checking facility to expose the current
raft term per member as a Prometheus metric to enable more granular
performance analysis.
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2020
@hexfusion
Copy link
Contributor

/lgtm
/hold cancel

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

19 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@ironcladlou ironcladlou changed the title Expose etcd raft term as a metric Bug 1881082: Expose etcd raft term as a metric Sep 21, 2020
@openshift-ci-robot
Copy link

@ironcladlou: This pull request references Bugzilla bug 1881082, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.6.0) matches configured target release for branch (4.6.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1881082: Expose etcd raft term as a metric

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/test-infra repository.

@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Sep 21, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link

@ironcladlou: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-disruptive 9d5cd75 link /test e2e-disruptive

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit ebe3222 into openshift:master Sep 21, 2020
@openshift-ci-robot
Copy link

@ironcladlou: All pull requests linked via external trackers have merged:

Bugzilla bug 1881082 has been moved to the MODIFIED state.

In response to this:

Bug 1881082: Expose etcd raft term as a metric

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/test-infra repository.

@openshift-ci-robot openshift-ci-robot mentioned this pull request Sep 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hexfusion hexfusion hexfusion left review comments

@retroflexer retroflexer Awaiting requested review from retroflexer

Assignees

@hexfusion hexfusion

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ironcladlou @hexfusion @openshift-ci-robot @openshift-bot @openshift-merge-robot