Include min cpu throttle query in performance profile used in remote monitoring mode.#1889
Open
kusumachalasani wants to merge 1 commit into
Open
Include min cpu throttle query in performance profile used in remote monitoring mode.#1889kusumachalasani wants to merge 1 commit into
kusumachalasani wants to merge 1 commit into
Conversation
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a minimum CPU throttling Prometheus query to the OpenShift resource optimization performance profile used for remote monitoring, complementing the existing average and maximum throttling metrics. Sequence diagram for remote monitoring using min/avg/max CPU throttling metricssequenceDiagram
participant RemoteMonitoringController
participant PerformanceProfile_resource_optimization_openshift
participant Prometheus
RemoteMonitoringController->>PerformanceProfile_resource_optimization_openshift: Load SLO CPU throttling queries
PerformanceProfile_resource_optimization_openshift-->>RemoteMonitoringController: avg, min, max throttling query templates
loop Each monitored container
RemoteMonitoringController->>Prometheus: Execute avg throttling query
Prometheus-->>RemoteMonitoringController: avg rate(container_cpu_cfs_throttled_seconds_total)
RemoteMonitoringController->>Prometheus: Execute min throttling query
Prometheus-->>RemoteMonitoringController: min rate(container_cpu_cfs_throttled_seconds_total)
RemoteMonitoringController->>Prometheus: Execute max throttling query
Prometheus-->>RemoteMonitoringController: max rate(container_cpu_cfs_throttled_seconds_total)
RemoteMonitoringController->>RemoteMonitoringController: Evaluate SLOs using avg, min, max
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
minquery (and the existingavg/maxones) use curly quotes around$CONTAINER_NAME$; these should be plain ASCII double quotes so the PromQL selector parses correctly. - Since the min/avg/max CPU throttling queries are identical except for the aggregation function, consider extracting the common selector into a shared template or variable (if your manifest tooling allows) to avoid duplication and reduce the chance of future inconsistencies.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `min` query (and the existing `avg`/`max` ones) use curly quotes around `$CONTAINER_NAME$`; these should be plain ASCII double quotes so the PromQL selector parses correctly.
- Since the min/avg/max CPU throttling queries are identical except for the aggregation function, consider extracting the common selector into a shared template or variable (if your manifest tooling allows) to avoid duplication and reduce the chance of future inconsistencies.
## Individual Comments
### Comment 1
<location path="manifests/autotune/performance-profiles/resource_optimization_openshift.yaml" line_range="123" />
<code_context>
+ # Minimum CPU throttling per container in a deployment
+ - function: min
+ query: 'min by(container, namespace)(rate(container_cpu_cfs_throttled_seconds_total{container!="", container!="POD", pod!="", namespace="$NAMESPACE$", container=”$CONTAINER_NAME$”}[15m]))'
+
# Maximum CPU throttling per container in a deployment
</code_context>
<issue_to_address>
**issue (bug_risk):** The query uses a Unicode smart quote around $CONTAINER_NAME$, which will break PromQL parsing.
The `container=”$CONTAINER_NAME$”` segment uses a Unicode smart quote (`”`) instead of a standard ASCII double quote (`"`), which Prometheus can’t parse. Please change it to `container="$CONTAINER_NAME$"` so the query parses correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| # Minimum CPU throttling per container in a deployment | ||
| - function: min | ||
| query: 'min by(container, namespace)(rate(container_cpu_cfs_throttled_seconds_total{container!="", container!="POD", pod!="", namespace="$NAMESPACE$", container=”$CONTAINER_NAME$”}[15m]))' |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The query uses a Unicode smart quote around
The container=”$CONTAINER_NAME$” segment uses a Unicode smart quote (”) instead of a standard ASCII double quote ("), which Prometheus can’t parse. Please change it to container="$CONTAINER_NAME$" so the query parses correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Include min cpu throttle query in performance profile used in remote monitoring mode which is missing.
Fixes # (issue)
Type of change
Summary by Sourcery
Enhancements: