[CLEAN] Synthetic Benchmark PR #137964 - Fix for GET /_migration/deprecations doesn't report node deprecations if low watermark exceeded and GET /_migration/deprecations doesn't report node-level failures properly

[tomerqodo]

User description

Benchmark PR elastic#137964

Type: Clean (correct implementation)

Original PR Title: Fix for GET /_migration/deprecations doesn't report node deprecations if low watermark exceeded and GET /_migration/deprecations doesn't report node-level failures properly
Original PR Description: Closes elastic#137010
Closes elastic#137004
Original PR URL: elastic#137964

---

PR Type

Bug fix

---

Description

• Fix node deprecation reporting when low watermark exceeded

• Improve node-level failure logging with proper exception context

• Refactor deprecation check logic to include watermark issues

• Add comprehensive test for disk watermark deprecation warnings

---

Diagram Walkthrough

flowchart LR
  A["Node Deprecation Checker"] -->|logs failures| B["Enhanced Logging"]
  C["Transport Node Deprecation Check"] -->|refactored| D["Improved Issue Collection"]
  D -->|includes| E["Disk Watermark Issues"]
  F["Test Suite"] -->|validates| E

Loading

File Walkthrough

	Relevant files
Bug fix	NodeDeprecationChecker.java Enhanced failure logging with exception context                    x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecker.java Changed debug logging to warn level with exception context Uses logger.atWarn().withThrowable() for better error reporting Improves visibility of node-level deprecation check failures +1/-1      TransportNodeDeprecationCheckAction.java Refactor deprecation issue collection logic                            x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckAction.java Removed unused NodeRequest parameter from nodeOperation method Replaced stream-based filtering with explicit loop for null handling Ensures disk watermark issues are properly included in results Removed unnecessary Objects import +14/-7
Tests	TransportNodeDeprecationCheckActionTests.java Add watermark deprecation test and update signatures          x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckActionTests.java Updated test calls to match refactored nodeOperation signature Added new test testDiskLowWatermarkIsIncludedInDeprecationWarnings Validates that disk watermark issues appear alongside other deprecations Added custom DeprecationIssueMatcher for flexible assertion matching Added necessary imports for test utilities and matchers +102/-4
Documentation	137964.yaml Add changelog entry for bug fix                                                    docs/changelog/137964.yaml New changelog entry documenting the bug fix References issues GET /_migration/deprecations doesn't report node-level failures properly elastic/elasticsearch#137010 and GET /_migration/deprecations doesn't report node deprecations if low watermark exceeded elastic/elasticsearch#137004 Categorized as infrastructure/core bug fix +9/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Logging Context: The PR adds warn-level logs for node failures, but does not include explicit user identity or structured fields, making it unclear if audit requirements are met for these critical actions. Referred Code logger.atWarn().withThrowable(failure).log("node {} failed to run deprecation checks", failure.nodeId()); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Input Assumptions: The change replaces stream filtering with a loop and processes deprecation checks without visible validation of external inputs (e.g., settings or cluster info), which may rely on validations outside the shown diff. Referred Code List<DeprecationIssue> issues = new ArrayList<>(); for (NodeDeprecationChecks.NodeDeprecationCheck< Settings, PluginsAndModules, ClusterState, XPackLicenseState, DeprecationIssue> c : nodeSettingsChecks) { DeprecationIssue deprecationIssue = c.apply(filteredNodeSettings, pluginsService.info(), filteredClusterState, licenseState); if (deprecationIssue != null) { issues.add(deprecationIssue); } } DeprecationIssue watermarkIssue = checkDiskLowWatermark( Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use instanceof pattern matching Simplify the matches method by using instanceof pattern matching to combine the type check and variable binding into a single, more concise step. x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckActionTests.java [289-296] @Override public boolean matches(Object actual) { - if (actual instanceof DeprecationIssue == false) { - return false; + if (actual instanceof DeprecationIssue actualDeprecationIssue) { + return level.equals(actualDeprecationIssue.getLevel()) && messageMatcher.matches(actualDeprecationIssue.getMessage()); } - DeprecationIssue actualDeprecationIssue = (DeprecationIssue) actual; - return level.equals(actualDeprecationIssue.getLevel()) && messageMatcher.matches(actualDeprecationIssue.getMessage()); + return false; } Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion correctly proposes using instanceof pattern matching for better readability, but it's a minor style improvement in a test helper class with low impact.	Low
More