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

metrics: add bpf_map_capacity metric which provides max size of maps #28146

Merged
merged 2 commits into from Oct 2, 2023
Merged

metrics: add bpf_map_capacity metric which provides max size of maps #28146

merged 2 commits into from Oct 2, 2023

Conversation

tommyp1ckles
Copy link
Contributor

@tommyp1ckles tommyp1ckles commented Sep 13, 2023 •
edited

It is useful to have bpf map capacity metric, which is the defined bpf max size, for writing general Promtheus queries that can detect various issues with map usage (for example, providing an upper bound on FQDN IPs).

This is especially helpful in the face of configurable map sizes, where you cannot assume the default value is always used.

bpf_map_capacity metrics are emitted per "group" of maps, this is any group of maps that share common purpose and attributes.
This avoids having excessive redundant cardinality due to similar map types each having their capacity emitted in a separate metric (ex. endpoint policy maps are all always the same size).

As well, maps that are the default map capacity size (i.e. 2^16) are not included in this metric.

A follow up to this change will be adding a bpf map size metric, which can be used in conjunction with this metric to specify exactly how much capacity there is left in a map.

bpf_map_capacity metrics are emitted

@maintainer-s-little-helper
Copy link

Commit 128fdebf2bbd5a8cfcb5332996006a54312df986 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Sep 13, 2023
@maintainer-s-little-helper
Copy link

Commit 62a76d063446190c694abdf38f2f04dad3704e4b does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

1 similar comment
@maintainer-s-little-helper
Copy link

Commit 62a76d063446190c694abdf38f2f04dad3704e4b does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit 12e551f7090601c0af1b6e3b3d049b856e43e1ae does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit 12e551f7090601c0af1b6e3b3d049b856e43e1ae does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Sep 13, 2023
@tommyp1ckles tommyp1ckles added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/metrics Impacts statistics / metrics gathering, eg via Prometheus. labels Sep 13, 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 Sep 13, 2023
@tommyp1ckles tommyp1ckles marked this pull request as ready for review September 13, 2023 20:45
@tommyp1ckles tommyp1ckles requested review from a team as code owners September 13, 2023 20:45
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/map-capacity-metrics branch 4 times, most recently from 2d41d6b to d3a8e44 Compare September 13, 2023 20:58
zacharysarah
zacharysarah approved these changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

LGTM for docs

@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles
metrics: add bpf_map_capacity metric which provides max size of maps
3c690d1 
It is useful to have bpf map capacity metric, which is the defined bpf max size, for writing general Promtheus queries that can detect various issues with map usage (for example, providing an upper bound on FQDN IPs).

This is especially helpful in the face of configurable map sizes, where you cannot assume the default value is always used.

bpf_map_capacity metrics are emitted per "group" of maps, this is any group of maps that share common purpose and attributes.
This avoids having excessive redundant cardinality due to similar map types each having their capacity emitted in a separate metric (ex. endpoint policy maps are all always the same size).

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
@tommyp1ckles
documentation: add documentation for bpf map capacity metric.
f958495 
Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
@tommyp1ckles
Copy link
Contributor Author

/test

derailed
derailed approved these changes Sep 28, 2023
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

Nice work! LGTM.

@tommyp1ckles
Copy link
Contributor Author

/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 Sep 30, 2023
@lmb lmb merged commit 980590e into cilium:main Oct 2, 2023
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derailed derailed derailed approved these changes

@nathanjsweet nathanjsweet nathanjsweet approved these changes

@zacharysarah zacharysarah zacharysarah approved these changes

@markpash markpash markpash approved these changes

@rgo3 rgo3 rgo3 approved these changes

Assignees
No one assigned
Labels
area/metrics Impacts statistics / metrics gathering, eg via Prometheus. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@tommyp1ckles @derailed @nathanjsweet @zacharysarah @markpash @rgo3 @lmb