-
Notifications
You must be signed in to change notification settings - Fork 168
feat(agno): add workflow wrapper for instrumentation #2450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(agno): add workflow wrapper for instrumentation #2450
Conversation
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...tation/openinference-instrumentation-agno/src/openinference/instrumentation/agno/__init__.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
| result = wrapped(*args, **kwargs) | ||
|
|
||
| # Check if result is an iterator (streaming) | ||
| is_streaming = hasattr(result, "__iter__") and not isinstance(result, (str, bytes)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Non-streaming iterables incorrectly detected as streams
The streaming detection uses hasattr(result, "__iter__") which matches any iterable including lists, tuples, dicts, and sets. While it excludes strings and bytes, it doesn't exclude other non-streaming collections. If a step returns a list or dict, it will be incorrectly treated as a stream, causing the wrong instrumentation path and potentially incorrect span attributes. The check should use isinstance(result, Iterator) from the typing module to properly detect actual iterators.
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...tation/openinference-instrumentation-agno/src/openinference/instrumentation/agno/__init__.py
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Show resolved
Hide resolved
| return str(response.content) | ||
| else: | ||
| return str(response.model_dump_json()) | ||
| return "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Unchecked method call may raise AttributeError
The _extract_output function assumes that any response without a content attribute has a model_dump_json method (Pydantic models). If a response object has neither attribute, calling response.model_dump_json() raises an AttributeError. Since this instrumentation code wraps user workflows, unexpected response types could crash the instrumentation and potentially disrupt the user's workflow execution. The function also has unreachable code at line 101 since all prior branches return.
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
...eninference-instrumentation-agno/src/openinference/instrumentation/agno/_workflow_wrapper.py
Outdated
Show resolved
Hide resolved
| # Check if result is an async iterator (streaming) | ||
| if hasattr(result, "__aiter__"): | ||
| # Streaming mode - return async generator that continues with this span | ||
| return self._arun_stream_continue(result, span, workflow_token, instance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Async methods missing error handling for span cleanup
The arun methods in _WorkflowWrapper, _StepWrapper, and aexecute in _ParallelWrapper lack try/except/finally around the initial wrapped(*args, **kwargs) call. Unlike their sync counterparts which have proper exception handling, these async methods start a span but if wrapped() raises a synchronous exception before returning a coroutine, the span is never ended and the error is not recorded. This causes orphaned spans in the tracing system.
What does this PR do?
Note
Add Agno workflow/step/parallel instrumentation with context propagation and graph-node spans, plus examples and tests.
Workflow.run/arun,Step.execute/execute_stream/aexecute/aexecute_stream, andParallel.execute/execute_stream/aexecute/aexecute_streamwith context propagation, graph node attributes, and streaming support.openinference.instrumentation.agno.utilsand refactor_RunWrapperto use them; minor adjustments to streaming output handling.workflow_with_condition_step.py,workflow_with_custom_function_step.py,workflow_with_parallel_step.py,workflow_with_structure_io.py.examples/requirements.txt(upgradeagno, addarize-phoenix,openai).test_workflow_instrumentation.pycovering basic, multi-step, async, conditional, and graph hierarchy cases; minor fix in existing tests.fastapito test dependencies.Written by Cursor Bugbot for commit edd117d. This will update automatically on new commits. Configure here.