Skip to content

fix(instructor): trace extraction requests without serializing clients - #385

Open
nik13 wants to merge 5 commits into
devfrom
fix/instructor-request-tracing
Open

nik13 wants to merge 5 commits into
devfrom
fix/instructor-request-tracing

Conversation

@nik13

@nik13 nik13 commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Repairs Instructor request tracing while retaining InstructorInstrumentor and the existing application APIs. Keep the package; do not release the unchanged implementation.

  • Stop tracing instructor.patch() construction and remove SDK-client/object introspection. Generated extraction callables are traced instead.
  • Trace all four sync/async public create variants, plus v2-generated callables and cache hits, without duplicate Instructor spans. Provider instrumentation remains active beneath a CHAIN extraction span.
  • Use per-call official hooks where supported for attempt/validation counts and supplied usage. No shared-client listener registration, and no validation-exception payloads added to hook metadata.
  • Serialize parsed output once, omit the raw SDK completion from create_with_completion, and retain lazy stream lifetime through consumption/error/close/cancellation. Stream output retains only the latest parsed item.
  • Keep generated callbacks inert after uninstrumentation and retain the existing shared Protect-wrapper ownership guarantees.
  • Require the already-published fi-instrumentation-otel>=1.2.2 so hidden content also hides exception events/status details. The targeted Poetry lock update changes only this core package.
  • Replace obsolete helper tests that asserted client introspection/double encoding with behavioral privacy/lifecycle contracts. Refresh the package README with from_openai, provider composition, and privacy defaults.

Why?

On the starting dev head d6a9966dd4d5d5fb344f7f5ab11b7483c2c2a6bc, offline tests reproduced a constructor span containing a synthetic client key, no extraction span for the documented patch() route, no create_with_completion span, TOOL labeling, and double-encoded model output. Initial regressions failed 10 tests on Instructor 1.17.0 and 9 on 1.0.0.

Design references, independently implemented rather than copied:

How was it tested?

All local test executions used an OS sandbox denying outbound networking except localhost. SDK HTTP traffic used in-process synthetic transports; no vendor/model API was called and no real credentials were used.

Environment Result
Instructor 1.17.0 / OpenAI 3.3.0 57 passed
Instructor 1.0.0 / OpenAI 1.14.3 / Pydantic 2.9.2 52 passed, 5 feature-unavailable skips
Instructor 1.10.0 / OpenAI 1.98.0 (framework versions in Poetry lock) 53 passed, 4 feature-unavailable skips

All use released core 1.2.2. Tests run: python/frameworks/instructor/tests and python/tests/test_framework_instructor.py. Coverage includes real SDK sync/async routes, streaming, retries, concurrent hook isolation, cache hits, positional arguments/hooks, suppression, return identity, wrapper ownership, cancellation, and serialized OTLP protobuf privacy/output-shape checks.

  • README and held docs snippets executed offline with released traceAI-openai==0.1.11: exactly one model-call child under one Instructor span, with content hidden on the OTLP wire.
  • Wheel and sdist build passed; git diff --check passed.
  • Core 1.1.0 was separately tested and failed both exception-redaction regressions, motivating the dependency floor rather than weakening the tests.
  • No local Ruff configuration, migrations, or frontend contracts are touched. No lint/type-check result is claimed.

E2E coverage

Not applicable (no e2e/ harness); real framework/provider contracts use offline HTTP transports.

Compatibility and rollout

  • Branch and PR target are dev, as requested for this repair; no manual package-version or CHANGELOG bump.
  • Instructor 1.0 itself cannot create partial streams with the Pydantic version required by the core (create_model keyword incompatibility). Tests prove the same native exception is preserved and traced; this repair does not claim to fix that upstream incompatibility. Upgrade Instructor for partial streaming.
  • Instructor 1.17 materializes synchronous partial streams itself. The integration preserves that eager result and does not claim to restore SDK laziness. Other lazy iterators remain lazy.
  • Application methods and returned values remain compatible; tracing changes intentionally correct span kind, output encoding, and constructor-vs-request boundaries. Instrument before constructing patched clients.
  • A corrected customer docs page is held locally until this package repair is released. No docs PR was opened against unreleased behavior. No registry publish, tag, or merge is part of this PR.
  • Disabling the instrumentor detaches owned wrappers and disables generated callbacks; do not roll back to the old client-serialization implementation to disable tracing.

Review

Self-reviewed against internal minimal-code, typing, failure-mode, review, and scenario standards. No unresolved blocking finding in the final diff; independent human review remains required. No unrelated package changes or overlapping Instructor PR were found before implementation. The repository template references docs/VOCABULARY.md, which is absent in this checkout.

🤖 Generated with Claude Code

Cover public create variants and generated callables with per-call hooks, preserve lazy stream lifetimes, and require privacy-safe core telemetry.

Co-Authored-By: Claude Code <noreply@anthropic.com>
nik13 and others added 4 commits November 2, 2026 14:50

This branch has not been deployed

No deployments
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.

1 participant