(B)ANP: Add num_anp && num_banp metrics #4239
Conversation
tssurya
requested review from
trozet,
dcbw,
girishmg and
jcaamano
as code owners
✅ Deploy Preview for subtle-torrone-bb0c84 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for resilient-beignet-2c8d1c ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
tssurya
force-pushed
the
add-anp-banp-number-count-metrics
branch
from
f4b87fa
to
155b72b
Compare
| @@ -269,6 +269,23 @@ var metricEgressFirewallCount = prometheus.NewGauge(prometheus.GaugeOpts{ | |||
| Help: "The number of egress firewall policies", | |||
| }) | |||
|
|
|||
| /** AdminNetworkPolicyMetrics Begin**/ | |||
There was a problem hiding this comment.
whats your goal with these metrics? what do you hope to achieve and how do you want the user to use them?
There was a problem hiding this comment.
Great question which I should have put in the PR description,
My goal with these two metrics are:
- understand how the user is using this feature
- how many admin network policies have they created (I plan to do future PRs for more fine-grained information)
- is the user using baseline admin network policy in their cluster?
So its also for the user but more so for us.
There was a problem hiding this comment.
Do you plan to create any alerts based on these metrics? For example, if theres a limit for any of the CRs, you can put a warning when they are closed to the threshold.
There was a problem hiding this comment.
I think I remember reading that theres a limit on at least one of the CRs.
There was a problem hiding this comment.
yeah there is a limit on ANP being max at 100, but TBH, I cannot fathom anyone using more than 10 :D so perhaps alerts is definitely good advice in case someone abuses this (as is always case with all features!)
There was a problem hiding this comment.
API enforces this anyway right - so alert might not be needed.
There was a problem hiding this comment.
ah no API doesn't enforce it (API says max 1000), its an ovnkube thing where I restrict them being able to do only 100 priorities so 100 ANPs, alert might be needed here actually (future PR/CNO).
tssurya
force-pushed
the
add-anp-banp-number-count-metrics
branch
from
155b72b
to
684ae18
Compare
|
flake is tracked at : #4242 |
This commits adds the metrics to count number of admin network policies and number of baseline admin network policies in a cluster Signed-off-by: Surya Seetharaman <[email protected]>
tssurya
force-pushed
the
add-anp-banp-number-count-metrics
branch
from
684ae18
to
a9b6bb2
Compare
|
unrelated flake in UT: saving link for debugging: https://github.com/ovn-org/ovn-kubernetes/actions/runs/8459367536/job/23175556649?pr=4239 |
This commits adds two metrics to:
My goal with these two metrics are:
So its also for the user but more so for us.
I am starting out with the metrics stuff for ANP/BANP
PTAL @martinkennelly at this one for starters. I think this is simple enough to not need the monitoring team's purview but future PRs will need their help. WDYT?