Update etcdRequestLatency metrics bucket size

[kkkkun]

What type of PR is this?

/kind flake
/release-note-none

What this PR does / why we need it:

Apiserver has two metrics etcdRequestLatency and requestLatencies which show latency.
But they have different bucket size.

When we have a large cluster, It is difficulty to distinguish long latency which is spent in etcd or apiserver. Keeping consistent would help us more simple to find question.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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

[k8s-ci-robot]

Hi @kkkkun. 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.

[kkkkun]

/release-note-none

[dgrisonnet]

These changes looks good from my side although I am wondering if we really need that much granularity for etcd.

That said, considering that the number of buckets allocated to the apiserver latency was recently reduced from 40 to 20, I think it is reasonable to reconsider aligning the buckets of both metrics to be consistent since that's what we used to have in the past, but I would still like to have the opinion from others considering this past effort to reduce cardinality: #96754.

/cc @wojtek-t @tkashem

[tkashem]

I had a PR to increase the buckets #94134 (similar to this PR), but then I had to reduce it #96754.

I agree that keeping the bucket size similar makes these metrics more comparable, but we have to trade it off with a performance impact. The cardinality explosion was noticeable, we saw it in OpenShift. That's why I reduced it.

Also, not sure how directly comparable it can be - an api call may result in multiple calls to storage? @wojtek-t will know the answer to this.

On a related note, I am working on a PR to add the latency incurred in the storage layer as an annotation in the audit logs, if you mine the audit logs you will be able to have a clear picture what portion of the latency is spent in storage layer, admission layer, object transformation.

[kkkkun]

PR #94134 has 41 buckets, and PR #96754 has 11 buckets.
In this PR, it has 21 buckets.

[aojea]

do we need more than 11 buckets?

[kkkkun]

The bucket size is not the matter. We just want to keep the bucket size similar with requestLatencies .

[tkashem]

PR #94134 has 41 buckets, and PR #96754 has 11 buckets.
In this PR, it has 21 buckets.

Even that comes with a cost, please note that type (label in this metric) is pretty much unbounded, it includes the built in types plus the CRDs in the cluster. imo, this is one of the most expensive metrics we have.

It would be nice to see how this change impacts cpu utilization or memory footprint on a cluster and how it grows with increasing number of CRDs.

[fedebongio]

/assign @jpbetz
/triage accepted

[dgrisonnet]

/lgtm
/assign @logicalhan

[kkkkun]

@logicalhan Hi, Could you please give some advice?

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

[logicalhan]

/reopen
/remove-lifecycle stale

[k8s-ci-robot]

@logicalhan: Reopened this PR.

In response to this:

/reopen
/remove-lifecycle stale

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.

[logicalhan]

/remove-lifecycle rotten

[logicalhan]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kkkkun, logicalhan

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:

• staging/src/k8s.io/apiserver/pkg/endpoints/metrics/OWNERS [logicalhan]
• staging/src/k8s.io/apiserver/pkg/storage/etcd3/metrics/OWNERS [logicalhan]
• test/instrumentation/testdata/OWNERS [logicalhan]

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

[k8s-triage-robot]

This PR may require stable metrics review.

Stable metrics are guaranteed to not change. Please review the documentation for the requirements and lifecycle of stable metrics and ensure that your metrics meet these guidelines.

[k8s-triage-robot]

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

• The PR does have any do-not-merge/* labels
• The PR does not have the needs-ok-to-test label
• The PR is mergeable (does not have a needs-rebase label)
• The PR is approved (has cncf-cla: yes, lgtm, approved labels)
• The PR is failing tests required for merge

You can:

• Review the full test history for this PR
• Prevent this bot from retesting with /lgtm cancel or /hold
• Help make our tests less flaky by following our Flaky Tests Guide

/retest

[k8s-triage-robot]

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

• The PR does have any do-not-merge/* labels
• The PR does not have the needs-ok-to-test label
• The PR is mergeable (does not have a needs-rebase label)
• The PR is approved (has cncf-cla: yes, lgtm, approved labels)
• The PR is failing tests required for merge

You can:

• Review the full test history for this PR
• Prevent this bot from retesting with /lgtm cancel or /hold
• Help make our tests less flaky by following our Flaky Tests Guide

/retest

[k8s-ci-robot]

@kkkkun: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kubernetes-e2e-gce-ubuntu-containerd	fb372d0	link	unknown	/test pull-kubernetes-e2e-gce-ubuntu-containerd

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

[k8s-triage-robot]

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

• The PR does have any do-not-merge/* labels
• The PR does not have the needs-ok-to-test label
• The PR is mergeable (does not have a needs-rebase label)
• The PR is approved (has cncf-cla: yes, lgtm, approved labels)
• The PR is failing tests required for merge

You can:

• Review the full test history for this PR
• Prevent this bot from retesting with /lgtm cancel or /hold
• Help make our tests less flaky by following our Flaky Tests Guide

/retest