Add Strands tools and agents instrumentation. #1563
Conversation
✅MegaLinter analysis: Success
See detailed reports in MegaLinter artifacts
|
* Add strands to tox.ini * Add mock models for strands testing * Add simple test file to validate strands mocking
…thon-agent into feat-strands-agents
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-strands #1563 +/- ##
==================================================
Coverage ? 81.67%
==================================================
Files ? 209
Lines ? 24274
Branches ? 3841
==================================================
Hits ? 19825
Misses ? 3156
Partials ? 1293 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* Add strands to tox.ini * Add mock models for strands testing * Add simple test file to validate strands mocking
* Add strands to tox.ini * Add mock models for strands testing * Add simple test file to validate strands mocking
…thon-agent into feat-strands-agents
mergify
bot
added
tests-failing
hmstepanek
left a comment
hmstepanek
left a comment
There was a problem hiding this comment.
Some minor comments but everything else looks good to me.
| if not parent: | ||
| return wrapped(*args, **kwargs) | ||
| else: | ||
| parent = None |
There was a problem hiding this comment.
Regardless of wether there is a wrapper or not shouldn't we still set the parent to the current trace if there is a current trace?
There was a problem hiding this comment.
The idea here was that the if we had an async wrapper, then the trace context management would happen directly in the wrapper (ex: https://github.com/newrelic/newrelic-python-agent/blob/main/newrelic/common/async_wrapper.py#L25). This has been the same approach we take for other trace types such as ExternalTraces and FunctionTraces
| def _construct_base_tool_event_dict(strands_attrs, tool_results, transaction, linking_metadata): | ||
| try: | ||
| try: | ||
| tool_output = tool_results[-1]["content"][0] if tool_results else None |
There was a problem hiding this comment.
I wonder if by combining these under the same try-catch we might be missing cases where there's an error but no content?
There was a problem hiding this comment.
The inner try-except handles the case where there is no content accessible. The outer try-except was meant to handle any other errors that come up when trying to grab attribute data in the case that there was or wasn't content. If there wasn't content, we set safeguards and keep going without raising an error. So technically that would cover cases with and without content?
This PR adds instrumentation for Strands tools and agents. It also includes tests for single agent use cases.