Skip misconfigured vts entries #227
Conversation
Misconfigured ingress resources have the ability to create invalid VTS entries. These are long-lived and are not rectified againt the running nginx config. Because of this, it is possible that the metrics exported by feed can flip between the valid and invalid VTS data causing erroneous metrics to be exported. This adds an extra filter to ensure that metrics are not generated for misconfigured ingress resources.
ghost
added
the
There was a problem hiding this comment.
Do not forget to put the reference of the issue when merging.
That is a very nice context which is unfortunately lost in the PR.
I think the summary of the PR is not enough to describe the change.
Just had one question about the new test scenarios that may make you want to change them or make them a bit more clear?
nginx/nginx_test.go
Outdated
| assertIngressRequestCounters(t, | ||
| "duplicate-path.sandbox.cosmic.sky", "/path/", | ||
| 5000.0, 2000.0, 0.0, 5.0, 0.0, 0.0, 0.0) | ||
| assertIngressRequestCounters(t, | ||
| "misconfigured-ingress.sandbox.cosmic.sky", "/bad/", | ||
| 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0) |
There was a problem hiding this comment.
it is not really clear to me what is the difference between these two.
Is it to prove that a bad path with a valid duplicate will report the valid count, whereas a `bad path without a valid duplicate' will report 0?
There was a problem hiding this comment.
Correct. I've changed the names of these hosts and added comments that will hopefully express the tests intentions
Clearer host names and comments to make the test cases more descriptive
Misconfigured ingress resources have the ability to create invalid
VTS entries. These are long-lived and are not rectified againt the
running nginx config.
Because of this, it is possible that the metrics exported by feed
can flip between the valid and invalid VTS data causing erroneous
metrics to be exported.
This adds an extra filter to ensure that metrics are not generated
for misconfigured ingress resources.
Required by https://github.com/sky-uk/core-infrastructure/issues/7091