apiserver: add a metric exposing etcd database size #89151

jingyih · 2020-03-16T15:06:14Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adding a metric exposing etcd database file size. Example metric output:

# HELP etcd_db_total_size_in_bytes [ALPHA] Total size of the etcd database file physically allocated in bytes.
# TYPE etcd_db_total_size_in_bytes gauge
etcd_db_total_size_in_bytes{endpoint="http://127.0.0.1:2379"} 585728

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

kube-apiserver backed by etcd3 exports metric showing the database file size.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/sig api-machinery

k8s-ci-robot · 2020-03-16T15:06:23Z

Hi @jingyih. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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

jingyih · 2020-03-16T15:07:30Z

cc @logicalhan @apelisse @wenjiaswe

neolit123 · 2020-03-16T22:42:34Z

/ok-to-test
/priority backlog

jingyih · 2020-03-17T08:14:20Z

/retest

answer1991 · 2020-03-17T14:01:21Z

cc

wenjiaswe · 2020-03-17T17:16:22Z

staging/src/k8s.io/apiserver/pkg/storage/storagebackend/config.go

@@ -28,7 +28,8 @@ const (
 	StorageTypeUnset = ""
 	StorageTypeETCD3 = "etcd3"

-	DefaultCompactInterval = 5 * time.Minute
+	DefaultCompactInterval      = 5 * time.Minute
+	DefaultDbMetricPollInterval = 30 * time.Second


Could you please explain why is the polling interval set to 30s? Just curious if there is a specific reason.

How expensive is the call? If it's cheap, then have you considered using a GaugeFunc?

@wenjiaswe I don't have a specific reason that leads to 30s. A normal k8s cluster has 3 to 5 etcd servers, so 5 calls every 30s should be negligible. On the other hand I don't feel we need to update this metric very often as its value changes rather slowly.

@logicalhan Using GaugeFunc means we make rpc call to each etcd server when user hits metrics endpoint? This is probably too much latency?

Yes, db size change is usually very slow, that why I was thinking and wonder if we want a longer interval. But I am fine with what it is now as long as it does not impact performance. Thanks

I still think a GaugeFunc is probably more idiomatic. You can cache the value and not call out to etcd if the value hasn't TTL'd yet (the TTL would be the same as the poll interval currently)

@logicalhan I did not find an easy way to incorporate GaugeFunc with a gauge vector metric?

Yeah, it turns out it isn't exposed.

logicalhan · 2020-03-16T19:38:47Z

staging/src/k8s.io/apiserver/pkg/server/options/etcd.go

+	fs.DurationVar(&s.StorageConfig.DbMetricPollInterval, "etcd-db-metric-poll-interval", s.StorageConfig.DbMetricPollInterval,
+		"The interval of requests to poll etcd and update metrics. 0 disables the metrics collection")


I'm unsure if we want to add a flag for this.

I am unsure neither. Apiserver already has a lot of flags. I was thinking for some users running super heavy workloads, they might want to monitor this metric more often. If so, this flag could be useful.

FYI. Today we do have a similar flag: --etcd-count-metric-poll-period

Agree that it could be useful, most of the time db size is pretty stable, but there are cases where overuse of etcd would cause db size increase more often than others.

s/metrics/metric/

s/metrics/metric/

Done.

logicalhan · 2020-03-17T17:18:32Z

staging/src/k8s.io/apiserver/pkg/storage/storagebackend/config.go

@@ -28,7 +28,8 @@ const (
 	StorageTypeUnset = ""
 	StorageTypeETCD3 = "etcd3"

-	DefaultCompactInterval = 5 * time.Minute
+	DefaultCompactInterval      = 5 * time.Minute
+	DefaultDbMetricPollInterval = 30 * time.Second


How expensive is the call? If it's cheap, then have you considered using a GaugeFunc?

fedebongio · 2020-03-17T20:20:23Z

/assign @logicalhan

wenjiaswe · 2020-03-18T15:55:28Z

/lgtm

logicalhan

/lgtm

lavalamp · 2020-03-30T18:21:57Z

cmd/kube-apiserver/app/options/options_test.go

@@ -159,6 +159,7 @@ func TestAddFlags(t *testing.T) {
 				Prefix:                "/registry",
 				CompactionInterval:    storagebackend.DefaultCompactInterval,
 				CountMetricPollPeriod: time.Minute,
+				DbMetricPollInterval:  storagebackend.DefaultDbMetricPollInterval,


Do you prefer DBSizePollInterval or the current DBMetricPollInterval? I am using DBMetricPollInterval hoping that we can reuse the flag if we add new etcd metrics in future.

If the goal is to compute multiple metrics, then the name is fine.

lavalamp · 2020-03-30T21:40:04Z

staging/src/k8s.io/apiserver/pkg/storage/storagebackend/factory/etcd3.go

@@ -218,13 +229,19 @@ func newETCD3Storage(c storagebackend.Config) (storage.Interface, DestroyFunc, e
 		return nil, nil, err
 	}

+	stopDbMetricMonitor, err := startDbMetricMonitorPerEndpoint(client, c.DbMetricPollInterval)


DB. But shouldn't this name be stopDBSizeMonitor? Are we going to measure more than just the size?

Renamed to stopDBSizeMonitor.

lavalamp · 2020-03-31T18:00:15Z

/lgtm
/approve

k8s-ci-robot · 2020-03-31T18:01:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingyih, lavalamp

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:

~~cmd/kube-apiserver/OWNERS~~ [lavalamp]
~~staging/src/k8s.io/apiserver/OWNERS~~ [lavalamp]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from deads2k and madhusudancs March 16, 2020 15:07

k8s-ci-robot added area/apiserver sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 16, 2020

jingyih force-pushed the add_metric_etcd_db_size branch from 03f6548 to 3c8ce5e Compare March 17, 2020 06:54

wenjiaswe reviewed Mar 17, 2020

View reviewed changes

logicalhan reviewed Mar 17, 2020

View reviewed changes

k8s-ci-robot assigned logicalhan Mar 17, 2020

k8s-ci-robot assigned wenjiaswe Mar 18, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 18, 2020

logicalhan approved these changes Mar 24, 2020

View reviewed changes

lavalamp reviewed Mar 30, 2020

View reviewed changes

jingyih force-pushed the add_metric_etcd_db_size branch from 3c8ce5e to 313d677 Compare March 31, 2020 16:00

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2020

jingyih added 2 commits March 31, 2020 09:02

Add a metric exposing etcd database size

922ec72

Generated

e15c49f

jingyih force-pushed the add_metric_etcd_db_size branch from 313d677 to e15c49f Compare March 31, 2020 16:03

k8s-ci-robot assigned lavalamp Mar 31, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 31, 2020

k8s-ci-robot merged commit 0804667 into kubernetes:master Mar 31, 2020

k8s-ci-robot added this to the v1.19 milestone Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

apiserver: add a metric exposing etcd database size #89151

apiserver: add a metric exposing etcd database size #89151

jingyih commented Mar 16, 2020 •

edited

k8s-ci-robot commented Mar 16, 2020

jingyih commented Mar 16, 2020

neolit123 commented Mar 16, 2020

jingyih commented Mar 17, 2020

answer1991 commented Mar 17, 2020

wenjiaswe Mar 17, 2020

logicalhan Mar 17, 2020

jingyih Mar 18, 2020

wenjiaswe Mar 18, 2020

logicalhan Mar 18, 2020

jingyih Mar 23, 2020

logicalhan Mar 24, 2020

logicalhan Mar 16, 2020

jingyih Mar 18, 2020

jingyih Mar 18, 2020

wenjiaswe Mar 18, 2020

lavalamp Mar 30, 2020

jingyih Mar 31, 2020

logicalhan Mar 17, 2020

fedebongio commented Mar 17, 2020

wenjiaswe commented Mar 18, 2020

logicalhan left a comment

lavalamp Mar 30, 2020

jingyih Mar 31, 2020

lavalamp Mar 31, 2020

lavalamp Mar 30, 2020

jingyih Mar 31, 2020

lavalamp commented Mar 31, 2020

k8s-ci-robot commented Mar 31, 2020

		fs.DurationVar(&s.StorageConfig.DbMetricPollInterval, "etcd-db-metric-poll-interval", s.StorageConfig.DbMetricPollInterval,
		"The interval of requests to poll etcd and update metrics. 0 disables the metrics collection")

apiserver: add a metric exposing etcd database size #89151

apiserver: add a metric exposing etcd database size #89151

Conversation

jingyih commented Mar 16, 2020 • edited

k8s-ci-robot commented Mar 16, 2020

jingyih commented Mar 16, 2020

neolit123 commented Mar 16, 2020

jingyih commented Mar 17, 2020

answer1991 commented Mar 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fedebongio commented Mar 17, 2020

wenjiaswe commented Mar 18, 2020

logicalhan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp commented Mar 31, 2020

k8s-ci-robot commented Mar 31, 2020

jingyih commented Mar 16, 2020 •

edited