Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Support For Default DataSource and Removing Monitoring Agent #1095

Open
wants to merge 11 commits into
base: kruize_local_poc
Choose a base branch
from

Conversation

shekhar316
Copy link
Contributor

@shekhar316 shekhar316 commented Feb 6, 2024

Description

This PR will add the support for setting up the default datasource instead of monitoring agent. Default datasource will use the newly implemented DataSourceInfo class objects.

Type of change

  • Refactoring Code
  • New feature

How has this been tested?

Using kruize-demo scripts.
Image: quay.io/rh-ee-shesaxen/autotune:kruize-local-ma-0

Test Configuration

  • Kubernetes clusters tested on: minikube

Checklist 馃幆

  • Followed coding guidelines
  • Comments added
  • Dependent changes merged
  • Documentation updated
  • Tests added or updated

Additional information

Remote Monitoring Demo Logs: https://pastebin.com/8JtWxsi5

@shekhar316 shekhar316 self-assigned this Feb 6, 2024
@shekhar316 shekhar316 added the kruize-local Tag for mentioning all the PR's and issues raised which covers the kruize local monitoring usecase label Feb 6, 2024
@shekhar316 shekhar316 changed the title Adding Support For Default DataSource and Removing Monitoring Agent [WIP] Adding Support For Default DataSource and Removing Monitoring Agent Feb 6, 2024
@dinogun
Copy link
Contributor

dinogun commented Mar 18, 2024

@shekhar316 Please fix the conflicts.
@msvinaykumar Can you please review

@shekhar316 shekhar316 changed the title [WIP] Adding Support For Default DataSource and Removing Monitoring Agent Adding Support For Default DataSource and Removing Monitoring Agent Mar 19, 2024
@msvinaykumar
Copy link
Contributor

ok

Copy link
Contributor

@msvinaykumar msvinaykumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -336,6 +336,7 @@ private Memory() {

public static class DataSourceConstants {
public static final String DATASOURCE_NAME = "name";
public static final String DEFAULT_DATASOURCE_NAME = "prometheus";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep the naming consistent, this should be DEFAULT_DATASOURCE_PROVIDER

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dinogun It's the name of the datasource which should be default. Originally, the monitoring agent was always linked to a datasource named "prometheus". Now, "prometheus" is the default name for the default datasource, but it can be changed to something else if desired.

@@ -29,7 +29,7 @@ private KruizeSupportedTypes() { }
public static final Set<String> DIRECTIONS_SUPPORTED =
new HashSet<>(Arrays.asList("minimize", "maximize"));

public static final Set<String> MONITORING_AGENTS_SUPPORTED =
public static final Set<String> DATASOURCES_SUPPORTED =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be DATASOURCE_PROVIDERS_SUPPORTED

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kruize-local Tag for mentioning all the PR's and issues raised which covers the kruize local monitoring usecase remote_monitoring
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants