-
Notifications
You must be signed in to change notification settings - Fork 594
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
feat(telemetry): introduce count for all Gateway API objects #4058
Conversation
I believe we're now at the point where we can try to address the issue with the fake objects. If https://github.com/Kong/kubernetes-ingress-controller/blob/2e5bcc8c8034daeab4bd8558c8768450619f379f/internal/manager/telemetry/manager_test.go cannot be made to work with fake objects then I'd extend it with an envtest based test so that we can actually test the counters end to end. There have been occasions where the telemetry reporting failed after release and we needed to fix with a subsequent release which caused issues with our telemetry data. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4058 +/- ##
=======================================
+ Coverage 60.3% 60.7% +0.4%
=======================================
Files 150 150
Lines 16745 16747 +2
=======================================
+ Hits 10099 10175 +76
+ Misses 6014 5926 -88
- Partials 632 646 +14
☔ View full report in Codecov by Sentry. |
Pull request was converted to draft
2e5bcc8
to
13ff207
Compare
13ff207
to
68cbd34
Compare
68cbd34
to
be7f917
Compare
9e3a392
to
95962d4
Compare
ee4221c
to
f29e4c4
Compare
f29e4c4
to
55dc91e
Compare
Co-authored-by: Patryk Małek <[email protected]>
Co-authored-by: Patryk Małek <[email protected]>
ca6fdd1
to
a77eb66
Compare
What this PR does / why we need it:
We decided to collect telemetry for all Gateway API objects in users' clusters.
Which issue this PR fixes:
closes #3649
Special notes for your reviewer:
It bumps package telemetry to version
v0.0.5
that includes Kong/kubernetes-telemetry#143. Exclude directive has to be extended withsigs.k8s.io/gateway-api v0.7.0
, because it's been discarded #4019 and pkg telemetry uses that version alreadyTest
kubernetes-ingress-controller/internal/manager/telemetry/manager_test.go
Line 37 in 2e5bcc8
is not extended, because of issue with mocking. For more comprehensive tests in telemetry pkg the issue has been created Kong/kubernetes-telemetry#144
I'm proposing new way for testing it e2e with usage of
envtest
approach proposed in #4094 and #4101. Maybe it should be extended with mesh discovery too, and the above mentionedTestCreateManager
removed entirely - something to consider (and maybe create separate issue). For more standardised approach toenvtest
check issue #4099.I've run proposed test locally in a loop and I haven't observed flakiness. Timeouts were adjusted empirically to ensure that test will pass, for weak hardware (like GH managed runners) it was a problem to have
TelemetryPeriod
set e.g. to100ms
(on local machines it used to pass without any issue).PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR