Skip to content

fix(loongsuite): isolate advice and stream failures#245

Open
sipercai wants to merge 4 commits into
mainfrom
fix/advice-stream-isolation
Open

fix(loongsuite): isolate advice and stream failures#245
sipercai wants to merge 4 commits into
mainfrom
fix/advice-stream-isolation

Conversation

@sipercai

Copy link
Copy Markdown
Collaborator

Description

This PR adds a small loongsuite-instrumentation-common package that provides
failure-isolated advice helpers and protocol-preserving synchronous/asynchronous
stream proxies.

The application call remains outside the advice boundary. Instrumentation
callbacks are best effort, while application return values, stream chunks,
original exceptions, close results, and context-manager suppression semantics
remain unchanged. This follows the
OpenTelemetry error-handling requirement
that instrumentation errors must not escape into application code.

The PR also routes LiteLLM stream accumulation through call_advice, registers
the common package in the LoongSuite test/lint matrix, and marks it as a shared
runtime package that must not be treated as an auto-enabled instrumentation by
the bootstrap generator.

No framework-owned or Python-ABI-specific dependency is added to the shared
runtime package.

Fixes # (N/A)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • tox -e precommit
  • tox -e generate-workflows
  • tox -c tox-loongsuite.ini -e py38-test-loongsuite-instrumentation-common,py39-test-loongsuite-instrumentation-common,py310-test-loongsuite-instrumentation-common,py311-test-loongsuite-instrumentation-common,py312-test-loongsuite-instrumentation-common,py313-test-loongsuite-instrumentation-common,lint-loongsuite-instrumentation-common
  • tox -c tox-loongsuite.ini -e py312-test-loongsuite-instrumentation-litellm,lint-loongsuite-instrumentation-litellm
  • python -m pytest -q scripts/loongsuite/tests/test_generate_loongsuite_bootstrap.py
  • Live DashScope non-streaming, streaming, concurrent, and error-path smoke
  • Weaver live-check against the LoongSuite GenAI profile

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Validation Evidence

Spec and Scope

  • Linked issue/spec: no public issue; the scoped design was approved before implementation.
  • Approved scope: common fail-open advice and stream lifecycle primitives, one LiteLLM integration, CI registration, and bootstrap opt-out.
  • Changed surface: loongsuite-instrumentation-common, LiteLLM stream accumulation, LoongSuite tox/workflow generation, and bootstrap package discovery.

Local Checks

Check Command Result Notes
Static readiness LoongSuite pipeline readiness checker pass No errors or warnings.
Precommit tox -e precommit with a fresh hook cache pass Working tree remained clean.
Common tests tox -c tox-loongsuite.ini -e py38-test-loongsuite-instrumentation-common,py39-test-loongsuite-instrumentation-common,py310-test-loongsuite-instrumentation-common,py311-test-loongsuite-instrumentation-common,py312-test-loongsuite-instrumentation-common,py313-test-loongsuite-instrumentation-common,lint-loongsuite-instrumentation-common pass 26 tests on every supported Python version; lint passed.
LiteLLM tests tox -c tox-loongsuite.ini -e py312-test-loongsuite-instrumentation-litellm,lint-loongsuite-instrumentation-litellm pass 48 tests; lint passed.
Workflow generation tox -e generate-workflows pass Generated workflows remained current.
Bootstrap tests python -m pytest -q scripts/loongsuite/tests/test_generate_loongsuite_bootstrap.py pass 5 tests.
Package build uv build --wheel instrumentation-loongsuite/loongsuite-instrumentation-common and LiteLLM equivalent pass Both wheels are py3-none-any; common has no runtime dependencies.
External review Three-round structured external review pass Two lifecycle findings fixed and re-reviewed; remaining P3 suggestions are non-blocking.
Privacy scan Branch diff and cassette privacy scans pass No credentials, private paths, tokens, or private infrastructure references found.

Fix Verification

The same LiteLLM stream reproduction was executed against the merge base,
current main, and this branch. An accumulator callback failure escaped and
prevented delivery of the application chunk on both the merge base and current
main; this branch returned the original chunk and completed normally.

Real E2E Matrix

Scenario Status Command or Demo Evidence
non-streaming pass litellm_genai_smoke.py with DashScope One bounded live request completed and emitted an LLM span.
streaming pass litellm_genai_smoke.py with DashScope Live stream completed with multiple chunks and a finalized LLM span.
concurrency pass Bounded two-call async DashScope smoke Both calls completed with distinct trace IDs and no content/parent crossover.
agent/tool/ReAct N/A LiteLLM client instrumentation This integration emits LLM spans and does not own an agent or tool execution lifecycle.
tool-heavy N/A LiteLLM client instrumentation The changed boundary does not execute tools; streamed tool-call delta accumulation remains covered by focused tests.
error path pass Live invalid-model request through the instrumented LiteLLM path The provider error remained visible and an error-status LLM span was finalized.

Telemetry and Weaver

Check Status Command or Artifact Notes
Span tree / span kinds pass In-memory exporter from the bounded live smoke Five root LLM spans were emitted for five calls.
Content capture modes pass Focused LiteLLM tests plus live NO_CONTENT smoke NO_CONTENT emitted no message attributes; SPAN_ONLY behavior is covered by focused telemetry tests. SPAN_AND_EVENT is N/A because this integration does not emit message events.
Concurrency isolation pass Two-call async live smoke Every call used a distinct trace ID.
Weaver live-check pass weaver registry live-check -r <registry>/model --advice-profile loongsuite-genai --input-format json ... Five live spans, zero violations; only registry stability/informational advice remained.

CI

  • GitHub checks: pending PR publication.
  • Known unrelated failures: none observed locally.
  • Follow-up needed: inspect the full generated LoongSuite matrix after publication; the shared workflow-generator change intentionally selects full LoongSuite CI.

@ralf0131 ralf0131 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@/tmp/pr245_review_body.md

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.

4 participants