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

Do not request "search_pipelines" metrics by default in NodesInfoRequest #12497

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

lukas-vlcek
Copy link
Contributor

@lukas-vlcek lukas-vlcek commented Feb 29, 2024

Description

This commit introduces a breaking change:

NodesInfoRequest does not request all metrics by default.

There are metrics there are not included in the default set of metrics. Right now "search_pipelines" metric is not included in the default set of metrics.

Related Issues

Related issue: #10296

Originally, this change was part of above mentioned PR but during the review it was decided that it needs to be broken into several commits.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • [ ] Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

Copy link
Contributor

❌ Gradle check result for 6997752: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for b04720a: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

github-actions bot commented Feb 29, 2024

Compatibility status:

Checks if related components are compatible with change dfe0e9a

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/performance-analyzer.git]

This commit introduces a breaking change:

NodesInfoRequest does not request all metrics by default.

There are metrics there are not included in the default set of metrics. Right
now "search_pipelines" metric is not included in the default set of metrics.

Signed-off-by: Lukáš Vlček <[email protected]>
@lukas-vlcek
Copy link
Contributor Author

The missing documentation part of this PR is "blocked" by opensearch-project/documentation-website#6544 (The documentation for search_pipelines parameter in NodesInfo is missing so I can not update the documentation and say this parameter is not requested by default.)

Copy link
Contributor

✅ Gradle check result for dfe0e9a: SUCCESS

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 71.41%. Comparing base (7921175) to head (dfe0e9a).
Report is 1 commits behind head on main.

Files Patch % Lines
...ction/search/PutSearchPipelineTransportAction.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #12497      +/-   ##
============================================
+ Coverage     71.37%   71.41%   +0.03%     
+ Complexity    59891    59886       -5     
============================================
  Files          4972     4972              
  Lines        281783   281800      +17     
  Branches      40910    40914       +4     
============================================
+ Hits         201136   201251     +115     
+ Misses        63977    63870     -107     
- Partials      16670    16679       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I'm good with this as a breaking change for 3.0, merging.

@dblock dblock merged commit f873d0e into opensearch-project:main Mar 12, 2024
35 of 36 checks passed
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…est (opensearch-project#12497)

This commit introduces a breaking change:

NodesInfoRequest does not request all metrics by default.

There are metrics there are not included in the default set of metrics. Right
now "search_pipelines" metric is not included in the default set of metrics.

Signed-off-by: Lukáš Vlček <[email protected]>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…est (opensearch-project#12497)

This commit introduces a breaking change:

NodesInfoRequest does not request all metrics by default.

There are metrics there are not included in the default set of metrics. Right
now "search_pipelines" metric is not included in the default set of metrics.

Signed-off-by: Lukáš Vlček <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
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.

2 participants