-
Notifications
You must be signed in to change notification settings - Fork 765
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: Add status metric to config #3656
base: master
Are you sure you want to change the base?
feat: Add status metric to config #3656
Conversation
Signed-off-by: Avinash Patnala <avinashpatnala@google.com>
Signed-off-by: Avinash Patnala <avinashpatnala@google.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3656 +/- ##
==========================================
- Coverage 54.49% 47.82% -6.68%
==========================================
Files 134 222 +88
Lines 12329 18738 +6409
==========================================
+ Hits 6719 8961 +2242
- Misses 5116 8936 +3820
- Partials 494 841 +347
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -93,13 +94,19 @@ func newReconciler(mgr manager.Manager, cm *cm.CacheManager, cs *watch.Controlle | |||
return nil, fmt.Errorf("cacheManager must be non-nil") | |||
} | |||
|
|||
r, err := newStatsReporter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the observability goals for this metric? As in: what kinds of conclusions should consumers be able to draw by observing this metric? Is the data we are expressing sufficient to support those goals?
What are the gaps in reporting present in this current code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If GK admin and consumer of metrics are two different users, then I think having number of kinds getting synced in config resource might be helpful when access to config resource is resticted?
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #2918
Special notes for your reviewer: