feat: add Rust-to-Python const code generator for prometheus_names.py #3425
Conversation
WalkthroughAdds a Rust-based codegen tool to generate a pure-Python prometheus_names module from Rust sources, replaces prior Rust PyO3 Python bindings with the generated Python module, updates imports and constant names in Python code and tests, adjusts runtime docs, and registers the new codegen crate in the workspace. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CLI as gen-python-prometheus-names (CLI)
participant Parser as PrometheusParser
participant Gen as PythonGenerator
participant FS as Filesystem
Dev->>CLI: Run (optional --source, --output)
CLI->>FS: Resolve default paths
CLI->>FS: Read Rust source
CLI->>Parser: parse_file(content)
Parser-->>CLI: modules map
CLI->>Gen: new(parser)
Gen->>FS: Load template
Gen-->>CLI: Generated Python content
CLI->>FS: Write prometheus_names.py
CLI-->>Dev: Done (paths/log)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
Cargo.toml(1 hunks)components/src/dynamo/planner/utils/prometheus.py(4 hunks)lib/bindings/python/codegen/Cargo.toml(1 hunks)lib/bindings/python/codegen/README.md(1 hunks)lib/bindings/python/codegen/src/gen_python_prometheus_names.rs(1 hunks)lib/bindings/python/codegen/src/lib.rs(1 hunks)lib/bindings/python/codegen/src/prometheus_parser.rs(1 hunks)lib/bindings/python/codegen/src/python_generator.rs(1 hunks)lib/bindings/python/codegen/templates/prometheus_names.py.template(1 hunks)lib/bindings/python/rust/lib.rs(0 hunks)lib/bindings/python/rust/prometheus_names.rs(0 hunks)lib/bindings/python/src/dynamo/_prometheus_names.pyi(0 hunks)lib/bindings/python/src/dynamo/prometheus_names.py(1 hunks)lib/runtime/src/metrics/prometheus_names.rs(2 hunks)tests/utils/payloads.py(2 hunks)
💤 Files with no reviewable changes (3)
- lib/bindings/python/src/dynamo/_prometheus_names.pyi
- lib/bindings/python/rust/lib.rs
- lib/bindings/python/rust/prometheus_names.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-16T00:26:43.641Z
Learnt from: keivenchang
PR: ai-dynamo/dynamo#3035
File: lib/runtime/examples/system_metrics/README.md:65-65
Timestamp: 2025-09-16T00:26:43.641Z
Learning: The team at ai-dynamo/dynamo prefers to use consistent metric naming patterns with _total suffixes across all metric types (including gauges) for internal consistency, even when this differs from strict Prometheus conventions that reserve _total for counters only. This design decision was confirmed by keivenchang in PR 3035, referencing examples in prometheus_names.rs and input from team members.
Applied to files:
lib/runtime/src/metrics/prometheus_names.rs
🧬 Code graph analysis (4)
tests/utils/payloads.py (1)
lib/bindings/python/src/dynamo/prometheus_names.py (1)
work_handler(175-192)
components/src/dynamo/planner/utils/prometheus.py (1)
lib/bindings/python/src/dynamo/prometheus_names.py (1)
frontend_service(31-69)
lib/bindings/python/codegen/src/prometheus_parser.rs (1)
lib/bindings/python/codegen/src/python_generator.rs (1)
new(15-19)
lib/bindings/python/codegen/src/gen_python_prometheus_names.rs (2)
lib/bindings/python/codegen/src/prometheus_parser.rs (1)
parse_file(31-45)lib/bindings/python/codegen/src/python_generator.rs (1)
new(15-19)
🪛 GitHub Actions: Copyright Checks
Cargo.toml
[error] 1-1: Copyright header missing or invalid for lib/bindings/python/codegen/Cargo.toml. CI copyright check failed during execution of the copyright-check.ps1 step (exit code 255).
lib/bindings/python/codegen/Cargo.toml
[error] 1-1: Copyright header missing or invalid for lib/bindings/python/codegen/Cargo.toml. CI copyright check failed during execution of the copyright-check.ps1 step (exit code 255).
🪛 Ruff (0.13.3)
lib/bindings/python/src/dynamo/prometheus_names.py
52-52: Possible hardcoded password assigned to: "TIME_TO_FIRST_TOKEN_SECONDS"
(S105)
54-54: Possible hardcoded password assigned to: "INTER_TOKEN_LATENCY_SECONDS"
(S105)
⏰ 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). (10)
- GitHub Check: vllm (amd64)
- GitHub Check: sglang
- GitHub Check: tests (launch/dynamo-run)
- GitHub Check: clippy (launch/dynamo-run)
- GitHub Check: clippy (.)
- GitHub Check: tests (lib/runtime/examples)
- GitHub Check: tests (lib/bindings/python)
- GitHub Check: tests (.)
- GitHub Check: clippy (lib/bindings/python)
- GitHub Check: Build and Test - dynamo
🔇 Additional comments (11)
tests/utils/payloads.py (2)
23-23: LGTM! Import path correctly updated to use public API.The import path change from
dynamo._coretodynamoaligns with the new code generation workflow whereprometheus_namesis now a public Python module generated from Rust sources rather than an internal PyO3 binding.
168-168: LGTM! Constant naming aligns with Python conventions.The change from
prometheus_names.work_handler.requests_totaltoprometheus_names.work_handler.REQUESTS_TOTALcorrectly adoptsUPPER_CASEnaming for Python constants, matching the generated code structure whereREQUESTS_TOTAL = "requests_total".Cargo.toml (1)
14-14: LGTM! Workspace member correctly added.The new
lib/bindings/python/codegenmember integrates the code generation crate into the workspace, enabling thegen-python-prometheus-namesbinary to be built and run as part of the project.components/src/dynamo/planner/utils/prometheus.py (2)
22-22: LGTM! Import path correctly updated to public API.Consistent with other Python modules in this PR, the import now uses the public
dynamo.prometheus_namespath instead of the internaldynamo._core.prometheus_names.
97-149: LGTM! Module references consistently updated fromfrontendtofrontend_service.All metric name references correctly updated to use
prometheus_names.frontend_service.*instead ofprometheus_names.frontend.*, aligning with the Rust source module structure inlib/runtime/src/metrics/prometheus_names.rswhere the module is namedfrontend_service.The changes cover:
INTER_TOKEN_LATENCY_SECONDS(line 97)TIME_TO_FIRST_TOKEN_SECONDS(line 105)REQUEST_DURATION_SECONDS(line 113)REQUESTS_TOTAL(line 122)INPUT_SEQUENCE_TOKENS(line 141)OUTPUT_SEQUENCE_TOKENS(line 149)lib/runtime/src/metrics/prometheus_names.rs (2)
9-14: LGTM! Documentation accurately reflects the new codegen workflow.The updated documentation clearly describes:
- The one-way generation flow (Rust → Python via codegen)
- The specific command to regenerate:
cargo run -p dynamo-codegen --bin gen-python-prometheus-names- The output location:
lib/bindings/python/src/dynamo/prometheus_names.py- That generated code is pure Python with no Rust bindings required
This is a significant improvement over the previous "sync with Python bindings" instruction, as it accurately reflects the automated code generation approach.
87-87: LGTM! Inline comment updated for consistency.The comment correctly updated from "Python bindings" to "Python codegen" to align with the module-level documentation and the new generation workflow.
lib/bindings/python/codegen/src/lib.rs (1)
1-9: LGTM! Clean crate structure for code generation utilities.The library correctly exposes two public modules (
prometheus_parserandpython_generator) that will be used by the code generation binary. The documentation is concise and appropriate for a specialized utility crate.lib/bindings/python/codegen/Cargo.toml (1)
7-15: LGTM! Dependencies and binary configuration are appropriate.The dependency choices are well-suited for the code generation task:
synwithfullandextra-traitsfeatures for comprehensive Rust AST parsingquoteandproc-macro2for code generationanyhowfor error handlingThe binary target
gen-python-prometheus-namesis correctly configured to point to the generator source.lib/bindings/python/codegen/README.md (1)
1-38: LGTM! Comprehensive and well-structured documentation.The README effectively documents:
- The purpose: generating
prometheus_names.pyfrom Rust sources- Clear usage instructions with the exact command
- Key features including macro expansion handling (e.g.,
kvstats_name!("active_blocks")→"kvstats_active_blocks")- Concrete before/after examples showing Rust input and Python output
- Clear guidance on when to run the generator
This documentation will help maintainers understand and use the code generation workflow.
lib/bindings/python/codegen/templates/prometheus_names.py.template (1)
1-30: LGTM! Well-structured template with clear usage guidance.The template provides:
- Proper SPDX copyright header
- Clear auto-generation warning to prevent manual edits
- Regeneration instructions matching the documented workflow
- Comprehensive usage examples showing both import patterns:
- Module-level access:
prometheus_names.frontend_service.REQUESTS_TOTAL- Direct class import:
from dynamo.prometheus_names import frontend_service- Modern Python best practices (
from __future__ import annotations)This template will serve as a solid foundation for the generated
prometheus_names.pymodule.
- Created dynamo-codegen crate with AST-based parser using syn - Generates pure Python constants from Rust prometheus_names.rs - Removed old PyO3 bindings for prometheus_names - Migrated Python code to use generated constants Changes: +755 lines: New codegen crate, generated Python file -616 lines: Removed Rust bindings and .pyi stub Benefits: - No Rust compilation needed for simple string constants - Easier to maintain and debug - Better IDE support with pure Python classes Signed-off-by: Keiven Chang <[email protected]> refactor: merge python_generator into gen_python_prometheus_names and ensure deterministic output Signed-off-by: Keiven Chang <[email protected]>
keivenchang
force-pushed
the
keivenchang/simplify-Prometheus-Python-Rust-constants
branch
from
d5d9eb8 to
c245b49
Compare
|
I just did a plain rebase, force push, and conflict resolution; cleaner than merge main. No other changes were made. |
keivenchang
deleted the
keivenchang/simplify-Prometheus-Python-Rust-constants
branch
3 participants
Overview:
Adds a Rust-based code generator that produces pure Python constants from
prometheus_names.rs, eliminating the need for PyO3 bindings for simple string constants.Details:
dynamo-codegencrate with AST-based parser usingsynto extract Rust constantslib/bindings/python/src/dynamo/prometheus_names.pywith pure Python classes containing metric name constantslib/bindings/python/rust/prometheus_names.rs) and type stub filetests/utils/payloads.py,components/src/dynamo/planner/utils/prometheus.py) to use generated constantslib/runtime/src/metrics/prometheus_names.rswith regeneration instructionsWhere should the reviewer start?
lib/bindings/python/codegen/src/prometheus_parser.rs- AST parsing logiclib/bindings/python/src/dynamo/prometheus_names.py- generated outputlib/bindings/python/rust/prometheus_names.rs/coderabbit profile chill
Summary by CodeRabbit