Skip to content

fix(vllm): protect private content and preserve request lifecycles - #518

Open
nik13 wants to merge 8 commits into
devfrom
fix/opus-followup-traceAI-424
Open

nik13 wants to merge 8 commits into
devfrom
fix/opus-followup-traceAI-424

Conversation

@nik13

@nik13 nik13 commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow up the recall of #424 without changing the native/shared-first migration or creating a second tracing owner. The original recall defects predate #424 and affect the retained legacy vLLM route and/or its recommended shared OpenAI route. Follow-up review also caught a legacy stream-disposal regression introduced by this PR; it is corrected here.

  • Exclude extra_headers, extra_query, and timeout from legacy input capture. Preserve the actual request, including vLLM extra_body parameters. Never serialize raw-response client internals.
  • Apply granular text/message/image privacy flags to opaque input.value, output.value, raw payload copies, and invocation-parameter copies in the central TraceConfig policy. Opaque copies are conservatively redacted; typed message attributes retain permitted content. Legacy multimodal inputs now use typed text/image attributes instead of stringifying the whole list. Hiding input messages also hides the separate input-image copy.
  • Finalize cancelled/interrupted requests and stream iteration on both routes, re-raise the original exception, and preserve error-detail privacy.
  • Return legacy SDK stream proxies that preserve context management, forward explicit cleanup, and finalize exactly once, including before iteration. OpenAI 1.0 cleanup uses its underlying response because that SDK predates stream close() and context managers.
  • Preserve span finalization when legacy stream proxies are disposed, including partially consumed streams abandoned by a break or return. The idempotent tracking-state finalizer ends tracing only; it does not close the transport, require an event loop, or overwrite an already-ended error span.
  • Materialize legacy single-use message iterables once and use the same value for capture and SDK invocation, including sync and async calls.
  • Intercept aclose() on both routes so direct cleanup and contextlib.aclosing finalize once before iteration or after one chunk, instead of delegating the SDK alias past tracing.
  • Honor FI_HIDE_LLM_INVOCATION_PARAMETERS in every legacy raw-input copy, including provider-specific extra_body settings, while retaining allowed messages and the unchanged wire request. Explicit TraceConfig overrides are covered.
  • Require fi-instrumentation-otel>=1.2.3 in both instrumentors; prepare that core version and test built wheels rather than allowing a dependency-valid install with the old privacy policy.

Existing fix deliberately not duplicated

The recall also verified shared-stream explicit close() losing spans. Open PR #411 already fixes both sync and async close. Its _stream.py at f8b6996c5dcc9116bd12343e421eda874def64f2, isolated against current dev, passes four real-SDK close contracts (sync/async, before/after first chunk) at OpenAI 1.69.0 and 3.42.1; all four fail on unchanged dev. This PR adds shared request/iteration exception handling and independent aclose() finalization, not #411's explicit close() methods. Both PRs are needed to close the complete recall. Round-one review reproduced all four shared close() failures on #518; composing only #411's close methods with the new alias passes the complete review probes (16 at the latest SDK, 12 plus 4 native-alias skips at 1.69.0). Do not close the recall until #411 and this PR merge. Round-two verification confirms that #411 remains OPEN/CONFLICTING at the same head. Its branch needs integration through its own repair/merge gate; this PR does not duplicate that work.

Verification

All application tests ran under the existing macOS sandbox denying outbound network except localhost, with an empty synthetic HOME and synthetic credentials. Real OpenAI SDK clients use MockTransport; the shared suite also exports OTLP to loopback. No vendor inference, Cloud collector, real credentials, registry publication, or merge.

Declared floors: legacy OpenAI 1.0.0; shared OpenAI 1.69.0. Latest PyPI release checked: 3.42.1.

Suite OpenAI 1.0.0 / Python 3.10 OpenAI 1.69.0 / Python 3.13 OpenAI 3.42.1 / Python 3.13
Full legacy vLLM suite, including both-route recall regressions where supported 110 passed, 20 skipped 170 passed 170 passed
Full shared OpenAI suite, including optional Fireworks ownership contracts Not supported 519 passed 519 passed
Core config/tracer/error/structured/embedding privacy suites 281 passed — 281 passed

SDK 1.0 skips context-manager and iterable-message contracts that its native API did not declare, plus the shared-package metadata check because that instrumentor is intentionally absent. Both modern shared-suite runs include all optional Fireworks migration tests and have no skips.

