fix(loongsuite): isolate advice and stream failures

[sipercai]

Description

This PR adds Robin-compatible hook_advice and async_hook_advice
decorators to the existing opentelemetry-util-genai package and applies the
shared fail-open boundary to LiteLLM probe-owned lifecycle work.

The application call and stream iteration remain outside advice. LiteLLM keeps
its existing stream accumulator and returns the original response, chunks, and
application exceptions unchanged. Probe failures in preparation, response
mapping, chunk recording, final callbacks, and lifecycle cleanup are isolated
from the application path.

Streaming context is detached before the stream is returned, while a recording
span may be finalized later from another task or context. _lifecycle_finalized
is limited to ordinary synchronous LLM lifecycle idempotency and is excluded
from dataclass equality. This PR does not change or harden the existing
multimodal asynchronous dispatch lifecycle.

This follows the
OpenTelemetry error-handling requirement
that instrumentation errors must not escape into application code.

The shared API is exposed from opentelemetry.util.genai, which LiteLLM already
depends on. Robin synchronization can rewrite
from opentelemetry.util.genai import hook_advice to the existing commercial
self-monitor decorator import without adding another 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 which would cause existing functionality not to work as expected)
• This change requires a documentation update

How Has This Been Tested?

• Fresh-cache tox -e precommit
• tox -c tox-loongsuite.ini -e py311-test-util-genai
• tox -c tox-loongsuite.ini -e py311-test-loongsuite-instrumentation-litellm
• tox -c tox-loongsuite.ini -e lint-util-genai
• tox -c tox-loongsuite.ini -e lint-loongsuite-instrumentation-litellm
• Independent review and re-review
• Bounded 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

• 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 revised design was approved before implementation.
• Approved scope: shared fail-open advice in the existing GenAI utility package, plus LiteLLM stream lifecycle isolation and contract tests.
• Changed surface: opentelemetry-util-genai, LiteLLM wrappers/tests, and Google ADK Python support metadata.
• Non-goals: Robin self-monitor metrics hardening, universal stream proxies, and multimodal asynchronous lifecycle idempotency.

Local Checks

Check	Command	Result	Notes
Static readiness	check_loongsuite_pr_readiness.py --repo .	pass	No errors or warnings.
Precommit	fresh-cache tox -e precommit	pass	All repository hooks passed.
util-genai	tox -c tox-loongsuite.ini -e py311-test-util-genai	pass	288 passed, one credential-gated upload test skipped.
LiteLLM	tox -c tox-loongsuite.ini -e py311-test-loongsuite-instrumentation-litellm	pass	67 passed.
Lifecycle focus	util-genai test_handler_lifecycle.py	pass	10 passed, including detached and non-recording spans.
Lint	util-genai pylint and LiteLLM ruff environments	pass	pylint 10/10; ruff passed.
Independent review	two review rounds on the narrowed lifecycle design	pass	Sampled-out stream handling fixed; multimodal hardening explicitly deferred.
Privacy scan	branch diff and cassette patterns	pass	No credentials, bearer tokens, private paths, or usernames found.

Business Isolation

Scenario	Status	Evidence
prepare/start fault	pass	One application call; original response preserved; partial span ended.
response mapping/stop fault	pass	Original response preserved; context detached and span ended.
fail callback fault	pass	Original application exception object preserved.
stream chunk/final callback fault	pass	Original chunk identity/order preserved; callback finalized once.
early close/cancellation/GeneratorExit	pass	Close/aclose and cancellation tests preserve application control flow.
cross-task or cross-Context stream	pass	Context is detached before return and span finalizes later without token reset leakage.
clean sibling after fault	pass	Focused lifecycle and concurrency tests show no leaked parent context.

Real E2E Matrix

The live provider evidence was collected on this PR's functional LiteLLM
revision. The final scope-narrowing commit only removes multimodal dispatch
state management and adds ordinary lifecycle regression tests.

Scenario	Status	Command or Demo	Evidence
non-streaming	pass	Bounded LiteLLM DashScope smoke	Request completed and emitted an LLM span.
streaming	pass	Bounded LiteLLM DashScope stream	Stream completed and finalized an LLM span.
concurrency	pass	Bounded concurrent DashScope smoke	Calls completed with distinct trace IDs and no parent/content crossover.
error path	pass	Live invalid-model request	Provider error remained visible and the error span finalized.
agent/tool/ReAct	N/A	LiteLLM client instrumentation	The integration does not own an agent or tool-execution lifecycle.
tool-heavy	N/A	LiteLLM client instrumentation	Tool-call delta accumulation is tested, but this integration does not execute tools.

Telemetry and Weaver

Check	Status	Command or Artifact	Notes
Span tree / span kinds	pass	In-memory exporter and bounded live smoke	Root LLM spans emitted for each call.
Content capture modes	pass	Focused tests and live NO_CONTENT smoke	NO_CONTENT suppresses message attributes; SPAN_ONLY is covered by focused tests.
Concurrency isolation	pass	Concurrent live smoke	Calls use distinct trace IDs.
Weaver live-check	pass	LoongSuite GenAI advice profile	Live spans produced zero violations.

CI

• The current remote revision completed all GitHub checks successfully.
• The next push will rerun CI for the narrowed lifecycle diff.
• Shared util changes intentionally select the full LoongSuite matrix.

Google ADK Python 3.9 CI

The Python 3.9 Google ADK jobs were dependency-resolution failures rather than
advice or stream regressions. The package metadata and generated matrix now
match Google ADK's Python 3.10+ dependency floor. Python 3.10 oldest/latest
tests, lint, workflow generation, precommit, and wheel metadata checks passed.

[ralf0131]

Review: fix(loongsuite): isolate advice and stream failures

Summary

New loongsuite-instrumentation-common package (23 files, +1310/-9) providing failure-isolated advice helpers and protocol-preserving sync/async stream proxies, following the OpenTelemetry error-handling requirement that instrumentation errors must not escape into application code.

Key Changes

1. advice.py (+158 lines) — Failure-isolated callbacks

• call_advice wraps instrumentation callbacks so failures are caught and logged, never propagated to application code
• Application return values, original exceptions, and close results remain unchanged
• Clean separation: application call stays outside the advice boundary

2. stream.py (+355 lines) — Protocol-preserving stream proxies

• Synchronous and asynchronous stream proxies that preserve the original protocol
• Stream chunks, original exceptions, and context-manager suppression semantics all preserved
• Instrumentation callbacks are best-effort only

3. LiteLLM integration

• Routes stream accumulation through call_advice — consistent with the new pattern
• test_stream_isolation.py (+68) verifies the isolation behavior

4. Package registration & bootstrap

• New package registered in CI/lint/test matrix (tox-loongsuite.ini, workflow files)
• Bootstrap generator updated to exclude common from auto-enabled instrumentations — correct, since it's a shared runtime package
• generate_loongsuite_bootstrap.py test coverage updated

CI Status

✅ All CI passing — comprehensive matrix:

• All instrumentation packages × Python 3.8-3.14 + pypy-3.9
• crewai tests passing (Issue #230 environment issue resolved)
• typecheck, spellcheck, precommit, shellcheck, docs, docker-tests
• CLA signed

Test Coverage

• test_advice.py (+100) — advice isolation tests
• test_stream.py (+369) — stream proxy tests covering sync/async, chunk handling, exception preservation, context manager semantics
• test_stream_isolation.py (+68) — LiteLLM-specific isolation tests
• Bootstrap generator tests updated (+10)

Verdict

Well-designed shared runtime package that properly isolates instrumentation failures from application code. The stream proxy implementation correctly preserves protocol semantics. Comprehensive test coverage. CI fully green. Approving.

---

Automated review by github-manager-bot

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR introduces a new shared LoongSuite runtime package that provides failure-isolated advice helpers and protocol-preserving stream proxies, and updates LiteLLM streaming instrumentation and bootstrap/workflow tooling to use and recognize it.

Changes:

• Added loongsuite-instrumentation-common with call_advice / hook decorators and IsolatedStream / IsolatedAsyncStream stream proxies plus tests/docs.
• Routed LiteLLM stream chunk accumulation through call_advice and added regression tests for chunk preservation.
• Updated bootstrap generation, tox matrices, and GitHub workflow generation to include the new package and allow shared-runtime bootstrap opt-out.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tox.ini	Adjusts codespell invocation to ignore asend.
tox-loongsuite.ini	Adds test/lint envs and deps/commands for loongsuite-instrumentation-common.
scripts/loongsuite/tests/test_generate_loongsuite_bootstrap.py	Adds coverage for bootstrap default behavior and opt-out.
scripts/loongsuite/generate_loongsuite_bootstrap.py	Adds tool.loongsuite.bootstrap opt-out and skips shared-runtime packages from auto-bootstrap.
instrumentation-loongsuite/loongsuite-instrumentation-litellm/tests/test_stream_isolation.py	Adds regression tests ensuring advice failures don’t drop streamed chunks.
instrumentation-loongsuite/loongsuite-instrumentation-litellm/tests/test-requirements.txt	Adds editable dependency on the common package for tests.
instrumentation-loongsuite/loongsuite-instrumentation-litellm/src/opentelemetry/instrumentation/litellm/_stream_wrapper.py	Wraps record-chunk accumulation in call_advice to isolate instrumentation failures.
instrumentation-loongsuite/loongsuite-instrumentation-litellm/pyproject.toml	Declares runtime dependency on loongsuite-instrumentation-common.
instrumentation-loongsuite/loongsuite-instrumentation-common/tests/test_stream.py	Adds comprehensive contract tests for isolated sync/async streams.
instrumentation-loongsuite/loongsuite-instrumentation-common/tests/test_advice.py	Adds tests for advice decorators and call isolation behavior.
instrumentation-loongsuite/loongsuite-instrumentation-common/tests/test-requirements.txt	Defines test deps for the new common package.
instrumentation-loongsuite/loongsuite-instrumentation-common/tests/init.py	Introduces tests package marker.
instrumentation-loongsuite/loongsuite-instrumentation-common/src/opentelemetry/instrumentation/loongsuite/version.py	Defines package version.
instrumentation-loongsuite/loongsuite-instrumentation-common/src/opentelemetry/instrumentation/loongsuite/stream.py	Implements isolated stream/async-stream proxies.
instrumentation-loongsuite/loongsuite-instrumentation-common/src/opentelemetry/instrumentation/loongsuite/advice.py	Implements isolated advice call helpers and decorators.
instrumentation-loongsuite/loongsuite-instrumentation-common/src/opentelemetry/instrumentation/loongsuite/init.py	Exposes the new common API surface.
instrumentation-loongsuite/loongsuite-instrumentation-common/pyproject.toml	Adds build/metadata and bootstrap opt-out configuration for shared runtime.
instrumentation-loongsuite/loongsuite-instrumentation-common/README.md	Documents intended usage and boundaries of advice/stream primitives.
instrumentation-loongsuite/loongsuite-instrumentation-common/CHANGELOG.md	Adds changelog scaffold and initial entries.
instrumentation-loongsuite/README.md	Registers the common package in the instrumentation list.
.github/workflows/loongsuite_test_0.yml	Adds common package to generated LoongSuite test matrix.
.github/workflows/loongsuite_lint_0.yml	Adds common package to generated LoongSuite lint matrix.
.github/workflows/generate_workflows_lib/src/generate_workflows_lib/init.py	Adds py38 alias mapping for workflow generation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ralf0131]

[Automated cleanup: this review was created during testing of inline comment API. The actual line-level inline comments have been posted directly on the relevant code lines.]

[ralf0131]

[Automated cleanup: this review was created during testing of inline comment API. The actual line-level inline comments have been posted directly on the relevant code lines.]

[ralf0131]

[Automated cleanup: this review was created during testing of inline comment API. The actual line-level inline comments have been posted directly on the relevant code lines.]

[ralf0131]

Thanks for the contribution! The code looks good from a review perspective, but CI is currently failing:

Check	Status
LoongSuite Test 0 result	❌ failure

Could you please fix these CI failures so the PR can be merged? You can find the failure details in the Actions tab.

Once CI is green, the PR will be ready for merge.

---

Automated notification by github-manager-bot

[ralf0131]

Summary

Refactors LiteLLM stream wrapper to route chunk accumulation and finalization callbacks through shared fail-open @hook_advice hooks. Solid refactoring with proper error isolation and idempotent stream lifecycle management.

Overall Assessment

Good changes that improve robustness of the streaming instrumentation. Code review complete — see findings below. However, CI is currently failing (LoongSuite Test 0 result). This PR cannot be approved until CI passes. Please fix the CI failure (see the CI reminder comment above).

Findings

• [Praise] util/opentelemetry-util-genai/src/opentelemetry/util/genai/extended_advice.py — The @hook_advice decorator cleanly wraps callbacks in fail-open logic, isolating instrumentation failures from the application path
• [Praise] _stream_wrapper.py — The _debug_safely() wrapper is a good defensive measure against custom logging handlers that may raise during serialization
• [Praise] _stream_wrapper.py — The _stream_closed idempotent guard correctly prevents double-close issues
• [Praise] _stream_wrapper.py — The try/finally pattern in _finalize() ensures the external callback always runs even when _close_stream() throws
• [Info] _stream_wrapper.py:228 — The type change from Iterator to Iterator[Any] is a minor improvement for type safety
• [Info] _stream_wrapper.py:319 — Changing Exception to BaseException in _finalize() signature allows catching KeyboardInterrupt/SystemExit — ensure this is intentional and the fail-open advice doesn't swallow them silently

Improvement Suggestions

• Consider adding a brief docstring to _debug_safely() explaining why the try/except is needed (custom logging handlers may raise during % formatting)

CI Status

⚠️ CI is failing: LoongSuite Test 0 result. This PR will be approved once CI passes.

---

Automated review by github-manager-bot

[ralf0131]

Thanks for the follow-up commit. CI is now fully green.

Summary of new changes (commit dcf2284)

CI fix: drop Google ADK Python 3.9 jobs — aligns the loongsuite-instrumentation-google-adk package with Google ADK's own Python 3.10+ runtime floor.

Changes reviewed:

• pyproject.toml — requires-python bumped from >=3.9 to >=3.10; removed Python :: 3.9 classifier ✅
• tox-loongsuite.ini — removed py39 from the google-adk test env matrix (py3{9,10,11,12,13} → py3{10,11,12,13}) ✅
• .github/workflows/loongsuite_test_0.yml — CI matrix updated to drop Python 3.9 for the google-adk job ✅
• CHANGELOG.md — added a Changed entry documenting the Python version floor alignment ✅

Verdict

Clean, consistent, and correctly scoped — the Python version floor change is applied uniformly across package metadata, tox config, CI workflow, and changelog. All 30 CI checks passing. CLA signed. No merge conflicts.

The core code changes (advice isolation, stream proxies, LiteLLM integration) were already approved in the previous review. This follow-up resolves the CI failures that were blocking merge. LGTM — approving.

---

Automated review by github-manager-bot

[ralf0131]

Summary

Well-architected PR that introduces hook_advice and async_hook_advice fail-open decorators in opentelemetry-util-genai and refactors LiteLLM stream wrappers to use them. The decorators correctly wrap only probe-owned work (chunk recording, finalization callbacks), keeping application calls and stream iteration outside the boundary per OpenTelemetry error-handling spec.

Key design decisions validated:

• Generators/async-generators are rejected (bodies execute after decorator returns)
• hook_advice rejects coroutines, async_hook_advice rejects sync functions
• BaseException is NOT caught — KeyboardInterrupt/CancelledError propagate correctly
• Logging failures are also isolated (custom handlers may fail)
• Stream lifecycle: try/finally ensures _finalize always runs even if _close_stream throws; _stream_closed flag prevents double-close
• f-string logging replaced with %s style (logging best practice)

Test coverage is comprehensive (439 lines across test_extended_advice.py and test_stream_isolation.py), covering success identity, exception swallowing, strict mode, logging isolation, generator rejection, BaseException propagation, and async cancellation. No issues found.

---

Automated review by github-manager-bot

[ralf0131]

Re-review: Changes since last approval (9d8e0f3 → 14c8bc6)

4 new commits since the previous approval. All changes are hardening and simplification of the advice/stream lifecycle:

Stream wrapper (_stream_wrapper.py):

• _stream_closed boolean → _finalize_lock = Lock() for thread-safe finalization idempotency
• Added explicit GeneratorExit handling — treated as early successful termination, not an error
• except Exception → except BaseException — now finalizes the span on any termination, then re-raises
• _finalize() simplified: lock-guarded idempotency check, callback invoked in try/finally so last_chunk is always cleared

Handler lifecycle (handler.py, extended_handler.py, types.py):

• New _lifecycle_finalized flag on GenAIInvocation prevents double finalization
• New detach_llm_context() separates context detachment from span ending
• New abandon_llm() for best-effort cleanup when start_llm fails
• stop_llm/fail_llm use try/finally to guarantee context detach + span end
• Checks span.is_recording() before span.end() — avoids double-end

Advice (extended_advice.py):

• Removed _log_advice_failure helper — logging inlined for simplicity
• Removed inspect.isgeneratorfunction/iscoroutinefunction checks — now docstring-only guidance

Tests: Added test_completion_isolation.py (314 lines) and test_handler_lifecycle.py (160 lines) covering lifecycle edge cases.

All changes maintain the fail-open design principles validated in the previous review. CLA signed, CI passing. LGTM.

---

Automated review by github-manager-bot

[ralf0131]

Re-review: Changes since last approval (14c8bc6 → fe36286)

One new commit since the previous approval: refactor(genai): defer multimodal lifecycle hardening. The change is a clean simplification of the multimodal lifecycle state management.

Severity Breakdown

🔴 Critical: 0 | 🟠 High: 0 | 🟡 Medium: 0 | 🔵 Low: 0 | ℹ️ Info: 1 | ✅ Praise: 2

Key Observations

Area	Observation
🔄 extended_handler.py — lifecycle simplification	Removed the fragile _lifecycle_finalized toggle (set True before multimodal call, reset on exception). Now process_multimodal_stop/fail is a simple boolean check — if handled, return; otherwise fall through to parent's sync path. This is safer: if multimodal processing raises, the exception propagates naturally instead of leaving partial state.
📝 types.py — compare=False	Correctly excludes _lifecycle_finalized from dataclass equality comparisons — it's internal runtime state, not a semantic property.
✅ Test coverage	Replaced test_multimodal_dispatch_failure_allows_finalize_retry with two focused tests: test_detached_non_recording_span_is_finalized_once (verifies idempotent finalization via metric call count) and test_detached_stream_span_can_be_finalized_by_extended_handler (verifies detached span context cleanup). Good edge-case coverage.

Positive

• Removing the BaseException catch is the right call — swallowing all exceptions to manage internal state was an anti-pattern
• The pylint: disable=unexpected-keyword-arg comments are appropriate since the method param is a runtime convention

ℹ️ Info: The CHANGELOG wording change ("synchronous probe finalization") clarifies scope — the hardening now applies to the sync path, with async/streaming handled separately.

---

Automated review by github-manager-bot