Skip to content

feat: Add PoC of sglang metrics server (default port 30000) #3407

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: Add PoC of sglang metrics server (default port 30000) #3407

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e2beb7
feat: Add PoC of sglang metrics server (default port 30000)
rmccorm4 Oct 3, 2025
e81d62f
pre-commit fix
keivenchang Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion components/src/dynamo/sglang/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
import signal
import sys

import sglang as sgl
import uvloop
from sglang.srt.utils import launch_dummy_health_check_server

import sglang as sgl
from dynamo.llm import ModelInput, ModelType
from dynamo.runtime import DistributedRuntime, dynamo_worker
from dynamo.runtime.logging import configure_dynamo_logging
Expand Down Expand Up @@ -66,6 +67,10 @@ async def init(runtime: DistributedRuntime, config: Config):
server_args, dynamo_args = config.server_args, config.dynamo_args

engine = sgl.Engine(server_args=server_args)
if server_args.enable_metrics:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some things to consider either now or later:

  1. Do we want this health check server up always even for non-metrics purposes? Then we can remove the if server_args.enable_metrics:
  2. Do we want metrics server up always / by default?
    • If so, we can default server_args.enable_metrics = True in our worker code
    • If not, we can also consider the worker-specific env vars that toggle metrics today like DYN_SYSTEM_ENABLED=true
    • However, our current UX proposition is to match sglang as closely as possible on CLI commands for seamless transition - so toggling sglang engine metrics with a unique dynamo env var here seems like an anti-pattern to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

in the future when we expose metrics via rust endpoint - do we need the dummy health check any more?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would use the same flag as sglang - but ideally it would be surfaced via our endpoint and not need a seperate server - but that is dependent on wait we find from @keivenchang 's work

launch_dummy_health_check_server(
server_args.host, server_args.port, server_args.enable_metrics
)
Comment on lines +70 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify the port parameter matches the intended metrics port.

According to the PR description, the metrics server should run on port 30000 by default. However, this code passes server_args.port, which is likely the main SGLang server port (typically 8000 or similar).

Please confirm whether:

  1. server_args.port contains the correct metrics port (30000), or
  2. A separate metrics port configuration should be used (e.g., server_args.metrics_port)

Run the following script to check how server_args.port is configured and if there's a separate metrics port:

🏁 Script executed:

#!/bin/bash
# Description: Check server_args configuration and port usage

# Find where server_args is defined and what ports are available
ast-grep --pattern 'class $_ {
  $$$
  port = $$$
  $$$
}'

# Search for metrics_port or similar configuration
rg -nP '(metrics_port|port.*30000|30000)' --type=py -C3

# Check the args parsing to see available port configurations
rg -nP 'def\s+parse_args' --type=py -A 50

Length of output: 47344

Introduce a dedicated --metrics-port CLI option

The metrics server currently binds to server_args.port (the main API port), causing conflicts. Add a --metrics-port flag (default 30000) in ServerArgs.add_cli_args (components/src/dynamo/sglang/args.py) and update the launch_dummy_health_check_server call in components/src/dynamo/sglang/main.py (around lines 68–71) to use server_args.metrics_port.

🤖 Prompt for AI Agents 
In components/src/dynamo/sglang/main.py around lines 68 to 71, the metrics
server is being started using server_args.port which conflicts with the main API
port; update the code to call launch_dummy_health_check_server(server_args.host,
server_args.metrics_port, server_args.enable_metrics) and add a new CLI flag in
components/src/dynamo/sglang/args.py by introducing a --metrics-port argument on
ServerArgs.add_cli_args with a default of 30000 (and parse/store it as
metrics_port on ServerArgs); ensure any type parsing matches other port args
(int) and update any help text accordingly.


component = runtime.namespace(dynamo_args.namespace).component(
dynamo_args.component
Expand Down
Loading