New regressions fail against unchanged dev 5ae2b9b4a3678af52abc01d90e4fe2fe1d4c2099 before the fix. Coverage includes credential exclusion without disabling capture, every granular FI_HIDE flag across both routes and sync/async stream/nonstream calls, cancellation after confirmed transport entry, error privacy, early/repeated close, consumer errors/context exit, and exact on-wire iterable message preservation. Core lazy-value tests ensure hidden copies are not evaluated.

Round-one regression tests fail first (18 failed / 16 passed on the reviewed head at the latest SDK), then pass with the fixes. A clean dependency-valid install of the original instrumentors with released core 1.2.2 reproduces 36 granular privacy failures; the corrected instrumentor wheels reject core 1.2.2 during dependency resolution and pass with built core 1.2.3.

Round-two disposal regressions fail first at all three SDK versions (15 failed / 2 passed, including the two independent review probes), then all 17 pass with the disposal fix. The permanent cases cover sync/async break and return, output hiding, disposal before/after the first chunk while retaining the native SDK stream, exactly-once successful/error finalization, and containment of tracing failures during destruction. Retaining the native SDK stream proves span export does not depend on transport closure or async-generator shutdown. Both complete package suites were rerun with built wheels. Shared explicit close() still fails all four cases on this PR alone at both modern SDKs; composing only #411’s existing close methods again passes the review probes.

python3 .github/release/release_plan.py --check, targeted Ruff correctness checks, and git diff --check pass. The CI planner installation guards pass all 59 tests; the new local-core installation step remains fail-closed. The core OpenAI framework test now patches the actual httpx2 transport on SDK 3.x (7 tests pass at both 1.69.0 and 3.42.1). CI retains the legacy floor, updates the modern matrix to 1.69.0/3.42.1, and runs the new lifecycle and core privacy regressions with networking disabled.

The initial full-repository CI also exposed two stale sibling assertions: DeepSeek explicitly excluded output.value from granular privacy checks, and Discovery Engine expected the opaque input.value to stay visible under input text/message hiding. This PR updates those tests to enforce the corrected core policy. DeepSeek's complete suite passes 71 tests at all three SDK versions. Local Discovery Engine gRPC tests cannot connect even to their synthetic loopback server under the required macOS sandbox (Operation not permitted); its full CI job is the validation path. No sandbox rule was relaxed.

Release boundary

The granular-copy fix is in fi-instrumentation-otel, not just either instrumentor. Both instrumentors now require core >=1.2.3, the minimum version prepared with the corrected policy. Core package/runtime/manifest versions agree at 1.2.3; instrumentor release versions remain owned by the release stage. Do not publish the instrumentor releases before the corrected core is available. The existing release workflow already builds/publishes core, verifies its PyPI availability, and only then builds/publishes dependent Python packages. Nothing was published by this repair stage.

The provider matrix installs built core and instrumentor wheels together at OpenAI 1.0.0, 1.69.0 and 3.42.1. The generic Python, Vercel gateway and Parallel distribution jobs also use the local corrected core rather than resolve an unpublished minimum from PyPI. This prevents pre-release CI from silently using an older policy. No customer docs claim a registry release from this branch.

🤖 Generated with Claude Code

nik13 and others added 8 commits November 1, 2026 21:50
Exclude transport credentials, mask opaque copies under granular privacy flags, preserve legacy stream cleanup and iterable messages, and finish cancelled legacy/shared requests. Leave shared explicit close to existing PR #411.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Remove DeepSeek's exception for output.value from granular privacy assertions and align Discovery Engine's opaque input expectation with the central mask.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Co-Authored-By: Claude Code <noreply@anthropic.com>
Require fi-instrumentation-otel 1.2.3 for OpenAI and vLLM, test built wheels, and preserve messages while hiding every legacy invocation-parameter copy. Finalize aclose on both routes without duplicating PR #411's shared close methods.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Keep fail-closed installation guards for the local minimum-core path, build core alongside Parallel distributions, and patch the actual httpx2 transport in SDK 3.x framework tests.

Co-Authored-By: Claude Code <noreply@anthropic.com>
OpenAI 3.x no longer installs classic httpx transitively; tests still exercise both supported transports.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Restore generator-finally tracing behavior through an idempotent tracking-state finalizer. Keep transport cleanup independent and cover sync/async disposal, output privacy, retained SDK transports, and finalizer failures at floor/latest SDK versions.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Preserve stream close and cancellation handling, and align privacy tests with redacted output.

Co-Authored-By: Claude Code <noreply@anthropic.com>

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant