feat(haystack): migrate openinference-instrumentation-haystack - #318
feat(haystack): migrate openinference-instrumentation-haystack#318srinjoy356 wants to merge 18 commits into
Conversation
Pull request dashboard statusWaiting on the author · refreshed 2026-08-06 09:10 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
Assisted-by: Claude Sonnet 5
Migration review: opentelemetry-instrumentation-genai-haystackMode: greenfield migration Compared against:
1. Instrumented API surfaceOpenInference wraps every registered Haystack component generically (via the
2. Gaps and open issues
3. Significant behavioral changes
4. Test coverage4a. Unit-test matrix per wrapped method
4b. Conformance scenarios
Actually run against 4c. Docstring / README coveragenone — 5. Follow-up work
|
|
@srinjoy356 the Since |
Thanks for catching this! I wasn't sure whether it made sense to add a new committed doc file for the gap list, so I'd left the details in MIGRATION_REPORT.md (gitignored, generated by the migration skill) and just pointed to it from README.rst and a few docstrings — not realizing that left a dangling reference for anyone reading the package off PyPI. Per your suggestion, I've folded the gap list directly into README.rst instead:
Pushed in 647446e. Please take another look whenever you get a chance. |
|
@srinjoy356 checked 647446e — the references are gone from the package and the Known limitations section reads well on its own. Thanks for turning it around so fast. |
Thanks! I actually had kept a track of where I put the references. It was a known thing. I just forgot to remove it beforehand. |
eternalcuriouslearner
left a comment
eternalcuriouslearner
left a comment
There was a problem hiding this comment.
Thanks a lot for your contribution @srinjoy356. Couple of things:
- Can you please check if haystack has native instrumentation?
- If yes, can you see if they're emitting genai spans or not.
- After this analysis can you please cut down the verbosity of this pr? You can space it like: skeleton, inference spans, agent spans etc. For now if you don't mind can you please close this pr and first let me know if 1 & 2 are not in place before we proceed adding the telemetry support.
There was a problem hiding this comment.
Pull request overview
This PR adds a new Haystack GenAI instrumentation package (opentelemetry-instrumentation-genai-haystack) migrated from OpenInference, built on opentelemetry-util-genai, and wires it into the repo’s tox test matrix with unit + conformance coverage and VCR cassettes.
Changes:
- Introduces Haystack instrumentation that wraps
Pipeline.run*, classified componentrun/run_async, andTool.invoke*into util-genai invocations. - Adds unit tests, conformance scenarios, and VCR cassettes for chat/tool-calling, embeddings, retrieval, workflows, and agents.
- Updates repo-level tox envs and gitignore to include the new package and workflows.
Reviewed changes
Copilot reviewed 44 out of 48 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tox.ini | Adds haystack test/lint/conformance envs and deps. |
| .gitignore | Ignores local .env* files and preserves existing MIGRATION_REPORT ignore. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/pyproject.toml | New package metadata, deps, entry point, towncrier config. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/README.rst | End-user docs: what’s instrumented, limitations, content capture + completion hook. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/LICENSE | Package license file. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/.changelog/318.added | Towncrier fragment for new instrumentation package. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/.changelog/.gitignore | Keeps .changelog/ tracked structure. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/init.py | Instrumentor implementation + module docstring. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/version.py | Package version definition. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/package.py | Declares instrumentation_dependencies(). |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/patch.py | Core wrappers for pipeline, components, and tools. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/component_types.py | Classifies Haystack components into GenAI operation types. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/message_utils.py | Maps Haystack message/tool/document shapes into util-genai typed message models. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/provider.py | Heuristic provider inference from component class name. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conftest.py | Shared fixtures, VCR config, and env setup for tests. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/requirements.oldest.txt | Oldest-factor test requirements (currently empty aside from comments). |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/requirements.latest.txt | Latest-factor test requirements (installs haystack-ai + local editable deps). |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_instrumentor.py | Verifies entry point + wrap/unwrap behavior. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_workflow.py | Unit tests for invoke_workflow spans + double-count prevention. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_inference.py | Unit tests for chat inference spans, content capture, errors, and tool-call parts. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_embedding.py | Unit tests for embeddings spans and dimension count. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_retrieval.py | Unit tests for retrieval spans, query/doc capture, and top_k handling. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_tool.py | Unit tests for execute_tool spans (sync/async/error/no-content). |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_agent.py | Unit tests for invoke_agent span nesting + late component registration regression. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_utils.py | Shared assertion helpers for spans and message JSON parsing. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/test_conformance.py | Conformance runner executing all scenarios. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/init.py | Conformance package marker. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/_known_gaps.py | Declares reusable ExpectedViolation constants. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/inference.py | Conformance scenario for chat inference. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/tool_calling.py | Conformance scenario for tool-call parts in output messages. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/embedding.py | Conformance scenario for embeddings. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/retrieval.py | Conformance scenario for retrieval. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/invoke_workflow.py | Conformance scenario for invoke_workflow + nested chat. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/conformance/invoke_agent.py | Conformance scenario for invoke_agent + nested chat/execute_tool. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/inference_conformance.yaml | VCR cassette for inference conformance scenario. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/tool_calling_conformance.yaml | VCR cassette for tool-calling conformance scenario. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/workflow_conformance.yaml | VCR cassette for workflow conformance scenario. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/embedding_conformance.yaml | VCR cassette for embedding conformance scenario. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_chat_generator_sync.yaml | VCR cassette for sync chat generator unit test. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_chat_generator_async.yaml | VCR cassette for async chat generator unit test (includes PostHog batch interaction). |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_chat_generator_no_content_capture.yaml | VCR cassette for no-content chat test. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_chat_generator_error.yaml | VCR cassette for auth error path test. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_tool_calling_captures_tool_call_on_output_message.yaml | VCR cassette for tool-call output-message unit test. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_pipeline_run_produces_workflow_and_chat_spans.yaml | VCR cassette for sync pipeline workflow test. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_pipeline_run_async_produces_workflow_and_chat_spans.yaml | VCR cassette for async pipeline workflow test (includes PostHog batch interaction). |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/cassettes/test_document_embedder.yaml | VCR cassette for embedder unit test. |
| instrumentation/opentelemetry-instrumentation-genai-haystack/tests/init.py | Test package marker. |
Comments suppressed due to low confidence (1)
instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/patch.py:295
- Same issue as the generator wrapper: passing
provider=""will emit an emptygen_ai.provider.nameattribute when the provider can’t be inferred. Use a non-empty fallback (or adjust util-genai to make provider truly optional for framework instrumentations).
request_model = getattr(component, "model", None)
server_address, server_port = _server_address_and_port(component)
return handler.embedding(
provider=infer_provider(component) or "",
request_model=request_model,
server_address=server_address,
server_port=server_port,
)
| # Copyright The OpenTelemetry Authors | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| _instruments = ("haystack-ai >= 2.18.0",) |
| Message content capture can be enabled by setting the environment variable: | ||
| ``OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=true`` | ||
|
|
| invocation = handler.inference( | ||
| provider=infer_provider(component) or "", | ||
| request_model=request_model, | ||
| operation_name=operation_name, | ||
| server_address=server_address, | ||
| server_port=server_port, | ||
| ) |
| "distinct_id": "0fbae5f1-414c-4dde-a02e-459ab7e75ae0", "event": "Pipeline run | ||
| (2.x)", "uuid": "4d1cce43-d74b-47c6-ba1a-10cdc05d8b90"}], "historical_migration": | ||
| false, "sentAt": "2025-10-14T03:56:18.209957+00:00", "api_key": "phc_C44vUK9R1J6HYVdfJarTEPqVAoRPJzMXzFcj8PIrJgP"}' | ||
| headers: {} | ||
| method: POST | ||
| uri: https://eu.i.posthog.com/batch/ |
- Fix instrumentation_dependencies() version floor to match pyproject.toml (haystack-ai >= 3.0.0, was stale at >= 2.18.0). - Fix __init__.py docstring documenting a non-existent OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=true value; document the real NO_CONTENT/SPAN_ONLY/EVENT_ONLY/SPAN_AND_EVENT modes, matching README. - Stop emitting gen_ai.provider.name="" for unmapped providers; fall back to "unknown" instead, matching the precedent in the already-merged genai-langchain package. Corrected README wording to match. - Strip a leftover Haystack/PostHog telemetry interaction (with a real project API key) from two VCR cassettes recorded before HAYSTACK_TELEMETRY_ENABLED=False was added to conftest.py; the interaction is unused dead weight now that telemetry is disabled in tests. Assisted-by: Claude Sonnet 5
…ce-haystack Resolves the merge-conflict indicator GitHub was showing on PR open-telemetry#318 (.gitignore, tox.ini envlist, uv.lock all had additive conflicts against main's newer state -- agno/llama-index instrumentation, langchain updates, etc. -- with no actual overlap in intent). uv.lock was regenerated via `uv lock` rather than hand-merged.
Thanks Copilot — all four points were real, addressed in 4499fe9:
Also merged All 22 unit tests still pass and ruff (including the version bump from #332) is clean. @lmolkova |
|
@eternalcuriouslearner Thanks for the questions — here's what I found: 1. Does Haystack have native instrumentation? 2. Does it emit GenAI semantic-convention spans?
None of these are So there's no overlap: nothing in Haystack, native or otherwise, emits 3. Splitting the PR / closing it for now |
|
I ran tox -e generate to update instrumentation/README.md with the new Haystack package, which should resolve the failing generate and check CI jobs. Let me know if there's anything else needed! @lmolkova @eternalcuriouslearner |
Yes can you please raise an issue on the parent repository of haystack to see if they're interested in adding native instrumentation while we are pushing through this. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 49 changed files in this pull request and generated no new comments.
Suppressed comments (3)
instrumentation/opentelemetry-instrumentation-genai-haystack/pyproject.toml:66
- This package enables towncrier with
filename = "CHANGELOG.md", but the PR doesn't addCHANGELOG.mdin the package root. That will typically breaktox -e changelog-preview/ release-time changelog compilation for this package. Add a minimalCHANGELOG.md(with the<!-- changelog start -->marker) matching the pattern used by other instrumentation packages.
[tool.towncrier]
directory = ".changelog"
filename = "CHANGELOG.md"
start_string = "<!-- changelog start -->\n"
template = "../../scripts/changelog_template.j2"
issue_format = "[#{issue}](https://github.com/open-telemetry/opentelemetry-python-genai/pull/{issue})"
wrap = true
instrumentation/opentelemetry-instrumentation-genai-haystack/README.rst:30
- New instrumentation packages in this repo are expected to ship minimal runnable examples under
examples/(bothmanual/andzero-code/variants). This PR adds the package but no examples directory, which makes it harder for users to validate the instrumentation quickly and is inconsistent with the repo’s instrumentation contribution guidelines.
Usage
-----
.. code-block:: python
from opentelemetry.instrumentation.genai.haystack import HaystackInstrumentor
# Instrument Haystack
HaystackInstrumentor().instrument()
instrumentation/opentelemetry-instrumentation-genai-haystack/src/opentelemetry/instrumentation/genai/haystack/patch.py:151
Pipeline.run_async_generator()wrapper only callsinvocation.stop()in theelse:branch, so if the caller stops consuming the async generator early (e.g., breaks out of the loop), the span never gets finalized. Using afinally:tostop()ensures the workflow span closes on normal exhaustion, errors, and early close (withstop()being a no-op iffail()already ended it).
else:
invocation.stop()
eternalcuriouslearner
left a comment
eternalcuriouslearner
left a comment
There was a problem hiding this comment.
can you please fix the failing gh actions?
yes I am on it. There are some naming convention issues. I will fix them right up. |
I've pushed a fix for the failing CI checks. The test assertions for ErrorAttributes.ERROR_TYPE were previously checking for the short exception name (e.g., AuthenticationError), but the semantic conventions dictate using the fully qualified module name (openai.AuthenticationError). I've updated the tests to dynamically resolve the fully qualified exception name (f"{type(excinfo.value).module}.{type(excinfo.value).name}"). I ran the specific instrumentation-genai-haystack test matrix locally to verify the fix, and the assertions now pass correctly. Let's see if the CI goes fully green now! |
Can you please fix the gh actions pipeline. |
Add opentelemetry-instrumentation-genai-haystack, porting the OpenInference Haystack instrumentation onto opentelemetry-util-genai's typed invocations (WorkflowInvocation for Pipeline.run/run_async, InferenceInvocation for classified generators, EmbeddingInvocation for embedders, RetrievalInvocation for retrievers/rankers) per .github/skills/migrate-from-openinference. Targets haystack-ai >= 3.0.0 only (Haystack 3.x merged AsyncPipeline into Pipeline and dropped the plain-text Generator/websearch components that 2.x-era OpenInference tests targeted). See MIGRATION_REPORT.md (gitignored, local review artifact) for the full gap list and follow-up items. Assisted-by: Claude Sonnet 5
Closes several gaps identified in the initial migration: - Classify haystack.components.agents.agent.Agent -> AgentInvocation (invoke_agent). Its own chat_generator/tool calls are already captured as nested chat/execute_tool spans via existing wrapping. - Wrap haystack.tools.tool.Tool.invoke/invoke_async -> ToolInvocation (execute_tool), independent of the component registry (a Tool is not a Haystack Component). - Wrap Pipeline.run_async_generator directly, using a contextvar to skip the span when it's driven internally by an already-wrapped run_async() call, so direct callers of the generator form now get a workflow span without double-counting. - Broaden provider.py's class-name map to cover Cohere, Amazon Bedrock, and Google Vertex AI generators/embedders (verified against each integration package's real source), not just OpenAI/Azure OpenAI. Also fixes a real correctness bug found while testing Agent support: component classes defined *after* instrument() runs were only ever wrapped if later invoked through a Pipeline (via the _run_component hook). Agent calls its chat_generator directly, never through a Pipeline, so this silently produced zero telemetry for that common ordering. Replaced the Pipeline-hook-based lazy discovery with a hook on _Component._component (the @component decorator's actual registration point), which catches every component the instant it's defined regardless of import order or how it's later invoked. MIGRATION_REPORT.md updated accordingly (gitignored, local review artifact). Assisted-by: Claude Sonnet 5
…er weaver Actually ran the tox test matrix (uv run tox -e ...-latest/-oldest/-conformance) instead of just pytest directly, and got a real weaver binary on PATH instead of relying on the auto-skip path. That surfaced genuine bugs, now fixed: - gen_ai.request.top_k was cast to float; the semconv registry requires int. - gen_ai.response.id and server.address/port were never populated for generator/embedder spans. response.id now extracts from reply.meta when a generator provides one (verified with the fake conformance generator); real Haystack OpenAIChatGenerator drops it entirely, which is a genuine upstream limitation, not a code gap. server.address/port now reads the SDK client's base_url when available -- which, it turns out, is every Pipeline-driven call (Pipeline.run() calls warm_up() first) and every call after a standalone component's first one. Declared the remaining permanent gaps (response.id, server.address on a component's first standalone call, tool.call.id) as ExpectedViolation in tests/conformance/_known_gaps.py per-scenario, rather than leaving them as undeclared failures or skipping the scenarios. All 6 conformance scenarios now pass against real weaver validation (previously only verified via the auto-skip path with no weaver binary installed). -latest and -oldest tox envs re-verified (22 passed each), plus tox -e precommit and tox -e typecheck, matching the CI gates in AGENTS.md. MIGRATION_REPORT.md updated accordingly (gitignored, local review artifact). Assisted-by: Claude Sonnet 5
Assisted-by: Claude Sonnet 5
…ngling refs MIGRATION_REPORT.md is gitignored by design and never ships in the repo, so pointers to it from README.rst (which becomes the PyPI long description) and various source/test docstrings were dangling. Moves the actual gap list into README.rst's new "Known limitations" section and drops the now-redundant references elsewhere, since most already carried the rationale inline. Assisted-by: Claude Sonnet 5
- Fix instrumentation_dependencies() version floor to match pyproject.toml (haystack-ai >= 3.0.0, was stale at >= 2.18.0). - Fix __init__.py docstring documenting a non-existent OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=true value; document the real NO_CONTENT/SPAN_ONLY/EVENT_ONLY/SPAN_AND_EVENT modes, matching README. - Stop emitting gen_ai.provider.name="" for unmapped providers; fall back to "unknown" instead, matching the precedent in the already-merged genai-langchain package. Corrected README wording to match. - Strip a leftover Haystack/PostHog telemetry interaction (with a real project API key) from two VCR cassettes recorded before HAYSTACK_TELEMETRY_ENABLED=False was added to conftest.py; the interaction is unused dead weight now that telemetry is disabled in tests. Assisted-by: Claude Sonnet 5
6fd033c to
1973aa3
Compare
|
Just force-pushed a rebase on top of the latest main to resolve the tox.ini conflicts from the new agents. I also bundled in a few fixes for the CI edge cases that were failing earlier: fixed the shellcheck path resolution issues in tox.ini, forced LF line endings on the shell scripts to fix Windows checkouts, and downgraded Sphinx to <9.0.0 so docs/linting pass again on Python 3.10. Verified everything passes locally! 
|
@srinjoy356 can you please break the pr for quicker review. This is verbose and review time will be higher for this pr. |
I'm on it. Help me with one thing how should I do it? As the tests seems to be passing for all the ones? Also I'm opening a pr on haystack today. |
Description
Migrates
openinference-instrumentation-haystackontoopentelemetry-util-genai's typed invocations, per the process in.github/skills/migrate-from-openinference/SKILL.md.Instruments:
Pipeline.run/run_async/run_async_generator→invoke_workflow(never double-counted when the generator form is driven internally byrun_async)runtype hints) →chat/text_completion(generators),embeddings(embedders),retrieval(retrievers/rankers),invoke_agent(Agent)Tool.invoke/invoke_async→execute_toolTargets
haystack-ai >= 3.0.0only, not OpenInference's>=2.18.0range — Haystack 3.0 mergedAsyncPipelineintoPipelineand dropped the plain-textGenerator/websearchcomponents the 2.x-era OpenInference tests used, so supporting both API shapes wasn't worth it for a first migration PR (see repo's "support only latest major versions" guidance).Also fixes a correctness bug found while testing: components defined after
instrument()runs were only wrapped if later invoked through aPipeline. SinceAgentcalls itschat_generatordirectly (never through aPipeline), this produced zero telemetry for that path — and would affect anyone who instruments at app startup before importing their pipeline components. Fixed by hooking_Component._component(the@componentdecorator's actual registration point) instead ofPipeline._run_component.Full gap/coverage analysis is in the comment below (from
MIGRATION_REPORT.md), not here, per this repo's guidance to keep AI-assisted analysis out of the PR description.Towards #141
Type of change
How has this been tested?
uv run tox -e py312-test-instrumentation-genai-haystack-latest— 22 passeduv run tox -e py310-test-instrumentation-genai-haystack-oldest— 22 passed (UV_RESOLUTION=lowest-direct, floors tohaystack-ai==3.0.0)uv run tox -e py314-test-instrumentation-genai-haystack-conformance— 6 passed, validated against realweaversemconv checks (not just the auto-skip path)uv run tox -e precommit— cleanuv run tox -e typecheck— 0 errorsManual verification against a local SigNoz instance — real
OpenAIChatGenerator+ a real tool call through anAgent:Trace list — 4 spans under
haystack-agent-tool-smoketest:invoke_agent Agent,chat×2,execute_tool get_weather.Waterfall view —
invoke_agent Agentcorrectly parenting all three child spans, with attributes panel showinggen_ai.agent.nameandgen_ai.input.messages.invoke_agentspan'sgen_ai.output.messages— correctly sliced to just the 3 new messages (tool call → tool result → final answer), not the echoed input.execute_toolspan attributes —gen_ai.tool.call.arguments,gen_ai.tool.call.result,gen_ai.tool.name,gen_ai.tool.type.execute_toolspan's raw metadata —parent_span_id/references: CHILD_OFconfirming the nesting at the data level.Checklist
.changelog/318.added