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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constants added for searchbackpressure rca reader to consume searchbackpressure metrics in shared folder #34

Merged
merged 3 commits into from
Jul 3, 2023

Conversation

CoderJeffrey
Copy link
Contributor

(Signed off-by: Jeffrey Liu [email protected])

Is your feature request related to a problem? Please provide an existing Issue # , or describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Describe the solution you are proposing
To fix on issue opensearch-project/performance-analyzer-rca#424. The PR mainly adds constatns required forthe reader to consume SearchBackPressure Service metrics.

Describe alternatives you've considered
N/A

Additional context

Check List

  • New functionality includes testing.
  • All tests pass
  • New functionality has been documented.
  • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

// Table Name stored in sqlitedb
public static final String SEARCHBP_TABLE_NAME = "Searchbp_Stats";
// Column Names for difference stats stored in sqlitedb
public static final String SEARCHBP_TYPE_DIM = "SearchBackPressureStats";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some details about the purpose of this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in latest commit.
Also paste here for better visibility.

Add SEARCHBP_TABLE_NAME for searchbackpressureRCA to find the corresponding table
Add SEARCHBP_TYPE_DIM for searchbackpressureRCA to find the cell value for each stats type

Copy link
Collaborator

Choose a reason for hiding this comment

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

@CoderJeffrey can you plz add comments for 2 constants separately?
e.g.

            /**
             * Add SEARCHBP_TABLE_NAME for searchbackpressureRCA to find the corresponding table
             */
            public static final String SEARCHBP_TABLE_NAME = "Searchbp_Stats";
            /**
            * Add SEARCHBP_TYPE_DIM for searchbackpressureRCA to find the cell value for each stats type
            * /
            public static final String SEARCHBP_TYPE_DIM = "SearchBackPressureStats";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest commit added comments for 2 constants separately. Resolved.

@@ -943,7 +943,9 @@ public enum SearchBackPressureStatsValue implements MetricValue {
SearchBackPressureStatsValue.Constants.SEARCHBP_SEARCH_BACK_PRESSURE_STATS_MODE),
SEARCHBP_SEARCH_BACK_PRESSURE_STATS_SEARCH_TASK_STATS(
SearchBackPressureStatsValue.Constants
.SEARCHBP_SEARCH_BACK_PRESSURE_STATS_SEARCH_TASK_STATS);
.SEARCHBP_SEARCH_BACK_PRESSURE_STATS_SEARCH_TASK_STATS),
SEARCHBP_TABLE_NAME(SearchBackPressureStatsValue.Constants.SEARCHBP_TABLE_NAME),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we replicating this information ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.SEARCHBP_SEARCH_BACK_PRESSURE_STATS_SEARCH_TASK_STATS); change to
.SEARCHBP_SEARCH_BACK_PRESSURE_STATS_SEARCH_TASK_STATS), because it is not the final constants in the list
SEARCHBP_TABLE_NAME(SearchBackPressureStatsValue.Constants.SEARCHBP_TABLE_NAME), use this for easier access and consistent coding style.

@ansjcy ansjcy merged commit c0117af into opensearch-project:main Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants