-
Notifications
You must be signed in to change notification settings - Fork 45
Add ADK instrumentation sample #418
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
Conversation
|
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated by snippet-bot.
|
aabmass
left a comment
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.
Looks great thank you!
| @tracer.start_as_current_span("run_sql") | ||
| def run_sql_tool(sql_query: str, tool_context: ToolContext) -> dict[str, Any]: | ||
| """Runs a SQLite query. The SQL query can be DDL or DML. Returns the rows if it's a SELECT query.""" | ||
| current_session_db_path = tool_context.state[SESSION_DB_KEY] |
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.
Should we check if this is None (model hasn't decided to create the session yet) and return error? I think as is it would throw an exception but I'm not sure how ADK handles tool call exceptions
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.
We can certainly do that, but, the model should figure out what tool to use and use it accordingly - if the dbpath is null, then the model should invoke the create_database tool to create one before executing the query.
If we start anticipating exceptions then we should also validate the sql_query before executing it.
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.
Did some experiments -
Added a tool that always raises exception. It halts the agent's response. It is recommended to always return structured responses from tools even in case of exception.
Based on this, I added a return statement to the create_db tool and added a check. LLM should be able to read the responses and recover.
Sample adopted from https://github.com/aabmass/opentelemetry-operations-python/tree/adk-sample/samples/adk-sql-agent.
Testing
TODO