fix(llmobs): treat google adk tools as either BaseTool or generic function
#15402
Conversation
|
|
sabrenner
changed the title
name and description directly for pure function agent tools for google adkBaseTool or generic function
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 223 ± 3 ms. The average import time from base is: 227 ± 3 ms. The import time difference between this PR and base is: -4.4 ± 0.1 ms. Import time breakdownThe following import paths have shrunk:
|
Performance SLOsComparing candidate sabrenner/google-adk-tool-bug (db07aac) with baseline main (e551758) 📈 Performance Regressions (2 suites)📈 iastaspectsospath - 24/24✅ ospathbasename_aspectTime: ✅ 5.054µs (SLO: <10.000µs 📉 -49.5%) vs baseline: 📈 +18.4% Memory: ✅ 40.167MB (SLO: <41.000MB -2.0%) vs baseline: +4.6% ✅ ospathbasename_noaspectTime: ✅ 1.085µs (SLO: <10.000µs 📉 -89.2%) vs baseline: +0.1% Memory: ✅ 40.324MB (SLO: <41.000MB 🟡 -1.6%) vs baseline: +4.9% ✅ ospathjoin_aspectTime: ✅ 6.099µs (SLO: <10.000µs 📉 -39.0%) vs baseline: -0.5% Memory: ✅ 40.423MB (SLO: <41.000MB 🟡 -1.4%) vs baseline: +5.4% ✅ ospathjoin_noaspectTime: ✅ 2.293µs (SLO: <10.000µs 📉 -77.1%) vs baseline: ~same Memory: ✅ 40.226MB (SLO: <41.000MB 🟡 -1.9%) vs baseline: +4.9% ✅ ospathnormcase_aspectTime: ✅ 3.507µs (SLO: <10.000µs 📉 -64.9%) vs baseline: ~same Memory: ✅ 40.383MB (SLO: <41.000MB 🟡 -1.5%) vs baseline: +5.4% ✅ ospathnormcase_noaspectTime: ✅ 0.572µs (SLO: <10.000µs 📉 -94.3%) vs baseline: -1.1% Memory: ✅ 40.442MB (SLO: <41.000MB 🟡 -1.4%) vs baseline: +5.3% ✅ ospathsplit_aspectTime: ✅ 4.887µs (SLO: <10.000µs 📉 -51.1%) vs baseline: +0.4% Memory: ✅ 40.285MB (SLO: <41.000MB 🟡 -1.7%) vs baseline: +4.9% ✅ ospathsplit_noaspectTime: ✅ 1.596µs (SLO: <10.000µs 📉 -84.0%) vs baseline: -0.3% Memory: ✅ 40.285MB (SLO: <41.000MB 🟡 -1.7%) vs baseline: +5.0% ✅ ospathsplitdrive_aspectTime: ✅ 3.694µs (SLO: <10.000µs 📉 -63.1%) vs baseline: -0.5% Memory: ✅ 40.265MB (SLO: <41.000MB 🟡 -1.8%) vs baseline: +5.1% ✅ ospathsplitdrive_noaspectTime: ✅ 0.700µs (SLO: <10.000µs 📉 -93.0%) vs baseline: -0.5% Memory: ✅ 40.246MB (SLO: <41.000MB 🟡 -1.8%) vs baseline: +5.0% ✅ ospathsplitext_aspectTime: ✅ 4.588µs (SLO: <10.000µs 📉 -54.1%) vs baseline: +0.9% Memory: ✅ 40.246MB (SLO: <41.000MB 🟡 -1.8%) vs baseline: +4.7% ✅ ospathsplitext_noaspectTime: ✅ 1.379µs (SLO: <10.000µs 📉 -86.2%) vs baseline: ~same Memory: ✅ 40.226MB (SLO: <41.000MB 🟡 -1.9%) vs baseline: +4.7% 📈 telemetryaddmetric - 30/30✅ 1-count-metric-1-timesTime: ✅ 3.401µs (SLO: <20.000µs 📉 -83.0%) vs baseline: 📈 +14.7% Memory: ✅ 35.232MB (SLO: <35.500MB 🟡 -0.8%) vs baseline: +4.9% ✅ 1-count-metrics-100-timesTime: ✅ 200.221µs (SLO: <220.000µs -9.0%) vs baseline: -0.4% Memory: ✅ 35.232MB (SLO: <35.500MB 🟡 -0.8%) vs baseline: +5.2% ✅ 1-distribution-metric-1-timesTime: ✅ 3.234µs (SLO: <20.000µs 📉 -83.8%) vs baseline: -1.8% Memory: ✅ 35.193MB (SLO: <35.500MB 🟡 -0.9%) vs baseline: +4.9% ✅ 1-distribution-metrics-100-timesTime: ✅ 212.220µs (SLO: <230.000µs -7.7%) vs baseline: -1.6% Memory: ✅ 35.271MB (SLO: <35.500MB 🟡 -0.6%) vs baseline: +5.0% ✅ 1-gauge-metric-1-timesTime: ✅ 2.188µs (SLO: <20.000µs 📉 -89.1%) vs baseline: +0.3% Memory: ✅ 35.212MB (SLO: <35.500MB 🟡 -0.8%) vs baseline: +4.9% ✅ 1-gauge-metrics-100-timesTime: ✅ 136.107µs (SLO: <150.000µs -9.3%) vs baseline: ~same Memory: ✅ 35.154MB (SLO: <35.500MB 🟡 -1.0%) vs baseline: +4.6% ✅ 1-rate-metric-1-timesTime: ✅ 3.068µs (SLO: <20.000µs 📉 -84.7%) vs baseline: -1.2% Memory: ✅ 35.212MB (SLO: <35.500MB 🟡 -0.8%) vs baseline: +4.9% ✅ 1-rate-metrics-100-timesTime: ✅ 214.758µs (SLO: <250.000µs 📉 -14.1%) vs baseline: ~same Memory: ✅ 35.193MB (SLO: <35.500MB 🟡 -0.9%) vs baseline: +5.0% ✅ 100-count-metrics-100-timesTime: ✅ 20.288ms (SLO: <22.000ms -7.8%) vs baseline: ~same Memory: ✅ 35.154MB (SLO: <35.500MB 🟡 -1.0%) vs baseline: +4.6% ✅ 100-distribution-metrics-100-timesTime: ✅ 2.230ms (SLO: <2.300ms -3.0%) vs baseline: -0.3% Memory: ✅ 35.232MB (SLO: <35.500MB 🟡 -0.8%) vs baseline: +4.9% ✅ 100-gauge-metrics-100-timesTime: ✅ 1.399ms (SLO: <1.550ms -9.8%) vs baseline: -0.5% Memory: ✅ 35.291MB (SLO: <35.500MB 🟡 -0.6%) vs baseline: +5.2% ✅ 100-rate-metrics-100-timesTime: ✅ 2.209ms (SLO: <2.550ms 📉 -13.4%) vs baseline: +0.3% Memory: ✅ 35.154MB (SLO: <35.500MB 🟡 -1.0%) vs baseline: +4.6% ✅ flush-1-metricTime: ✅ 4.418µs (SLO: <20.000µs 📉 -77.9%) vs baseline: -0.7% Memory: ✅ 35.154MB (SLO: <35.500MB 🟡 -1.0%) vs baseline: +4.6% ✅ flush-100-metricsTime: ✅ 173.253µs (SLO: <250.000µs 📉 -30.7%) vs baseline: ~same Memory: ✅ 35.311MB (SLO: <35.500MB 🟡 -0.5%) vs baseline: +4.7% ✅ flush-1000-metricsTime: ✅ 2.182ms (SLO: <2.500ms 📉 -12.7%) vs baseline: -0.4% Memory: ✅ 36.019MB (SLO: <36.500MB 🟡 -1.3%) vs baseline: +4.6% 🟡 Near SLO Breach (18 suites)🟡 coreapiscenario - 10/10 (1 unstable)
|
…unction (#15402) ## Description Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all `BaseTool`s, when really they can also be a generic function. This PR incorporates that logic. Fixes #15174 ## Testing Changing one of the tools in the tests to a function instead of a `FunctionTool` reproduced this issue in tests (error + missing span event). ## Risks None ## Additional Notes We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents. (cherry picked from commit c8b04fc)
…unction (#15402) ## Description Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all `BaseTool`s, when really they can also be a generic function. This PR incorporates that logic. Fixes #15174 ## Testing Changing one of the tools in the tests to a function instead of a `FunctionTool` reproduced this issue in tests (error + missing span event). ## Risks None ## Additional Notes We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents. (cherry picked from commit c8b04fc) Signed-off-by: Sam Brenner <sam.brenner@datadoghq.com>
…unction (#15402) ## Description Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all `BaseTool`s, when really they can also be a generic function. This PR incorporates that logic. Fixes #15174 ## Testing Changing one of the tools in the tests to a function instead of a `FunctionTool` reproduced this issue in tests (error + missing span event). ## Risks None ## Additional Notes We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents. (cherry picked from commit c8b04fc)
…unction (#15402) ## Description Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all `BaseTool`s, when really they can also be a generic function. This PR incorporates that logic. Fixes #15174 ## Testing Changing one of the tools in the tests to a function instead of a `FunctionTool` reproduced this issue in tests (error + missing span event). ## Risks None ## Additional Notes We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents. (cherry picked from commit c8b04fc) Signed-off-by: Sam Brenner <sam.brenner@datadoghq.com>
…unction [backport 4.0] (#15423) Backport c8b04fc from #15402 to 4.0. ## Description Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all `BaseTool`s, when really they can also be a generic function. This PR incorporates that logic. Fixes #15174 ## Testing Changing one of the tools in the tests to a function instead of a `FunctionTool` reproduced this issue in tests (error + missing span event). ## Risks None ## Additional Notes We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents. Signed-off-by: Sam Brenner <sam.brenner@datadoghq.com> Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>
…unction [backport 3.19] (#15422) Backport c8b04fc from #15402 to 3.19. ## Description Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all `BaseTool`s, when really they can also be a generic function. This PR incorporates that logic. Fixes #15174 ## Testing Changing one of the tools in the tests to a function instead of a `FunctionTool` reproduced this issue in tests (error + missing span event). ## Risks None ## Additional Notes We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents. Signed-off-by: Sam Brenner <sam.brenner@datadoghq.com> Co-authored-by: Sam Brenner <106700075+sabrenner@users.noreply.github.com>
Description
Previously, for the Google ADK LLM Observability integration, we would try and extract tool name and descriptions as if they were all
BaseTools, when really they can also be a generic function. This PR incorporates that logic.Fixes #15174
Testing
Changing one of the tools in the tests to a function instead of a
FunctionToolreproduced this issue in tests (error + missing span event).Risks
None
Additional Notes
We should in the future update the Google ADK integration as I believe we currently only capture the top-level runner span as the agent when we should also capture sub agents.