feat(telemetry): emit adapter-function prepare/activate/deactivate spans from a tracing plugin - #1558
Open
planetf1 wants to merge 13 commits into
Open
feat(telemetry): emit adapter-function prepare/activate/deactivate spans from a tracing plugin#1558planetf1 wants to merge 13 commits into
planetf1 wants to merge 13 commits into
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>
This was referenced Aug 20, 2026
jakelorocco
approved these changes
Aug 20, 2026
Assisted-by: Codex Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Contributor
Author
|
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) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Issue
Progresses #1466; this PR intentionally delivers only its lifecycle-span slice.
Description
Adapter-function lifecycle metrics existed, but tracing could not produce corresponding spans because the hook family had completion events only. A tracing plugin had no point at which to open a parent or phase span.
This PR adds paired start hooks and a tracing plugin for the
prepare,activate, anddeactivatelifecycle. It produces anadapter_functionparent span with phase children, keeps tracing out of backend code, and records the existing adapter-function attributes consistently.Where this fits
This is Phase 2 work for Epic #929. It can merge independently of #1465, but
generate/parsespans and content capture remain blocked on #1465.releaseremains intentionally untraced because it occurs outside an invocation. Embedded-adapter activation in #1142 follows this hook/plugin pattern.What changed
Caveat
This does not add exemplars, content capture, or
generate/parsespans. Those need a real generation scope and belong to the remaining work in #1466 after #1465.Testing
Focused adapter and telemetry tests, the non-qualitative suite, Ruff, mypy, markdownlint, and the documentation quality gate pass. Required GitHub checks pass.
Testing
Attribution
Adding a new component, requirement, sampling strategy, or tool?
If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.
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.