-
Notifications
You must be signed in to change notification settings - Fork 3
feat(purge): add Prometheus metrics for purge request tracking #35
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
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 pull request adds Prometheus metrics tracking to the purge plugin to monitor purge request outcomes and implements the previously stubbed /plugin/purge/tasks endpoint to return purge task data from the shared key-value store as JSON.
Changes:
- Introduced Prometheus counter metric for tracking purge requests by response code (200, 403, 404, 500)
- Implemented
/plugin/purge/tasksendpoint to query and return purge task list from SharedKV - Added metric increment calls at all response points in the purge handler
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| plugin/purge/metrics.go | New file defining Prometheus counter metric for purge request tracking with label initialization |
| plugin/purge/purge.go | Added imports for binary encoding and codec; implemented tasks endpoint with SharedKV iteration; added metric tracking to all handler response paths |
Comments suppressed due to low confidence (1)
plugin/purge/purge.go:79
- The /plugin/purge/tasks endpoint does not track Prometheus metrics for its requests. Consider adding metric tracking for this endpoint as well, using appropriate labels (e.g., "200" for success and "500" for internal server errors), to maintain consistency with the metrics tracking in the HandleFunc method and ensure comprehensive observability.
router.Handle("/plugin/purge/tasks", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
// query sharedkv purge task list
purgeTaskMap := make(map[string]uint64)
sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error {
purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val)
return nil
})
// marshal to json
payload, err := codec.Marshal(purgeTaskMap)
if err != nil {
w.WriteHeader(http.StatusInternalServerError)
return
}
w.Header().Set("Content-Length", fmt.Sprintf("%d", len(payload)))
w.Header().Set("Content-Type", "application/json")
w.Header().Set("X-Device-Plugin", "purger")
w.WriteHeader(http.StatusOK)
_, _ = w.Write(payload)
}))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| purgeTaskMap := make(map[string]uint64) | ||
|
|
||
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { |
Copilot
AI
Jan 21, 2026
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.
The binary.LittleEndian.Uint64(val) call will panic if val is less than 8 bytes long. Add a length check to ensure len(val) >= 8 before calling Uint64, or skip entries with invalid value lengths to prevent a potential runtime panic.
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { | |
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { | |
| if len(val) < 8 { | |
| // Skip entries with invalid value lengths to avoid panic in Uint64 | |
| return nil | |
| } |
| sharedkv := storage.Current().SharedKV() | ||
|
|
||
| router.Handle("/plugin/purge/tasks", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { | ||
| // TODO: query sharedkv purge task list | ||
| // query sharedkv purge task list | ||
|
|
||
| purgeTaskMap := make(map[string]uint64) | ||
|
|
||
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { |
Copilot
AI
Jan 21, 2026
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.
The sharedkv variable is captured from storage.Current().SharedKV() at AddRouter initialization time (outside the handler), rather than being retrieved inside the handler function when it's actually needed. If storage.Current() can change or be reinitialized during the application lifecycle, this could lead to stale references. Consider retrieving the SharedKV inside the handler function to ensure you always have the current storage instance, consistent with the pattern used in HandleFunc at line 113.
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { | ||
| purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val) | ||
| return nil | ||
| }) |
Copilot
AI
Jan 21, 2026
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.
The error returned by IteratePrefix is being ignored. If the iteration fails, the handler will still return a 200 OK status with potentially incomplete data. Consider checking the error and returning an appropriate HTTP error status if the iteration fails.
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { | |
| purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val) | |
| return nil | |
| }) | |
| if err := sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { | |
| purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val) | |
| return nil | |
| }); err != nil { | |
| w.WriteHeader(http.StatusInternalServerError) | |
| return | |
| } |
| purgeTaskMap := make(map[string]uint64) | ||
|
|
||
| sharedkv.IteratePrefix(req.Context(), []byte("dir/"), func(key, val []byte) error { | ||
| purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val) |
Copilot
AI
Jan 21, 2026
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.
The string slicing operation string(key)[4:] will panic if the key length is less than 4 bytes. Before slicing, add a length check to ensure len(key) >= 4, or skip keys that are too short to prevent a potential runtime panic.
| purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val) | |
| if len(key) >= 4 { | |
| purgeTaskMap[string(key)[4:]] = binary.LittleEndian.Uint64(val) | |
| } |
This pull request adds Prometheus metrics tracking to the purge plugin and improves the
/plugin/purge/tasksendpoint to return the current purge task list in JSON format. The most important changes are grouped below.Metrics and Observability:
_metricPurgeRequestsTotalinmetrics.goto track the total number of purge requests, labeled by response code (200, 403, 404, 500). This metric is incremented at appropriate points in the purge handler to reflect request outcomes. [1] [2] [3] [4] [5] [6] [7] [8]API Improvements:
/plugin/purge/tasksendpoint to query the shared key-value store for purge tasks, aggregate them into a map, and return the results as a JSON response using the default codec.Codebase Maintenance:
encoding/binaryand the internalencodingpackage to support the updated endpoint logic. [1] [2]