Conversation
Progresses generative-computing#1466. The ADAPTER_FUNCTION_INVOCATION_COMPLETE and ADAPTER_FUNCTION_PHASE_COMPLETE hooks had no start-side sibling, so no plugin could open a span for the adapter-function lifecycle -- this is the structural root cause generative-computing#1454 worked around by opening spans inline in mellea/backends/. Adds ADAPTER_FUNCTION_INVOCATION_START and ADAPTER_FUNCTION_PHASE_START, each carrying a new invocation_id correlation field (also added to the existing COMPLETE payloads) so a tracing plugin can key spans safely under concurrent invocations. Fires the new hooks from AdapterMixin. adapter_scope() (activate/deactivate) and from LocalFileBinding. prepare(), which now opens its own single-phase invocation since it runs outside adapter_scope -- this also guarantees invocation-complete always fires (even if prepare() raises), which the phase-complete hook's success-only contract cannot, so a later span registry can drain to zero. Reconciles the phase Literal: "release" now appears in it (per generative-computing#1466's acceptance criteria) with a documented reason it has no firing site -- WeightsBinding.release() runs outside any invocation, unlike prepare/activate/deactivate. No spans yet -- that's the next commit, from a plugin in mellea/telemetry/tracing_plugins.py per generative-computing#1464/generative-computing#1466. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Progresses generative-computing#1466. Adds AdapterFunctionTracingPlugin to mellea/telemetry/tracing_plugins.py, which turns the ADAPTER_FUNCTION_* hooks added in the previous commit into an adapter_function parent span with one adapter_function.<phase> child per lifecycle phase (prepare/activate/deactivate; generate/parse are blocked on generative-computing#1465). On the mellea.backend tracer -- adapter/model lifecycle work is a backend concern, not a user-facing operation. The child spans parent explicitly via trace.set_span_in_context, looked up by invocation_id, rather than via the ambient-attach convention every other span pair in this codebase uses. ADAPTER_FUNCTION_*_START/_COMPLETE fire from sync code (adapter_scope, LocalFileBinding.prepare) via _run_async_in_thread, which runs each hook as an independent task seeded from a fresh contextvars snapshot of the calling thread -- an ambient-context attach inside one hook's task is invisible to the next hook's snapshot, so ambient nesting can't work here regardless of Python version. Explicit parenting sidesteps that entirely and needs no _CONTEXT_ATTACH_SUPPORTED gating. adapter_function_invocation_complete defensively closes any phase child span still open (a phase that raised fires phase_start but never its own success-only phase_complete), so the in-flight span registry still drains to zero on a raised phase. adapter_function.prepare records the resolved Hugging Face SHA as mellea.adapter_function.revision, not "main" (moved from generative-computing#1141). Content capture (MELLEA_TRACES_CONTENT) is not wired here: no phase in this scope carries adapter input/output content -- that applies to generate/parse, landing with generative-computing#1465. Documents the span schema, the tracer choice and its rationale, and the explicit-parenting decision in docs/docs/observability/tracing.md -- the current home for this content now that docs/dev/adapter_observability.md (the location generative-computing#1466 named) has been deleted and folded into published docs and code (see PR generative-computing#1483/generative-computing#1548). Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…r linkage Progresses generative-computing#1466. AdapterFunctionTracingPlugin and the pre-existing AdapterFunctionMetricsPlugin are two separate plugins subscribed to the same hooks, so exemplar linkage (SKILL.md §3) isn't structurally guaranteed by "one plugin owns both". Checked and found genuinely unreachable here regardless of firing order: no span in this family is ever attached as ambient OTel context (a deliberate choice, since ambient attach can't establish anything across separate _run_async_in_thread-dispatched hook calls -- see the previous commit), so there is nothing for the metrics plugin to sample as an exemplar even if it ran while the span were still open. Documents this as a known, explained gap rather than leaving it to be found later. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…arding Progresses generative-computing#1466. Fixes two real bugs found by independent review of the previous two commits, both confirmed by reproducing them against the pre-fix code and observing the failure: - LocalFileBinding.prepare() fired adapter_function_invocation_complete before adapter_function_phase_complete on the success path (the phase hook fired outside the with-block, after the invocation hook's finally). This silently made finish_adapter_function_span's defensive dangling-child-span cleanup -- documented as the failure-only path -- the only path that ever closed adapter_function.prepare's span, and inverted the order AdapterMixin.adapter_scope uses for the same pair. Fixed by firing phase-complete from an else: clause, before the finally. - adapter.py's _fire_phase_start_hook built its payload outside its own try, so a non-str .revision on a duck-typed (non-LocalFileBinding) WeightsBinding raised a pydantic ValidationError that escaped adapter_scope entirely, aborting before activate() ever ran -- exactly the failure the function's docstring says it prevents. _core.py's sibling _fire_phase_start already guarded this correctly; adapter.py's now matches it. Also fixes AdapterMixin.adapter_scope's docstring, which still claimed the ADAPTER_FUNCTION_* family "currently has no start hook" -- the exact gap the previous two commits closed -- and pointed at the deleted docs/dev/adapter_observability.md. Adds regression tests for both bugs, each verified against the pre-fix code (temporarily reverted, confirmed failing, restored) per the project's regression-guard verification standard, plus the two error-path tests review flagged as untested: a failing prepare() asserting both spans close ERROR and the registry drains, and a failing adapter_function_invocation_start hook dispatch not blocking activation (mirroring the existing invocation-complete coverage). Also, cleanup from the same review pass: - Drop the attach_context parameter from start_adapter_function_span/ start_adapter_function_phase_span -- no caller ever passed it, and passing True would misbehave (mismatched attach/detach tasks), so it was configurability that could not be used correctly. - Iterate list(_in_flight_spans) rather than the live dict in finish_adapter_function_span's dangling-child sweep, so a concurrent insert from another invocation's sync-dispatched hook can't raise "dictionary changed size during iteration". - Record error.type on a dangling phase child span too, matching the parent invocation span's existing convention. - Reword the "Nesting is unconditional"/exemplar-gap doc and docstring passages for precision (an enclosing application span can still be ambiently current; it's just not the adapter_function span the metric is about). - Fix a stale test comment, a redundant re-import in a test, and a doc cross-reference to a note that had moved sections. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Per AGENTS.md section 13's own feedback-loop rule. Encountered while fixing a bug found in review of generative-computing#1466: a code comment containing the literal text "raise " false-triggered tooling/docs-autogen/audit_coverage.py's "missing Raises section" check on a function with no actual raise statement, since that check is a substring match over the whole function source, not an AST check for real raise statements. Assisted-by: Claude Code Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Each adapter_function hook dispatch blocks the calling thread on the shared background event loop (_run_async_in_thread resolves the hook coroutine via run_coroutine_threadsafe(...).result()). Holding the non-reentrant _lifecycle_lock across all four of them let a plugin handler that re-enters this binding's lifecycle from the background loop deadlock: the handler waits on the lock while prepare() waits on the handler. Split prepare() into two short lock windows - the released/loaded check, then the registration/load work - and run the invocation/phase dispatches between them, preserving the pinned invocation_start -> phase_start -> phase_complete -> invocation_complete order. An already-loaded (or released) binding still opens no invocation and fires no hooks, and a release interleaving now surfaces as the backend's 'refused to register' error rather than racing the load. Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
The three firing sites measured phase_duration on different clocks: activate started its timer before its blocking phase-start dispatch, deactivate after it, and prepare at method entry (covering the lock wait and both start dispatches). Samples from the three phases were therefore not clock-comparable. Take started_at after the phase-start dispatch at every site, so each phase_duration sample covers the phase's own work only. Also move the invocation_id parameter first in LocalFileBinding._fire_phase_complete to match the module-level twins in adapter.py. Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…ry hygiene An idempotent prepare() (already loaded) must fire zero adapter_function hooks; nothing pinned that - a refactor moving the _loaded check below the hook firings would silently open a duplicate invocation per re-entry. Extend the hook-count test with a second prepare() under capture, and give the phase-span no-op test a registry-untouched assertion so the no-op cannot pass while corrupting _in_flight_spans. Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
The invocation hook builders in adapter.py construct payloads and dispatch unguarded by deliberate design - the call site in adapter_scope carries the try/except - but only the regression test explained why. State the obligation in both builders' docstrings so a future call site (e.g. generative-computing#1465's generate/parse wiring) inherits the contract. Reword the tracing.md nesting claim: 'unconditional' overstates it - every firing site swallows dispatch failures, so a failed invocation-start dispatch leaves that invocation's phase spans unparented (ambient fallback). Document the edge. Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
…plit The two-window check/work structure let a concurrent prepare() pass the pre-flight check while a winner held the work lock, then re-run load_peft_adapter and open a full second invocation (duplicate span and phase-duration sample) for work that was already done - the old single-window code made the loser a pure no-op. Re-check backend/_loaded as the first statement of the work window and return: the load never ran under the losing call, so no phase-complete fires (success-only contract) and the finally still closes the invocation it opened. Also correct the body comment: a release() completing between the windows clears _staged_backend, so the work window raises the bind-missing error, not the 'refused to register' one. Pinned by test_concurrent_prepare_loses_the_race_without_reloading (observed failing on the racy code: load_peft_adapter called twice). Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
_fire_phase_complete_hook built its pydantic payload outside its guard, contradicting its own docstring promise. Identity is a plain frozen dataclass with no runtime coercion, so an adapter with a non-str Identity.name raised a pydantic ValidationError from the activate phase-complete site after activate() had succeeded - the body never ran and a healthy invocation was reported as an error. Move the construction under the same guard as the dispatch. The invocation hooks stay unguarded by design (call-site guards, documented in their docstrings). Pinned by test_adapter_scope_swallows_non_str_identity_name_on_phase_ complete (observed failing on the unguarded code with the exact ValidationError). Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Rename the public hook payload field 'invocation_id' to 'adapter_function_invocation_id', consistent with the other families' 'tool_invocation_id', closing the review round-1 deferred-rename item. None of the four payloads is released, so no migration is needed. The id travels through the private tracing helpers as a short local parameter and as the in-memory _in_flight_spans registry key; no span attribute carries it, so nothing to preserve on the wire. Assisted-by: opencode Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Assisted-by: Codex Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
|
As this touches telemetry I'm going to hold on any merge until @ajbozarth has had an opportunity to review (unless this starts blocking other changes) |
Assisted-by: Codex Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
|
currently working on changes. not yet ready for review |
Assisted-by: Codex Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Assisted-by: Codex Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Assisted-by: Codex Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
|
Closing this draft for now. The reviewed implementation provides useful LocalFile activation/deactivation spans, but it is not complete adapter-function observability: Metrics already cover the current lifecycle phases. We will return to tracing when there is a concrete operational need, when another adapter reality needs comparable visibility, or when we can define one coherent lifecycle and metric contract across adapter types. |
Pull Request
Issue
Progresses #1466 with invocation-scoped adapter-function tracing.
Description
Adapter-function metrics could report that activation had finished, but tracing could not represent the activation lifecycle as spans: the hook family had completion events only, so a plugin had no event at which to open a parent span or a phase span.
This PR adds the paired start events and lets
AdapterFunctionTracingPluginbuild the span tree. Backend code still fires hooks only; it does not import tracing or create spans directly.sequenceDiagram participant Scope as AdapterMixin.adapter_scope participant Hooks as Hook dispatcher participant Plugin as AdapterFunctionTracingPlugin participant Trace as OpenTelemetry tracer Scope->>Hooks: invocation_start(id) Hooks->>Plugin: open parent Plugin->>Trace: adapter_function Scope->>Hooks: phase_start(id, activate) Hooks->>Plugin: open child Plugin->>Trace: adapter_function.activate Scope->>Hooks: phase_complete(id, activate) Hooks->>Plugin: close child Scope->>Hooks: phase_start(id, deactivate) Hooks->>Plugin: open child Scope->>Hooks: phase_complete(id, deactivate) Hooks->>Plugin: close child Scope->>Hooks: invocation_complete(id, outcome) Hooks->>Plugin: close parentThe hook calls begin in synchronous code and are dispatched to the shared async loop. Ambient OpenTelemetry context cannot cross those dispatches, so phase spans are explicitly parented from the invocation ID rather than relying on context attachment.
flowchart LR A[Adapter scope emits correlated lifecycle hooks] --> B[Tracing plugin builds parent and child spans] C[Embedded request activation emits metric-only phase completion] --> D[Metrics plugin records duration] C --> E[No correlation ID] E --> F[Tracing plugin ignores event]The second path is deliberate. An embedded request edit has no matching invocation outcome yet, so this PR preserves its existing metric while avoiding an orphan span.
Scope
Included:
activateanddeactivatespans insideAdapterMixin.adapter_scope.Deferred:
prepareremains metric-only. It is setup work, not an adapter-function invocation; tracing it needs a separate contract that does not alter invocation metrics.generateandparseremain blocked on refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465.releaseremains untraced because it has no invocation-scoped hook site.Verification
uv run pytest test/ -m "not qualitative": 4,142 passed, 47 skipped.Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?
NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.