Skip to content

feat: Parameterize health and live HTTP endpoint paths#2230

Merged
grahamking merged 9 commits intomainfrom
yinggeh-DIS-240-parameterize-server-status-endpoints
Aug 5, 2025
Merged

feat: Parameterize health and live HTTP endpoint paths#2230
grahamking merged 9 commits intomainfrom
yinggeh-DIS-240-parameterize-server-status-endpoints

Conversation

@yinggeh
Copy link
Contributor

@yinggeh yinggeh commented Jul 31, 2025

Overview:

This PR adds parameterization support for health and live HTTP endpoint paths, allowing custom configuration of server status endpoints via environment variables.

Details:

Changes Made

  • Added environment variable support for custom health and live endpoint paths
  • Updated configuration system to read custom path settings from environment variables
  • Implemented parameterized routing to handle both default and custom endpoint configurations
  • Enhanced test coverage to validate custom endpoints functionality

Environment Variables Added

  • DYN_SYSTEM_HEALTH_PATH - Custom health endpoint path
  • DYN_SYSTEM_LIVE_PATH - Custom live endpoint path

Test Coverage

  • ✅ Default endpoints (/health, /live) when no custom paths configured
  • ✅ Custom endpoints when environment variables are set
  • ✅ 404 responses for non-existent paths
  • ✅ Proper fallback handling for unmatched endpoint routes

Where should the reviewer start?

  1. Configuration changes - Review environment variable handling
  2. HTTP server routing - Check parameterized endpoint logic
  3. Integration tests - Verify custom path validation

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

Summary by CodeRabbit

  • New Features

    • Added support for customizing health and live endpoint paths via configuration and environment variables.
    • Health and live endpoints now default to /health and /live, but can be changed at runtime.
  • Tests

    • Enhanced integration tests to verify custom and default health/live endpoint behavior.

@yinggeh yinggeh self-assigned this Jul 31, 2025
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jul 31, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@yinggeh yinggeh requested review from keivenchang and nnshah1 July 31, 2025 22:55
@yinggeh yinggeh force-pushed the yinggeh-DIS-240-parameterize-server-status-endpoints branch from 3993372 to 510e57a Compare July 31, 2025 23:02
@yinggeh yinggeh marked this pull request as ready for review July 31, 2025 23:17
@yinggeh yinggeh requested a review from a team as a code owner July 31, 2025 23:17
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 31, 2025

Walkthrough

This change introduces configurable health and live endpoint paths to the runtime system. New fields and environment variable support are added to the configuration, propagated through the runtime initialization, and utilized by the HTTP server. Tests are updated to validate both default and custom endpoint path behaviors.

Changes

Cohort / File(s) Change Summary
Configuration Struct & Loading
lib/runtime/src/config.rs
Added system_health_path and system_live_path fields to RuntimeConfig, with defaults and environment variable support. Updated constructors, environment mapping, Display implementation, and added unit tests for path configuration.
Distributed Runtime Initialization
lib/runtime/src/distributed.rs
Updated DistributedRuntime constructor to extract and pass the new health and live path values to SystemHealth::new.
HTTP Server & Integration Tests
lib/runtime/src/http_server.rs
Modified HTTP server to use configurable health/live paths from runtime state. Enhanced integration test to cover custom and default endpoint paths, and removed unused test code.
SystemHealth Struct & Constructor
lib/runtime/src/lib.rs
Added health_path and live_path fields to SystemHealth and updated its constructor to accept these parameters.

Sequence Diagram(s)

sequenceDiagram
    participant Env as Environment
    participant Config as RuntimeConfig
    participant Dist as DistributedRuntime
    participant SysHealth as SystemHealth
    participant HttpServer as HTTP Server

    Env->>Config: Provide DYN_SYSTEM_HEALTH_PATH / DYN_SYSTEM_LIVE_PATH
    Config->>Dist: Pass system_health_path, system_live_path
    Dist->>SysHealth: Initialize with paths
    SysHealth->>HttpServer: Expose health_path, live_path
    HttpServer->>Client: Serve health/live endpoints at configured paths
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

Poem

A rabbit hops by servers' side,
Where health and live endpoints now reside.
Configurable paths, both fresh and new,
Let you choose where your checks hop through!
With tests that bound and code that thrives,
This warren’s endpoints come alive.
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f10e44c and 510e57a.

📒 Files selected for processing (4)
  • lib/runtime/src/config.rs (7 hunks)
  • lib/runtime/src/distributed.rs (1 hunks)
  • lib/runtime/src/http_server.rs (5 hunks)
  • lib/runtime/src/lib.rs (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: nnshah1
PR: ai-dynamo/dynamo#1444
File: tests/fault_tolerance/scenarios.py:57-57
Timestamp: 2025-07-01T15:39:56.789Z
Learning: The fault tolerance tests in tests/fault_tolerance/ are designed to run only in the mounted container environment, so hardcoded absolute paths with `/workspace/` prefix are intentional and should not be changed to relative paths.
📚 Learning: the asyncenginecontextprovider trait in lib/runtime/src/engine.rs was intentionally changed from `se...
Learnt from: ryanolson
PR: ai-dynamo/dynamo#1919
File: lib/runtime/src/engine.rs:168-168
Timestamp: 2025-07-14T21:25:56.930Z
Learning: The AsyncEngineContextProvider trait in lib/runtime/src/engine.rs was intentionally changed from `Send + Sync + Debug` to `Send + Debug` because the Sync bound was overly constraining. The trait should only require Send + Debug as designed.

Applied to files:

  • lib/runtime/src/distributed.rs
  • lib/runtime/src/http_server.rs
  • lib/runtime/src/config.rs
📚 Learning: in lib/runtime/src/component/client.rs, the current mutex usage in get_or_create_dynamic_instance_so...
Learnt from: grahamking
PR: ai-dynamo/dynamo#1962
File: lib/runtime/src/component/client.rs:270-273
Timestamp: 2025-07-16T12:41:12.543Z
Learning: In lib/runtime/src/component/client.rs, the current mutex usage in get_or_create_dynamic_instance_source is temporary while evaluating whether the mutex can be dropped entirely. The code currently has a race condition between try_lock and lock().await, but this is acknowledged as an interim state during the performance optimization process.

Applied to files:

  • lib/runtime/src/http_server.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: pre-merge-rust (.)
  • GitHub Check: Build and Test - vllm
🔇 Additional comments (15)
lib/runtime/src/distributed.rs (2)

91-92: LGTM! Proper extraction of configurable endpoint paths.

The health and live endpoint paths are correctly extracted from the runtime configuration, supporting the parameterization of these endpoints.


96-97: LGTM! Correct propagation to SystemHealth constructor.

The extracted endpoint paths are properly passed to the SystemHealth::new constructor, maintaining consistency with the updated constructor signature.

lib/runtime/src/lib.rs (3)

90-91: LGTM! Proper field additions for configurable endpoints.

The health_path and live_path fields are correctly added to the SystemHealth struct to support configurable endpoint paths.


98-99: LGTM! Constructor signature properly updated.

The constructor parameters are correctly updated to accept the health and live endpoint paths, maintaining consistency with the new struct fields.


109-110: LGTM! Proper field initialization.

The new fields are correctly initialized with the provided parameters, completing the implementation of configurable endpoint paths.

lib/runtime/src/http_server.rs (4)

154-155: LGTM! Proper access to system health configuration.

The code correctly obtains a lock on the system health configuration to access the dynamic endpoint paths.


164-171: LGTM! Dynamic routing implementation.

The HTTP server now correctly uses the configurable health and live endpoint paths instead of hardcoded routes, enabling runtime customization of these endpoints.


399-408: LGTM! Comprehensive test case coverage.

The test cases properly cover both default and custom endpoint path scenarios, ensuring the functionality works correctly in all configurations.


430-472: LGTM! Thorough test implementation.

The test logic correctly:

  • Sets up environment variables for custom paths
  • Validates that default paths return 404 when custom paths are configured
  • Verifies that custom paths respond correctly
  • Maintains existing test coverage for unknown paths
lib/runtime/src/config.rs (6)

20-22: LGTM! Well-defined default constants.

The default endpoint path constants are properly defined with sensible default values that follow REST API conventions.


118-126: LGTM! Proper configuration field implementation.

The new configuration fields are correctly implemented with:

  • Appropriate builder attributes and defaults
  • Clear documentation referencing environment variables
  • Consistent naming patterns

188-189: LGTM! Correct environment variable mapping.

The environment variable mapping properly translates DYN_SYSTEM_HEALTH_PATH and DYN_SYSTEM_LIVE_PATH to their corresponding configuration field names.


151-152: LGTM! Display implementation updated.

The Display trait implementation is properly updated to include the new configuration fields, maintaining consistent formatting.


228-229: LGTM! Consistent default initialization.

Both the single_threaded() method and Default implementation correctly initialize the new fields with the defined constants.

Also applies to: 257-258


457-490: LGTM! Comprehensive test coverage.

The unit tests thoroughly verify:

  • Default values when environment variables are not set
  • Custom values when environment variables are configured
  • Both health and live path configurations

This ensures the configuration system works correctly for the new endpoint path features.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@keivenchang keivenchang left a comment

Choose a reason for hiding this comment

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

Overall looks good, but I have a question for both @nnshah1 @yinggeh regarding the removed code for tracing.

Copy link
Contributor

@nnshah1 nnshah1 left a comment

Choose a reason for hiding this comment

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

LGTM

@yinggeh yinggeh requested a review from keivenchang August 1, 2025 21:00
@yinggeh
Copy link
Contributor Author

yinggeh commented Aug 1, 2025

@keivenchang How to get approval from "ai-dynamo/dynamo-rust-codeowners"?

@yinggeh
Copy link
Contributor Author

yinggeh commented Aug 4, 2025

Hi @rmccorm4. This PR is blocked by approval from dynamo-rust-codeowners. Can you please approve?

@grahamking
Copy link
Contributor

This will mean the frontend uses env var DYN_HTTP_SVC_HEALTH_PATH and the worker uses env var DYN_SYSTEM_HEALTH_PATH (similarly for /live). Should we harmonize? @nnshah1 @keivenchang

@grahamking grahamking merged commit 7c8f8fd into main Aug 5, 2025
10 checks passed
@grahamking grahamking deleted the yinggeh-DIS-240-parameterize-server-status-endpoints branch August 5, 2025 17:14
@grahamking
Copy link
Contributor

Merged. We can rename the env var in a follow up.

@keivenchang
Copy link
Contributor

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants