[PR-3] Runtime Integration#1954
Open
khansaad wants to merge 7 commits into
Open
Conversation
Contributor
Reviewer's GuideIntegrates multi-datasource awareness (with Prometheus as mandatory for metrics) throughout experiment validation, layer detection, and recommendation runtime, while preserving backward compatibility with the legacy single Sequence diagram for Prometheus datasource selection in RecommendationEnginesequenceDiagram
participant RecommendationEngine
participant KruizeObject
participant DataSourceCollection
participant DataSourceInfo
RecommendationEngine->>KruizeObject: getDatasources()
alt datasources not empty
loop for each ds in datasources
RecommendationEngine->>DataSourceCollection: getInstance()
RecommendationEngine->>DataSourceCollection: getDataSourcesCollection()
RecommendationEngine->>DataSourceCollection: get(ds)
DataSourceCollection-->>RecommendationEngine: DataSourceInfo
alt DataSourceInfo not null
RecommendationEngine->>DataSourceInfo: getProvider()
alt provider is PROMETHEUS
RecommendationEngine->>RecommendationEngine: set dataSource = ds
RecommendationEngine->>RecommendationEngine: break
end
end
end
else datasources empty
RecommendationEngine->>KruizeObject: getDataSource()
alt dataSource not null
RecommendationEngine->>DataSourceCollection: getInstance()
RecommendationEngine->>DataSourceCollection: getDataSourcesCollection()
RecommendationEngine->>DataSourceCollection: get(dataSource)
DataSourceCollection-->>RecommendationEngine: DataSourceInfo
alt DataSourceInfo null or provider not PROMETHEUS
RecommendationEngine->>RecommendationEngine: set dataSource = null
end
end
end
alt dataSource is null
RecommendationEngine->>RecommendationEngine: throw Exception
end
Sequence diagram for PromQL datasource resolution with CryostatsequenceDiagram
participant RecommendationEngine
participant DataSourceCollection
participant DataSourceInfo
RecommendationEngine->>RecommendationEngine: fetchMetricsBasedOnProfileAndDatasource(..., dataSourceInfo, ...)
alt [dataSourceInfo.provider is CRYOSTAT]
RecommendationEngine->>RecommendationEngine: promQLDataSourceInfo = dataSourceInfo
RecommendationEngine->>DataSourceCollection: getInstance()
RecommendationEngine->>DataSourceCollection: getDataSourcesCollection()
loop for each ds in values()
RecommendationEngine->>DataSourceInfo: getProvider()
alt [provider is PROMETHEUS]
RecommendationEngine->>RecommendationEngine: promQLDataSourceInfo = ds
RecommendationEngine->>RecommendationEngine: loop break
end
end
else [dataSourceInfo.provider is not CRYOSTAT]
RecommendationEngine->>RecommendationEngine: promQLDataSourceInfo = dataSourceInfo
end
RecommendationEngine->>RecommendationEngine: fetchContainerMetricsBasedOnDataSourceAndProfile(..., promQLDataSourceInfo, ...)
alt [promQLDataSourceInfo.provider is PROMETHEUS]
RecommendationEngine->>RecommendationEngine: accelerator detection enabled
else [non-PROMETHEUS]
RecommendationEngine->>RecommendationEngine: skip accelerator detection
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- When falling back from a Cryostat datasource to Prometheus in
fetchMetricsBasedOnProfileAndDatasource, you currently select the first Prometheus entry from the globalDataSourceCollection; consider restricting this to the experiment's configured datasources to avoid silently using an unintended Prometheus instance. - RuntimeRecommendationProcessor still treats the datasource as effectively singular (only gating on presence of
getDataSource/getDatasources); if multiple datasources are configured, it may be worth explicitly selecting or validating which one drives runtime recommendations to avoid ambiguous behavior. - The datasource validation logic mixes direct field checks (
getDataSource()+ null checks ongetDatasources()) withgetDatasources()'s backward-compatible behavior; consider standardizing ongetDatasources()everywhere to simplify the conditions and avoid subtle differences between validation paths.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When falling back from a Cryostat datasource to Prometheus in `fetchMetricsBasedOnProfileAndDatasource`, you currently select the first Prometheus entry from the global `DataSourceCollection`; consider restricting this to the experiment's configured datasources to avoid silently using an unintended Prometheus instance.
- RuntimeRecommendationProcessor still treats the datasource as effectively singular (only gating on presence of `getDataSource`/`getDatasources`); if multiple datasources are configured, it may be worth explicitly selecting or validating which one drives runtime recommendations to avoid ambiguous behavior.
- The datasource validation logic mixes direct field checks (`getDataSource()` + null checks on `getDatasources()`) with `getDatasources()`'s backward-compatible behavior; consider standardizing on `getDatasources()` everywhere to simplify the conditions and avoid subtle differences between validation paths.
## Individual Comments
### Comment 1
<location path="src/main/java/com/autotune/analyzer/serviceObjects/CreateExperimentAPIObject.java" line_range="177-178" />
<code_context>
+ */
+ public List<String> getDatasources() {
+ // Backward compatibility: if datasources is null but datasource is set
+ if (datasources == null && datasource != null) {
+ return Arrays.asList(datasource);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `List.of` for backward compatibility may cause issues on Java 8 runtimes.
`KruizeObject#getDatasources` already uses `Arrays.asList(datasource)` for this backward-compatibility case. For consistency and to avoid Java 8 compilation issues, use `Collections.singletonList(datasource)` or `Arrays.asList(datasource)` instead of `List.of(datasource)` here.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
03b6cd6 to
7e13736
Compare
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
Signed-off-by: Saad Khan <saakhan@ibm.com>
7e13736 to
8b5aaac
Compare
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
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
This PR integrates multi-datasource layer handling into the runtime recommendation flow so detected layer information is available during recommendation processing.
Fixes # (issue)
Type of change
How has this been tested?
Please describe the tests that were run to verify your changes and steps to reproduce. Please specify any test configuration required.
Test Configuration
Checklist 🎯
Additional information
Include any additional information such as links, test results, screenshots here
Summary by Sourcery
Add multi-datasource support to experiments and integrate it into layer detection, validation, and recommendation flows while preserving backward compatibility with the existing single datasource field.
New Features:
Bug Fixes:
Enhancements: