-
Notifications
You must be signed in to change notification settings - Fork 500
[MISC]: add vtc_bucket_size_active metric gauge for vtc-basic #1065
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
Conversation
|
I will take a look at it tomorrow |
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.
Pull Request Overview
This PR introduces a new custom metric gauge, vtc_bucket_size_active, along with centralized metrics utilities to support its use in the vtc-basic algorithm and its tests. Key changes include:
- Adding gauge metric update in the BasicVTCRouter.Route method.
- Extending test coverage with TestVTCBucketSizePatterns and related helper functions.
- Integrating the new metric into the metrics package and updating module dependencies.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/plugins/gateway/algorithms/vtc/vtc_basic_test.go | Added tests for vtc_bucket_size_active metric behavior and patterns |
| pkg/plugins/gateway/algorithms/vtc/vtc_basic.go | Updated routing logic to record the new metric using adaptiveBucketSize |
| pkg/metrics/metrics.go | Introduced the constant and configuration for vtc_bucket_size_active |
| pkg/metrics/custom_metrics_test.go | Added tests for custom metric registration and update behavior |
| pkg/metrics/custom_metrics.go | Updated helper functions for gauge and counter metric handling |
| go.mod | Added dependency for godebug |
Comments suppressed due to low confidence (2)
pkg/plugins/gateway/algorithms/vtc/vtc_basic_test.go:617
- Consider adding assertions to verify that the detected pattern matches expected outcomes instead of solely logging results for improved test validation.
if pattern.detectFunc(sizes) {
pkg/metrics/custom_metrics.go:94
- The GetGaugeValueForTest function always returns 0; consider implementing it to actually return the current gauge value to improve the reliability of tests referencing it.
return 0
Jeffwan
left a comment
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.
Overall looks good to me!
Signed-off-by: Venkat Raman <[email protected]>
Signed-off-by: Venkat Raman <[email protected]>
Signed-off-by: Venkat Raman <[email protected]>
Signed-off-by: Venkat Raman <[email protected]>
0a999a7 to
2469e97
Compare
…roject#1065) * feat: metrics initial gauge impl with tests pass * feat: use custrom_metrics under /metrics * feat: lint * feat: address review comments --------- Signed-off-by: Venkat Raman <[email protected]>
Pull Request Description
This change adds:
vtc_bucket_size_activecustom metric gauge to see if any config change is needed invtc-basicRelated Issues
#1011 (comment)
Submission Checklist