User level resources #3
Conversation
* S3 classes and methods for prom_range and prom_instant * prom_date formatter helper * edit tests
* Add time formatting utilities
There was a problem hiding this comment.
Pull Request Overview
This PR adds user-level resource monitoring functions and enhances cost filtering capabilities. It provides more user-friendly functions to get data summaries at the user level and overall summaries by integrating Prometheus queries for resource usage tracking with improved AWS Cost Explorer filtering.
- Enhanced Prometheus query handling with S3 classes and methods for better data formatting
- Added user-level resource monitoring functions for CPU, memory, and directory usage
- Extended cost summaries with hub and cluster filtering capabilities
Reviewed Changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| R/prometheus-usage-summaries.R | Adds new user-level functions for directory snapshots, resource requests/usage tracking |
| R/prometheus.R | Refactors query result formatting with S3 classes and improved time handling |
| R/cost-summaries.R | Extends cost filtering with hub and cluster parameters plus service component mapping |
| R/cost-explorer.R | Adds service mapping and AWS tag filtering functions |
| R/utils.R | Adds time formatting utilities for Prometheus queries |
| tests/testthat/test-*.R | Comprehensive test coverage for new functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| Default is today (\code{Sys.Date()}). If a \code{POSIXt} object, it will be converted | ||
| to UTC, if a \code{Date} or \code{character} object, it will be assumed to be \code{UTC}} | ||
|
|
||
| \item{by_user}{A logical value indicating whether to group by user (directory). Defau} |
There was a problem hiding this comment.
Documentation for the 'by_user' parameter is incomplete. The description ends abruptly with 'Defau' instead of 'Default FALSE'.
| \item{by_user}{A logical value indicating whether to group by user (directory). Defau} | |
| \item{by_user}{A logical value indicating whether to group by user (directory). Default is \code{FALSE}.} |
| day would be \code{"24h0m0s"}. Default is 1 hour ()} | ||
| } | ||
| \value{ | ||
| A data frame of directory sizes over time, with dirctory sizes in megabytes. |
There was a problem hiding this comment.
Corrected spelling of 'dirctory' to 'directory'.
| A data frame of directory sizes over time, with dirctory sizes in megabytes. | |
| A data frame of directory sizes over time, with directory sizes in megabytes. |
|
|
||
| hub <- match.arg(hub) | ||
|
|
||
| cluster = match.arg(cluster) |
There was a problem hiding this comment.
Assignment operator should be '<-' instead of '=' for consistency with R style guidelines.
| cluster = match.arg(cluster) | |
| cluster <- match.arg(cluster) |
| ) | ||
|
|
||
| if (hub == "support") { | ||
| filter_list = list( |
There was a problem hiding this comment.
Assignment operator should be '<-' instead of '=' for consistency with R style guidelines.
| ) | ||
| ) | ||
| } else if (hub != "all") { | ||
| filter_list = list( |
There was a problem hiding this comment.
Assignment operator should be '<-' instead of '=' for consistency with R style guidelines.
| filter_list = list( | |
| filter_list <- list( |
I got stalled on this, but I think it's time to finish where we got to and merge. This adds more user-friendly functions to get data summaries at the user level, and also for overall summaries.