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

Update kvstore_duration_operation_seconds to not include rate-limiting latency #27396

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Aug 9, 2023

Before, kvstore_duration_operation_seconds metric included time spent in rate-limiting.
Now it does not include rate-limited time spent.
This change makes the metric more useful when troubleshooting if propagation delays are caused by slow kvstore or rate-limiting.
Users can still observe delays caused by rate-limiting using metric api_limiter_wait_duration_seconds

*_kvstore_operations_duration_seconds metrics do not include client-side rate-limiting latency anymore.

@marseel marseel requested review from a team as code owners August 9, 2023 15:28
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 9, 2023
@marseel marseel changed the title Update kvstore metrics Update kvstore_duration_operation_seconds to not include rate-limiting latency Aug 9, 2023
Documentation/operations/upgrade.rst Outdated Show resolved Hide resolved
Copy link
Member

@giorio94 giorio94 left a comment

Choose a reason for hiding this comment

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

/lgtm

Before, kvstore_duration_operation_seconds metric included time spent in rate-limiting.
Now it does not include rate-limited time spent.
This change makes metric more useful when troubleshooting if propagation
delayes are caused by slow kvstore or rate-limiting.
Users can still observe delayes caused by rate-limiting using metric
api_limiter_wait_duration_seconds.

Now, kvstore_duration_operation_seconds metric does not include errors
caused by rate-limiting.

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Aug 10, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 10, 2023
@marseel
Copy link
Contributor Author

marseel commented Aug 10, 2023

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 10, 2023
@ldelossa ldelossa merged commit 496dd5a into cilium:main Aug 11, 2023
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants