Add Haystack integration (auto-instrumentation + Penelope Target) - #2009
Add Haystack integration (auto-instrumentation + Penelope Target)#2009Ayush7614 wants to merge 10 commits into
Conversation
Implement HaystackIntegration with OpenTelemetry tracing and Pipeline.run patching, add HaystackTarget for multi-turn Penelope testing, and add unit tests for issue rhesis-ai#1869.
![peqy[bot]](https://avatars.githubusercontent.com/in/1930020?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two potential correctness issues in the new Haystack tracing wrappers:
- Duplicate agent-input events:
add_agent_io_events(span, data, result)records input twice. - Potential double-instrumentation:
_patch_pipeline_run()is applied even when Haystack native OTel tracing is successfully enabled.
Also flagged a consistency issue where helpers still add events when is_tracing_disabled().
Found 3 issues (1 bug, 2 improvements).
| framework="haystack", | ||
| operation_type=AIAttributes.OPERATION_TRANSFORM, | ||
| ) as span: | ||
| add_agent_io_events(span, data, None) |
There was a problem hiding this comment.
Bug: add_agent_io_events(span, data, result) will record the input event again (since input_data is non-null). Probably want add_agent_io_events(span, None, result) on the second call (same for run_async).
There was a problem hiding this comment.
Fixed in the refactor (fallback patch now records input once and output once). Can mark this thread resolved.
| callback = _enable_haystack_tracing() | ||
| except Exception as exc: | ||
| logger.debug(f"Haystack tracing API unavailable, using pipeline patch: {exc}") | ||
| _patch_pipeline_run() |
There was a problem hiding this comment.
Improvement: _patch_pipeline_run() runs even when _enable_haystack_tracing() succeeds. If Haystack's OTel tracer already instruments Pipeline.run, this can lead to double spans/events. Suggest patching only as a fallback when enabling Haystack tracing fails (or add an explicit opt-in).
There was a problem hiding this comment.
Addressed: native Haystack tracing + ai.* translation is now the default; Pipeline.run patching is only a fallback when tracing enablement fails. Can resolve.
| attributes: Optional[dict[str, Any]] = None, | ||
| ) -> Iterator[trace.Span]: | ||
| """Create a span for a framework operation with latency and error handling.""" | ||
| if is_tracing_disabled(): |
There was a problem hiding this comment.
Improvement: when is_tracing_disabled() is true you yield get_current_span(), but callers (e.g. Haystack wrapper) still add events/attrs to it. Other integrations seem to fully no-op when tracing is disabled. Consider making observe_framework_call yield a non-recording span, or have add_agent_io_events/helpers check is_tracing_disabled().
There was a problem hiding this comment.
Addressed in new tracing_helpers: observe_framework_call yields INVALID_SPAN when tracing is disabled and helpers no-op in that case. Can resolve.
Avoid duplicate pipeline input events, patch Pipeline.run only when native Haystack tracing is unavailable, and no-op tracing helpers when tracing is disabled.
There was a problem hiding this comment.
Follow-up review: the previously raised issues look addressed in edfb242:
- no longer double-records input events (
add_agent_io_events(span, None, result)) - pipeline monkeypatching is now a fallback only when Haystack tracing enablement fails
- tracing helpers now yield/propagate
INVALID_SPANand no-op when tracing is disabled
LGTM.
|
cc: @harry-rhesis |
Add a translating exporter for native Haystack OTel spans, enable content tracing by default, extend the Penelope target for multi-component pipelines, and add examples plus schema-focused tests.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Keep Haystack optional deps alongside pydantic-ai and agent-framework integrations landed on main.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Improvement: In Otherwise this looks solid: native Haystack spans translated to Found 1 issue (0 critical, 1 improvement). |
Surface RhesisClient-before-auto_instrument requirement in enable() warnings, package docs, and auto-instrumentation guide; refuse enable when span translation cannot be wired.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Docs update for Haystack auto-instrumentation (including the “order matters: create Improvement: In Improvement: Otherwise this looks ready to ship. |
|
Hi @Ayush7614, I went back through this after the rework, and honestly the turnaround is great. The translating exporter, the content sanitizer, the nested One thing worth knowing: while this PR was open, Here's what I'd like to sort out before we merge, roughly in priority order: 1. Build the translator on 2. Honor 3. Add 4. Fresh CI + a human pass on the two new commits. The earlier peqy approval is on a commit this branch no longer resembles, and the translator plus the ~400-line 5. Wire up A few minor things, no need to block on them: Thanks again for pushing this so far. Points 1–3 are small and mechanical given what's already on the branch, and once they're in I think this is in good shape to land. Happy to pair on the genai.py refactor if it helps. |
…ntent capture, reuse TranslatedSpan - Wrap both BatchSpanProcessor and SimpleSpanProcessor (was only Batch), using shared genai helpers instead of private attribute poking. - Gate Haystack content tracing on RHESIS_DISABLE_CONTENT_CAPTURE instead of writing to os.environ, mirroring MAF/Pydantic AI. - Drop the haystack-private _TranslatedSpan and reuse genai.TranslatedSpan. - Remove unused set_token_attributes from tracing_helpers. - Document that HaystackTarget rejects file attachments. - Fix haystack example api_key handling; extend optional-imports test to cover haystack.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Improvement: HaystackIntegration.enable() still has a “side effects before fail-closed checks” edge case (Haystack tracing may be enabled even if exporter wrapping fails). The rest of the integration (exporter swap via shared genai helpers, content capture gating, optional-import coverage, Penelope target) looks solid.
Found 4 issues (0 critical, 4 improvements).
| return False | ||
|
|
||
| try: | ||
| haystack_tracer = _enable_haystack_tracing() |
There was a problem hiding this comment.
Improvement: enable() calls _enable_haystack_tracing() (global side effects: tracing.enable_tracing(...) and possibly content tracing) before verifying we can translate spans (SDK TracerProvider + wrappable exporter). If the provider check / exporter wrapping fails, enable() returns False but Haystack tracing may already be enabled, so you can end up emitting un-translated native spans (and potentially content) despite “fail closed”.
Fix: do the provider/exporter checks + wrapping first, then enable Haystack tracing; or add an explicit rollback path on the early-return branches.
There was a problem hiding this comment.
Addressed: enable() now calls _disable_haystack_tracing() on the early-return paths (non-Rhesis TracerProvider / no wrappable exporter), so we don’t leave the process emitting untranslated native Haystack spans/content. This thread can be resolved.
There was a problem hiding this comment.
Confirmed addressed in current tip: on the early-return paths (non-Rhesis TracerProvider / no wrappable exporter), enable() now calls _disable_haystack_tracing() so we don’t leave native Haystack tracing/content capture enabled when span translation can’t be installed. Safe to resolve.
| for synth_name, synth_attrs in mapping.synthesize_events(attributes, span_name=span_name): | ||
| if synth_name in existing_names: | ||
| continue | ||
| new_events.append(Event(name=synth_name, attributes=synth_attrs, timestamp=0)) |
There was a problem hiding this comment.
Improvement: synthesized events are created with timestamp=0. In the shared genai translator helpers we typically omit timestamps for synthesized events (while preserving original timestamps), letting the SDK handle defaults.
Fix: consider dropping the explicit
timestamp=0to matchgenai.translate_events()behavior.
There was a problem hiding this comment.
Fixed: synthesized events now omit the explicit timestamp=0 (we pass timestamp only for original events). Can resolve.
There was a problem hiding this comment.
Confirmed: synthesized events no longer set timestamp=0 (we preserve timestamps only for original events). This matches the shared genai.translate_events() behavior. Safe to resolve.
| translated.setdefault(AIAttributes.OPERATION_TYPE, _operation_to_ai_type(kind)) | ||
| if component_name: | ||
| translated.setdefault(AIAttributes.AGENT_NAME, str(component_name)) | ||
|
|
There was a problem hiding this comment.
Improvement: for component spans, translate_attributes() sets ai.agent.name from haystack.component.name. For non-agent operations (LLM/tool/retrieval/etc.) that can be semantically confusing, since ai.agent.name usually represents the pipeline/agent rather than a component.
Fix: consider reserving
ai.agent.namefor pipeline/agent spans and using more specific fields for components (e.g., tool already usesai.tool.name; for other components, attach component name under ahaystack.*attribute or a schema-appropriate attribute if one exists).
There was a problem hiding this comment.
Fixed: component spans no longer set ai.agent.name from haystack.component.name (agent name is reserved for pipeline spans; components keep identity under haystack.component.name / tool fields). Can resolve.
There was a problem hiding this comment.
Confirmed: component spans no longer set ai.agent.name from haystack.component.name (agent name reserved for pipeline spans; components keep identity under haystack.component.name / tool attrs). Safe to resolve.
| if isinstance(value, list): | ||
| if len(value) > 32 and value and all(isinstance(item, (int, float)) for item in value[:8]): | ||
| return f"[embedding vector omitted: {len(value)} dimensions]" | ||
| return [sanitize_for_tracing(item) for item in value] |
There was a problem hiding this comment.
Improvement: sanitize_for_tracing() has recursive sanitization but no max depth / cycle protection. A cyclic object graph (or extremely deep nesting) could still cause high CPU/stack usage before truncation.
Fix: add a depth limit and/or
seenset for containers/objects.
There was a problem hiding this comment.
Still applicable: sanitize_for_tracing() is recursively descending lists/dicts without a depth limit or cycle detection. If someone passes a cyclic object (or very deep nesting) as component input/output, this could blow the stack / burn CPU before truncation.
Fix idea: add
max_depth(e.g. 6–10) + aseenset keyed byid(value)for containers/objects, and return a placeholder like"[max depth reached]"/"[cycle omitted]"when tripped.
There was a problem hiding this comment.
Fixed now: sanitize_for_tracing() has a max depth (MAX_SANITIZE_DEPTH) and cycle handling (via seen + circular placeholder), so it won’t recurse forever on pathological inputs. 👍
This thread can be resolved.
- enable() now reverts Haystack tracing/content capture on the
early-return paths (no Rhesis TracerProvider / no wrappable
exporter) so a failed enable never leaks untranslated spans
(addresses peqy fail-closed review point).
- disable() reverts tracing content capture as well.
- Wire agents/dr-rhesis as the first real consumer: app.py calls
auto_instrument('haystack') after RhesisClient is constructed;
unstub serve_playground example and update architecture docs.
- Minor formatting cleanup in mapping.py/translator.py.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
… token fallback, align translator
- Remove os.environ.setdefault('HAYSTACK_CONTENT_TRACING_ENABLED') from
dr-rhesis app.py; content tracing is enabled in-memory by the SDK at
auto_instrument time, gated on RHESIS_DISABLE_CONTENT_CAPTURE. The env
write reintroduced the exact process-global leak Arman flagged in PR rhesis-ai#2009
point 2.
- Fix dead Secret.from_env_var(...) or Secret.from_token(...) fallback in the
haystack example (from_env_var is always truthy); only fall back when the
resolved value is None.
- Drop timestamp=0 on synthesized events to match genai.translate_events().
- Stop overloading ai.agent.name on every component span; reserve it for
pipeline/agent spans (peqy review).
- Remove now-unused os import; re-sort app.py imports.
There was a problem hiding this comment.
Looks good — the integration now reuses shared genai exporter-swap helpers, honors RHESIS_DISABLE_CONTENT_CAPTURE, wraps SimpleSpanProcessor, and wires the first in-repo consumer (dr-rhesis) + docs.
Non-blocking follow-ups (already noted inline):
translator.py: avoidtimestamp=0for synthesized events (omit timestamp likegenai.translate_events()does) so event ordering/timing isn’t weird.sanitize_for_tracing(): consider adding a depth limit / cycle protection to avoid pathological recursion on cyclic objects.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Thanks @Arman-Beykmohammadi for the thorough re-review — and @peqy for the re-audit. All points are now addressed on Arman's points 1–3 (priority):
Point 4 (fresh CI + human pass): CI is green on the current tip and the translator/ Point 5 (wire up Minor items:
Left as follow-up (not blocking, out of scope for this PR): the |
|
Changes look solid now: Haystack uses native tracing + ai.* translation via exporter wrapping, content capture is gated by Improvement: Found 1 issue (0 critical, 1 improvement). |
|
@Ayush7614 thanks for the update — I spot-checked the latest diff and it looks like the key review items are addressed (genai.py reuse, SimpleSpanProcessor wrapping via get/set_processor_exporter, content capture gated w/ no env leak, fail-closed rollback, synthesized events w/o Only thing I’d still keep as a (non-blocking) follow-up is hardening |
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Thanks @peqy — addressed the final follow-up in commit `40a781740`:
Verified: `ruff` clean and the full `tests/sdk/telemetry/` suite passes (383 passed, 1 skipped). This was the only outstanding (non-blocking) item, so the review follow-ups should now be fully cleared. |
|
Latest update looks good: No further issues from me. Ship it. |
|
@Ayush7614 confirmed — the new |
|
@Ayush7614 Thank you so much for the work. It looks very promising. Just one last comment, can you please also make sure the lint checks will pass properly? It's a requirement for PRs to be merged, to have all the checks pass. best, |
…ntegrations Bring the haystack telemetry mapping/translator and penelope target into line-length compliance (default ruff max-line-length 100) so CI lint passes.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Thanks for putting this together @Ayush7614, and sorry for the slow turnaround. There's a lot of solid mechanics in here (patch/unpatch state handling, the fallback logic, the disable path), but I can't merge it as it stands: the branch is ~2 months behind 1. The direction for Haystack changed, and it's no longer an SDK-side integrationSince this PR was opened, Haystack support in Rhesis moved out of the SDK and into a Haystack connector package, You can see the intended usage on # agents/visit-prep/examples/run_scenarios_traced.py
os.environ.setdefault("HAYSTACK_CONTENT_TRACING_ENABLED", "true") # before any haystack import
from haystack_integrations.tracing.rhesis import RhesisTracing
tracing = RhesisTracing("Visit-Prep", turn_span_name="function.visit_prep_turn")
tracing.start_conversation(str(uuid.uuid4()))
...
tracing.flush()and in So
2. The branch is stale in a way that would silently delete newer code
Same story for Please 3. The native-tracer path doesn't work on the Haystack version we targetThis is the bug that worries me most, because it fails quietly. # sdk/src/rhesis/sdk/telemetry/integrations/haystack.py:95-101
def _enable_haystack_tracing() -> Any:
from haystack import tracing
from haystack.tracing import OpenTelemetryTracer
Combined with # haystack.py:119-126
try:
return _enable_haystack_tracing()
except Exception as exc:
logger.debug(f"Haystack tracing API unavailable, using pipeline patch: {exc}")
_patch_pipeline_run()
return "haystack_patched"on Haystack 3.x (i.e. every user we actually have, per Related version-matrix issue: 4. Span names and attributes would be rejected by the backend# haystack.py:49-53
with observe_framework_call(
f"haystack.pipeline.run {pipeline_name}",
...
This is exactly the problem A few more things in that area:
5.
|
3 participants
Purpose
Add Haystack pipeline auto-instrumentation and a Penelope target for multi-turn NLP/RAG pipeline testing, aligned with the same
ai.*schema standard used by LangChain, LangGraph, and MAF integrations.Closes #1869
What Changed
SDK telemetry (
ai.*schema conformance)HaystackTranslatingExporter(MAF-style) to rewrite native Haystack OTel spans into Rhesisai.*before exporthaystack/mapping.pyfor span name, attribute, and event translation (pipeline →ai.agent.invoke, components →ai.llm.invoke,ai.retrieval,ai.tool.invoke, etc.)Pipeline.runpatching remains as fallback onlyintegrations/haystack/package (integration.py,mapping.py,translator.py)tracing_helpers.pywithsanitize_for_tracing()for binary/sensitive/signed-URL dataPenelope target
input_mappingandrun_inputsfor nested{component: {socket: value}}pipeline inputsa_send_message→pipeline.run_asyncfilesattachments are not supported (explicit error)Examples
examples/telemetry/haystack_example.py— transparentauto_instrument("haystack")flowpenelope/examples/haystack_minimal.py—PenelopeAgent.execute_test()demoTests
ai.*attributes (no mocking of the primary tracing path for schema checks)Testing
All 17 Haystack-related tests passing.