Skip to content

Conversation

@rishabhmalikMS
Copy link
Contributor

@rishabhmalikMS rishabhmalikMS commented Dec 10, 2025

Context

This PR integrates the Node.js version selection strategy framework with the NodeHandler to enable feature flag-controlled testing of the new orchestration pattern. This builds on the previously established strategy framework and provides the foundation for Node.js version handling across both host and container environments.

Related Work: This is part 2 of the Node.js strategy modernization effort, following the strategy framework implementation in PR #5422 and preceding the comprehensive test restoration and container integration phases.

AB#2339020
AB#2339021
AB#2339022


Description

Integrated the NodeVersionOrchestrator with NodeHandler to enable Node.js version selection:

NodeHandler.cs Changes:

  • Added GetNodeLocationWithStrategy() method that creates context and calls orchestrator
  • Integrated feature flag AGENT_USE_NODE_STRATEGY to control strategy selection (Name of feature flag is likely to be updated)
  • Maintained custom node path early return for separate PR handling for custom node related work.
  • Added comprehensive debug logging with [Strategy] prefix for clear traceability
  • Preserved all existing legacy logic as fallback when feature flag is disabled

NodeVersionOrchestrator.cs Updates:

  • Not covered CustomNodeStrategy from default strategy registration (to be handled in separate PR)
  • Maintained strategy priority order: Node24 → Node20 → Node16 → Node10 → Node6
  • Enhanced logging to include environment type ([Container] vs [Host]) for better debugging

Key Integration Points:

  • Context building with all required runtime data (glibc flags, container info, handler data)
  • Result processing with warning display and telemetry integration
  • Exception propagation for EOL policy violations

Updates from Previous PR comments

  • Removed output fields such as selected node, reason, warning from NodeContext
  • Reusing the NodeRunnerInfo for these details
  • Removed getNodePath from inside each strategy and moved it one level up to orchestrator further simplifying the individual strategy.

Risk Assessment (Low / Medium / High)

Low Risk - Changes are protected by feature flag (false by default). All existing NodeHandler logic remains unchanged and continues to serve as the primary code path. The strategy is only invoked when explicitly enabled, allowing for controlled testing and gradual rollout.


Unit Tests Added or Updated (Yes / No)

No unit tests modified in this integration PR #5421 5421. Existing comprehensive test specifications established in previous PR will be activated and expanded in the subsequent test restoration PR to validate equivalence between legacy and new strategies.


Additional Testing Performed

List manual or automated tests performed beyond unit tests (e.g., integration, scenario, regression).


Change Behind Feature Flag (Yes / No)

Yes - Controlled by AGENT_USE_NODE_STRATEGY (default: false). This enables safe testing and gradual rollout without affecting existing functionality. Legacy NodeHandler logic remains the default execution path.


Tech Design / Approach

  • Design has been written and reviewed.
  • Any architectural decisions, trade-offs, and alternatives are captured.

Documentation Changes Required (Yes/No)

Indicate whether related documentation needs to be updated.

  • User guides, API specs, system diagrams, or runbooks are updated.

Logging Added/Updated (Yes/No)

  • Strategy Selection Logging - Clear [Strategy] prefix for strategy execution tracing
  • Context Information - Handler type, glibc compatibility, container mode logged for debugging
  • Result Tracking - Selected strategy, node version, and selection reasoning captured
  • Environment Distinction - [Container]/[Host] prefixes in orchestrator for execution context clarity
  • Exception Context - Enhanced exception messages include "trying next strategy" guidance

Telemetry Added/Updated (Yes/No)

  • Custom telemetry (e.g., counters, timers, error tracking) is added as needed.
  • Events are tagged with proper metadata for filtering and analysis.
  • Telemetry is validated in staging or test environments.

Rollback Scenario and Process (Yes/No)

  • Rollback plan is documented.

Dependency Impact Assessed and Regression Tested (Yes/No)

  • All impacted internal modules, APIs, services, and third-party libraries are analyzed.
  • Results are reviewed and confirmed to not break existing functionality.

@rishabhmalikMS rishabhmalikMS changed the title Users/rishabhmalik ms/node handler orchestrator Node handler integration with Node Version Orchestrator Dec 10, 2025
@rishabhmalikMS
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rishabhmalikMS
Copy link
Contributor Author

This PR is closed as we are covering this integration work in PR #5422
along with L0 tests for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants