Skip to content

feat(telemetry): emit adapter-function activate/deactivate spans from a tracing plugin - #1558

Closed
planetf1 wants to merge 17 commits into
generative-computing:mainfrom
planetf1:issue-1466
Closed

planetf1 wants to merge 17 commits into
generative-computing:mainfrom
planetf1:issue-1466

Conversation

@planetf1

@planetf1 planetf1 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

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 AdapterFunctionTracingPlugin build 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 parent
Loading

The 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]
Loading

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:

  • activate and deactivate spans inside AdapterMixin.adapter_scope.
  • Invocation and phase start hooks plus correlation IDs for that scope.
  • Explicit parent/child span construction in the tracing plugin.
  • Compatibility handling for existing metric-only phase completions.
  • Regression tests for hook ordering, parentage, failures, and existing Hugging Face assertions.

Deferred:

Verification

  • uv run pytest test/ -m "not qualitative": 4,142 passed, 47 skipped.
  • Pre-commit: SPDX, Ruff, Mypy, codespell, and markdownlint passed.
  • GitHub quality checks pass on Python 3.11, 3.12, and 3.13.

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code was added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

  • AI coding assistants used

Adding a new component, requirement, sampling strategy, or tool?

  • Component
  • Requirement
  • Sampling Strategy
  • 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.

@github-actions github-actions Bot added the enhancement New feature or request label Aug 18, 2026
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>
@planetf1
planetf1 marked this pull request as ready for review August 20, 2026 10:04
@planetf1
planetf1 requested a review from a team as a code owner August 20, 2026 10:04
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>
@planetf1 planetf1 added the do-not-merge/hold Block merging this PR label Aug 24, 2026
@planetf1

Copy link
Copy Markdown
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)

Assisted-by: Codex
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1 planetf1 removed the do-not-merge/hold Block merging this PR label Aug 26, 2026
@planetf1
planetf1 marked this pull request as draft August 26, 2026 17:33
@planetf1

Copy link
Copy Markdown
Contributor Author

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>
@planetf1 planetf1 changed the title feat(telemetry): emit adapter-function prepare/activate/deactivate spans from a tracing plugin feat(telemetry): emit adapter-function activate/deactivate spans from a tracing plugin Aug 27, 2026
Assisted-by: Codex
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
@planetf1

Copy link
Copy Markdown
Contributor Author

Closing this draft for now.

The reviewed implementation provides useful LocalFile activation/deactivation spans, but it is not complete adapter-function observability: prepare, generation/parsing, embedded invocation outcomes, server-mediated adapters, content capture, and exemplar linkage all remain outside the slice.

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.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants