feat: Add JVM memory metrics support for accurate Java application resource recommendations #440
Conversation
|
|
|
""" WalkthroughThe changes introduce JVM-specific memory metric loaders and detection logic into the resource recommendation strategy. New classes for querying and parsing JVM memory metrics from Prometheus are added, and the strategy is updated to use these metrics and a distinct buffer percentage when a JVM is detected. Comprehensive unit tests for these features are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant Strategy as SimpleStrategy
participant Metrics as PrometheusMetric Loaders
participant Prometheus as Prometheus
participant K8s as K8sObjectData
Strategy->>Metrics: Request JVMDetector query for object
Metrics->>Prometheus: Query for JVM memory usage
Prometheus-->>Metrics: Return JVM metric data
Metrics-->>Strategy: Indicate JVM presence (or absence)
alt JVM detected
Strategy->>Metrics: Use MaxJVMMemoryLoader and JVMMemoryAmountLoader
Metrics->>Prometheus: Fetch JVM-specific memory metrics
Prometheus-->>Metrics: Return JVM memory data
Metrics-->>Strategy: Provide JVM memory stats
Strategy->>Strategy: Apply JVM buffer percentage
else Not JVM
Strategy->>Metrics: Use standard memory loaders
Metrics->>Prometheus: Fetch standard memory metrics
Prometheus-->>Metrics: Return standard memory data
Metrics-->>Strategy: Provide standard memory stats
Strategy->>Strategy: Apply standard buffer percentage
end
Strategy-->>Strategy: Calculate memory recommendation
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
robusta_krr/strategies/simple.py (1)
86-100:⚠️ Potential issueJVM-specific loaders are never scheduled – will raise
KeyErrorat runtime
__calculate_memory_proposalexpectshistory_datato contain"MaxJVMMemoryLoader"and"JVMMemoryAmountLoader", yet themetricsproperty only registersJVMDetectoralongside the generic loaders.
In a JVM workload the firsthistory_data[...]access will therefore blow up with aKeyError.Add the two JVM loaders to the list (optionally only when JVM support is enabled):
metrics = [ PercentileCPULoader(self.settings.cpu_percentile), MaxMemoryLoader, CPUAmountLoader, MemoryAmountLoader, JVMDetector, + MaxJVMMemoryLoader, + JVMMemoryAmountLoader, ]This also eliminates the Ruff “unused import” warnings on lines 24-27.
🧹 Nitpick comments (3)
tests/test_jvm_metrics.py (1)
1-4: Remove unuseddatetime/timedeltaimports to silence RuffThey are not referenced anywhere in this module.
-import pytest -from datetime import datetime, timedelta -import numpy as np +import pytest +import numpy as np🧰 Tools
🪛 Ruff (0.11.9)
2-2:
datetime.datetimeimported but unusedRemove unused import
(F401)
2-2:
datetime.timedeltaimported but unusedRemove unused import
(F401)
tests/test_jvm_strategy.py (1)
1-11: Prune unused imports (Ruff F401)
datetime,timedelta,MetricsPodData, and all JVM loader symbols are unused in this file – they generate CI noise.-import pytest -import numpy as np -from datetime import datetime, timedelta - -from robusta_krr.core.abstract.strategies import MetricsPodData, K8sObjectData, PodData -from robusta_krr.core.integrations.prometheus.metrics.memory import ( - JVMMemoryLoader, - MaxJVMMemoryLoader, - JVMMemoryAmountLoader, - JVMDetector, -) +import pytest +import numpy as np + +from robusta_krr.core.models.objects import K8sObjectData, PodData + +# No need to import JVM loaders – the strategy accesses them internally🧰 Tools
🪛 Ruff (0.11.9)
3-3:
datetime.datetimeimported but unusedRemove unused import
(F401)
3-3:
datetime.timedeltaimported but unusedRemove unused import
(F401)
5-5:
robusta_krr.core.abstract.strategies.MetricsPodDataimported but unusedRemove unused import:
robusta_krr.core.abstract.strategies.MetricsPodData(F401)
7-7:
robusta_krr.core.integrations.prometheus.metrics.memory.JVMMemoryLoaderimported but unusedRemove unused import
(F401)
8-8:
robusta_krr.core.integrations.prometheus.metrics.memory.MaxJVMMemoryLoaderimported but unusedRemove unused import
(F401)
9-9:
robusta_krr.core.integrations.prometheus.metrics.memory.JVMMemoryAmountLoaderimported but unusedRemove unused import
(F401)
10-10:
robusta_krr.core.integrations.prometheus.metrics.memory.JVMDetectorimported but unusedRemove unused import
(F401)
robusta_krr/core/integrations/prometheus/metrics/memory.py (1)
111-132: Minor: query label list may miss a comma beforeclusterlabelIf
get_prometheus_cluster_label()returns something that does not start with a comma, the rendered PromQL will be syntactically invalid because the precedingcontainer="{object.container}"line lacks a trailing comma.Consider guaranteeing the leading comma inside
get_prometheus_cluster_label()or add one here conditionally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
robusta_krr/core/integrations/prometheus/metrics/memory.py(1 hunks)robusta_krr/strategies/simple.py(7 hunks)tests/test_jvm_metrics.py(1 hunks)tests/test_jvm_strategy.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/test_jvm_metrics.py (3)
robusta_krr/core/integrations/prometheus/metrics/memory.py (12)
JVMMemoryLoader(111-132)MaxJVMMemoryLoader(134-155)JVMMemoryAmountLoader(157-178)JVMDetector(180-199)get_query(13-25)get_query(33-48)get_query(56-71)get_query(81-109)get_query(119-132)get_query(139-155)get_query(162-178)get_query(187-199)robusta_krr/core/models/objects.py (2)
K8sObjectData(38-107)PodData(14-19)tests/test_jvm_strategy.py (1)
mock_pod_data(16-26)
robusta_krr/strategies/simple.py (2)
robusta_krr/core/integrations/prometheus/metrics/memory.py (4)
JVMMemoryLoader(111-132)MaxJVMMemoryLoader(134-155)JVMMemoryAmountLoader(157-178)JVMDetector(180-199)robusta_krr/core/abstract/strategies.py (2)
description(117-122)ResourceRecommendation(21-36)
🪛 Ruff (0.11.9)
tests/test_jvm_metrics.py
2-2: datetime.datetime imported but unused
Remove unused import
(F401)
2-2: datetime.timedelta imported but unused
Remove unused import
(F401)
tests/test_jvm_strategy.py
3-3: datetime.datetime imported but unused
Remove unused import
(F401)
3-3: datetime.timedelta imported but unused
Remove unused import
(F401)
5-5: robusta_krr.core.abstract.strategies.MetricsPodData imported but unused
Remove unused import: robusta_krr.core.abstract.strategies.MetricsPodData
(F401)
7-7: robusta_krr.core.integrations.prometheus.metrics.memory.JVMMemoryLoader imported but unused
Remove unused import
(F401)
8-8: robusta_krr.core.integrations.prometheus.metrics.memory.MaxJVMMemoryLoader imported but unused
Remove unused import
(F401)
9-9: robusta_krr.core.integrations.prometheus.metrics.memory.JVMMemoryAmountLoader imported but unused
Remove unused import
(F401)
10-10: robusta_krr.core.integrations.prometheus.metrics.memory.JVMDetector imported but unused
Remove unused import
(F401)
robusta_krr/strategies/simple.py
24-24: robusta_krr.core.integrations.prometheus.metrics.JVMMemoryLoader imported but unused
Remove unused import
(F401)
25-25: robusta_krr.core.integrations.prometheus.metrics.MaxJVMMemoryLoader imported but unused
Remove unused import
(F401)
26-26: robusta_krr.core.integrations.prometheus.metrics.JVMMemoryAmountLoader imported but unused
Remove unused import
(F401)
🔇 Additional comments (1)
robusta_krr/strategies/simple.py (1)
54-63: Buffer selection now depends onis_jvm– good but drop the unused ternary on NaN
np.max(data_)already returns-infon empty input, but you gate forlen(data_) == 0above; safe. Implementation looks correct after the JVM buffer addition.
- Add MaxJVMMemoryLoader and JVMMemoryAmountLoader to metrics property - Fix KeyError when processing JVM workloads - Resolve unused import warnings for JVM metric loaders This change ensures that JVM-specific metrics are properly collected when a JVM application is detected, preventing runtime KeyError exceptions in __calculate_memory_proposal method.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
robusta_krr/strategies/simple.py (2)
105-112: Update strategy description to reflect the new JVM-specific buffer.The help text still states
Memory request: max + {self.settings.memory_buffer_percentage}% …
which is misleading for JVM workloads now governed byjvm_memory_buffer_percentage. Consider appending a sentence such as:“For JVM containers the buffer defaults to {self.settings.jvm_memory_buffer_percentage}% of max JVM heap.”
Keeps the CLI self-documentation accurate.
174-179: Minor readability: avoid re-usingdata_countvariable name.
data_countfirst holds the loader result object, then is re-assigned to a dict of point counts. Renaming the second usage (e.g.points_per_pod) would prevent mental context switches and ease debugging.No functional impact, pure clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
robusta_krr/strategies/simple.py(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
robusta_krr/strategies/simple.py (2)
robusta_krr/core/integrations/prometheus/metrics/memory.py (4)
JVMMemoryLoader(111-132)MaxJVMMemoryLoader(134-155)JVMMemoryAmountLoader(157-178)JVMDetector(180-199)robusta_krr/core/abstract/strategies.py (2)
description(117-122)ResourceRecommendation(21-36)
🪛 Ruff (0.11.9)
robusta_krr/strategies/simple.py
24-24: robusta_krr.core.integrations.prometheus.metrics.JVMMemoryLoader imported but unused
Remove unused import: robusta_krr.core.integrations.prometheus.metrics.JVMMemoryLoader
(F401)
| JVMMemoryLoader, | ||
| MaxJVMMemoryLoader, | ||
| JVMMemoryAmountLoader, | ||
| JVMDetector, |
There was a problem hiding this comment.
Unused import triggers Ruff F401 – drop or actually use JVMMemoryLoader.
JVMMemoryLoader is imported but never referenced. Ruff flags this as F401, which will fail lint-only CI steps.
Either add the loader to the metrics list or remove the import:
- JVMMemoryLoader,
MaxJVMMemoryLoader,If a plain (non-max) JVM timeseries is not needed here, simply deleting the import keeps the strategy lean and CI green.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| JVMMemoryLoader, | |
| MaxJVMMemoryLoader, | |
| JVMMemoryAmountLoader, | |
| JVMDetector, | |
| MaxJVMMemoryLoader, | |
| JVMMemoryAmountLoader, | |
| JVMDetector, |
🧰 Tools
🪛 Ruff (0.11.9)
24-24: robusta_krr.core.integrations.prometheus.metrics.JVMMemoryLoader imported but unused
Remove unused import: robusta_krr.core.integrations.prometheus.metrics.JVMMemoryLoader
(F401)
🤖 Prompt for AI Agents
In robusta_krr/strategies/simple.py around lines 24 to 27, the import
JVMMemoryLoader is unused and triggers a Ruff F401 lint error. To fix this,
either remove the JVMMemoryLoader import if it is not needed or add
JVMMemoryLoader to the metrics list or wherever appropriate in the code to
ensure it is used. This will resolve the lint error and keep the code clean.
Description
This PR addresses the issue #367 by adding support for JVM memory metrics to provide more accurate resource recommendations for Java applications. Currently, KRR relies on container-level memory metrics, which can be misleading for JVM applications since they often pre-allocate memory. This change introduces JVM-specific memory analysis using heap memory metrics.
Changes
New JVM Metrics Support
JVMMemoryLoader: Collects JVM heap memory usageMaxJVMMemoryLoader: Collects maximum JVM heap memory usageJVMMemoryAmountLoader: Counts JVM memory data pointsJVMDetector: Detects if a container is running a JVM applicationStrategy Updates
SimpleStrategyto support JVM memory analysis:Configuration
jvm_memory_buffer_percentage: Configurable buffer percentage for JVM applications (default: 30%)Testing
Added comprehensive test coverage:
JVM Metrics Tests (
test_jvm_metrics.py)Strategy Tests (
test_jvm_strategy.py)Usage
The JVM memory analysis is automatically enabled when JVM metrics are detected. No additional configuration is required if your Prometheus is already collecting JVM metrics.
Example usage:
Requirements
jvm_memory_bytes_usedwitharea="heap"labelRelated Issues
Closes #367