hubble: Add a metric for lost events #22865
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
github-actions
bot
added
the
kind/community-contribution
rolinh
added
the
release-note/minor
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
lambdanis
force-pushed
the
pr/lost-events-metric
branch
from
ce1da36
to
caf39c4
Compare
aanm
removed
the
kind/community-contribution
|
This pull request has been automatically marked as stale because it |
github-actions
bot
added
the
stale
lambdanis
force-pushed
the
pr/lost-events-metric
branch
from
caf39c4
to
8890be4
Compare
github-actions
bot
removed
the
stale
lambdanis
force-pushed
the
pr/lost-events-metric
branch
3 times, most recently
from
28543ba
to
078b60a
Compare
lambdanis
force-pushed
the
pr/lost-events-metric
branch
from
078b60a
to
203afa0
Compare
rolinh
added
area/metrics
|
/test |
qmonnet
approved these changes
Mar 24, 2023
|
Should there be a note in Documentation/operations/upgrade.rst about the new metric? |
nebril
approved these changes
Mar 24, 2023
|
/test-1.26-net-next |
kaworu
reviewed
Mar 27, 2023
Currently numbers of lost events are visible in logs and in the observer, but they tend to be noisy and difficult to consume in such form. A Prometheus counter is much more suitable for this kind of information, thus this patch adds hubble_lost_events_total metric. Fixes cilium#15112 Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
lambdanis
force-pushed
the
pr/lost-events-metric
branch
from
203afa0
to
db321d9
Compare
The number of lost messages is now exposed as a metric, which is more practical than a log, so the log level can be reduced. Also rephrased the log message, to indicate only the fact that a LostEvent message was received - quite often the queue will be full immediately after that, so saying that Hubble is "back to nromal" can be misleading. Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
Signed-off-by: Anna Kapuscinska <anna@isovalent.com>
lambdanis
force-pushed
the
pr/lost-events-metric
branch
from
db321d9
to
72a2640
Compare
Hmm, should there? The new metric has low cardinality, so it shouldn't require any extra capacity planning from users. I think a release note is enough, but let me know @qmonnet if I should add something extra. |
|
/test |
kaworu
approved these changes
Mar 27, 2023
|
/ci-gke |
|
/ci-multicluster |
|
/ci-external-workloads |
maintainer-s-little-helper
bot
added
the
ready-to-merge
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently numbers of lost events are visible in logs and in the observer, but they tend to be noisy and difficult to consume in such form. A Prometheus counter is much more suitable for this kind of information, thus this patch adds
hubble_lost_events_totalmetric.The metric is exposed always when any of the "regular" Hubble metrics is enabled. It's low cardinality, and most users will want some of the flows metrics in addition to lost events - so this shouldn't be a problem.
I left the log line with lost events count, but to reduce logs noisiness (it was reported to be noisy in the past) I reduced its level to Debug.
Fixes #15112