fix(vllm): resolve ModuleNotFoundError on vLLM >= 0.14.0 [MLOS-854] - #19789
fix(vllm): resolve ModuleNotFoundError on vLLM >= 0.14.0 [MLOS-854]#19789heyitsgrace996 wants to merge 4 commits into
Conversation
vLLM 0.14.0 removed the deprecated `vllm.v1.engine.processor` shim and moved `Processor` to `vllm.v1.engine.input_processor.InputProcessor`. patch()/unpatch() hard-coded the old module path, so the whole vLLM integration silently failed to instrument on vLLM >= 0.14.0, dropping all engine-level spans and metrics (TTFT, queue, prefill, decode, token counts). Add a _resolve_processor_target() helper that probes for the new module first and falls back to the legacy one, used symmetrically in both patch() and unpatch(). Also add a `no_gpu` pytest marker so the new resolver regression test can run without a GPU, without changing skip behavior for any existing model-loading test.
Codeowners resolved asResolved from the full PR diff against |
Circular import analysis
|
Dependency direction analysis
|
heyitsgrace996
changed the title
| vllm: Fixes an issue where the integration failed to instrument vLLM with | ||
| ``ModuleNotFoundError: No module named 'vllm.v1.engine.processor'`` when | ||
| using vLLM versions that renamed this module, silently disabling all | ||
| vLLM tracing and metrics. |
There was a problem hiding this comment.
Should we maybe include the version of vLLM where the module was renamed so that users can more easily identify whether this fix is relevant to them?
There was a problem hiding this comment.
Agreed - updated to include version in dbaa6bf
| def test_resolve_processor_target_module_importable(): | ||
| """The resolved processor module must actually import on the installed vLLM version. | ||
|
|
||
| vLLM >= 0.14.0 removed vllm.v1.engine.processor and moved Processor to |
There was a problem hiding this comment.
Do we test vLLM >= 0.14.0? I wonder if we should update the riotfile with an explicit version >= 0.14.0 here: https://github.com/DataDog/dd-trace-py/blob/main/riotfile.py#L3662-L3670
There was a problem hiding this comment.
Oh really good point, we do not! The lockfiles were actually stuck on 0.13.0, so we were never testing any version with the error.
I updated the riotfile to run two separate venvs, one for the min supported version (0.10.2), and one for latest as we do for the anthropic integration, so it should now auto-update the version in the lockfiles and we won't need to manually pin anything - see commit c53808f.
There was a problem hiding this comment.
Hello ! About that, I'm working on a refactor of supported_versions.json. It will now use a function that every integration was implementing except vllm.
I added a change in #19810 in VLLM that you can add directly to that PR if you prefer. Similar to what I did in my PR, you can merge the two riotfile venvs that you have in your riotfile.py
|
|
||
| if found: | ||
| return PROCESSOR_MODULE_NEW, f"{PROCESSOR_CLASS_NEW}.{PROCESSOR_METHOD}" | ||
| return PROCESSOR_MODULE_OLD, f"{PROCESSOR_CLASS_OLD}.{PROCESSOR_METHOD}" |
There was a problem hiding this comment.
Instead of defaulting to old, I wonder if we should check if this path is indeed importable before patching to avoid the same silent bug as before. We could also include a debug log here, so if the module moves again then we have a signal.
There was a problem hiding this comment.
Makes sense - updated so now we try to import the module and check the class exists (since vLLM renamed both), and log a debug with a missing module/class and vLLM version in use so it should be easy to figure out if/when the module has been moved again. dbaa6bf
The vllm riot venv pinned a literal ">=0.10.2" range instead of using
riot's `latest` sentinel, which excludes it from the nightly automated
lockfile-bump workflow (generate-package-versions.yml only updates
packages tagged `latest`). That's how vLLM 0.14.0's breaking rename
went unnoticed for months until a customer hit it (MLOS-854) -- CI was
only ever validating against a frozen 0.13.0.
Split the vllm venv into two sub-venvs per the documented pattern in
docs/contributing-testing.rst ("How do I add a new test suite?"),
matching how anthropic/openai already do this: one pinned to the
declared floor (~=0.10.2), one tracking `latest` so the nightly bot
keeps it current going forward.
Regenerated the 8 affected lockfiles and ran the documented
supported-versions/registry update scripts, which now correctly show
vllm tested from 0.10.2 through 0.27.1 (today's latest) instead of a
single frozen 0.13.0.
- Release note now names vLLM >= 0.14.0 explicitly so users can tell whether the fix applies to them. - _resolve_processor_target() now validates that the expected class actually exists in each candidate module (not just that the module imports), and logs a debug message with the installed vLLM version if neither location resolves, instead of silently defaulting to the legacy target with no signal. - Narrowed the fallback's exception handling to ModuleNotFoundError keyed on the missing module's own name, so a real import failure inside an existing candidate module propagates instead of being misdiagnosed as "try the other vLLM version".
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 633e60a | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-21 15:36:46 Comparing candidate commit 633e60a in PR branch Found 0 performance improvements and 7 performance regressions! Performance is the same for 615 metrics, 10 unstable metrics.
|
Per review feedback: the floor/latest split doesn't need a second package to correlate against (unlike anthropic's httpx pairing), so the nested venvs=[...] from the prior commit was more verbose than necessary. Collapsed to "vllm": ["~=0.10.2", latest] in the flat pkgs dict, matching the simpler list-value pattern openai already uses. Same 8 riot hashes as before, so no lockfile/registry changes needed. Re-verified all 6 buildable venvs (py3.10-3.12 x floor/latest) still pass cleanly.
3 participants
Description
vLLM 0.14.0 removed
vllm.v1.engine.processor(a deprecated compatibility shim in 0.13.0) and movedProcessortovllm.v1.engine.input_processor.InputProcessor.patch()/unpatch()hard-coded the old module path, so the vLLM integration raisedModuleNotFoundErroron vLLM >= 0.14.0, silently disabling all vLLM tracing and metrics.vLLM Change Ref: https://github.com/vllm-project/vllm/blob/v0.13.0/vllm/v1/engine/processor.py
Changes
Fix:
patch.py: added_resolve_processor_target()to resolve the moved module/class dynamically instead of hard-coding the old pathconftest.py: added ano_gpumarker so GPU-free tests can opt out ofrequire_gpu, plus a fixture-ordering fix so GPU-dependent tests still skip cleanlyTest infra:
riotfile.py: split thevllmvenv into a floor-pinned +latest-tracked sub-venv, so CI stops being frozen onvllm==0.13.0and stays current going forwardlockfiles: regenerated the 8 affected vLLM lockfiles (4 old ones removed, 8 new ones added for floor + latest across Python 3.10–3.13)registry.yaml/supported_versions.json: regenerated via script to reflect the real tested range (0.10.2–0.27.1)Testing
test_vllm_patch.py: resolver picks an importable module, andpatch()/unpatch()correctly wrap/unwrapprocess_inputson whichever processor class the installed vLLM exposes.tests/contrib/vllmrun completes successfully, validated locally in Docker against real vLLM 0.10.2 and 0.27.1 across Python 3.10-3.12Risks
Low. Only changes module resolution at patch time; no change to what gets traced.
Additional Notes
Supersedes community PRs #18397, #18512, #18521 (stale + closed, never merged) — fix independently reimplemented and verified against real vLLM source.
Fixes #18393. Fixes MLOS-854.