fix: export thoughts_token_count to OpenTelemetry trace spans#4835
fix: export thoughts_token_count to OpenTelemetry trace spans#4835brucearctor wants to merge 3 commits into
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Hi @brucearctor, Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix the mypy-diff errors. |
|
@rohityan -- will do |
|
@rohityan going to let it run, but I think addressed the diff/new error, and solved a couple others :-) Let me know if other concerns. Cheers - |
|
Hi @brucearctor , can you please resolve branch conflicts. |
Add thoughts_token_count as gen_ai.usage.experimental.reasoning_tokens span attribute in trace_generate_content_result() and trace_inference_result(), matching the existing pattern in trace_call_llm(). Fixes google#4829
- Refactor trace_inference_result() to use otel_span local variable, eliminating all 4 union-attr mypy errors (1 new + 3 pre-existing) - Add tests for trace_generate_content_result() thinking tokens - Import trace_generate_content_result in test_spans.py
bfb9457 to
3a2cef7
Compare
|
@rohityan Rebased onto latest |
|
Does the scope of this issue (and PR #4835) also cover the We have LookML dashboards consuming If BQ plugin is out of scope for current one, happy to open a separate issue. |
|
I went ahead and got conflicts resolved again. @wojcikm : I do not mind doing, but this has also been open for quite awhile, and would be good to close based on my original understanding. Happy to address [ or for someone else to ], if we want to include But, looks like we need @rohityan , @jawoszek as assigned reviewer or other to take a look. Not sure who is in charge of determining scope. |
|
ah ... actually pushing --> |
Resolve conflict in tests/unittests/telemetry/test_spans.py by keeping both our thinking token tests and upstream's new tests for error detection, error_type parameter, and extra generate content attributes.
Description
Fixes #4829
ADK's OpenTelemetry tracing does not export
thoughts_token_countto span attributes. When using Gemini models withThinkingConfig, theusage_metadatainLlmResponsecorrectly containsthoughts_token_count, but this field is never written to spans bytrace_generate_content_result()ortrace_inference_result().Interestingly,
trace_call_llm()already exports this field (asgen_ai.usage.experimental.reasoning_tokens). This PR adds the same export to the two remaining functions that were missing it.Changes
src/google/adk/telemetry/tracing.pythoughts_token_count→gen_ai.usage.experimental.reasoning_tokensspan attribute export intrace_generate_content_result()(~line 746)trace_inference_result()(~line 789)try/except AttributeErrorguard pattern astrace_call_llm()for backward compatibility with older SDK versionstests/unittests/telemetry/test_spans.pytest_trace_inference_result_with_thinking_tokens— verifies the attribute is exported whenthoughts_token_countis non-Nonetest_trace_inference_result_without_thinking_tokens— verifies no attribute is set whenthoughts_token_countis NoneTesting Plan
Unit Tests
All 23 telemetry tests pass:
New tests specifically verify:
thoughts_token_count=50→ span attributegen_ai.usage.experimental.reasoning_tokens=50is setthoughts_token_count=None→ nogen_ai.usage.experimental.reasoning_tokensattribute on spanVerification
Before fix —
Event.usage_metadata.thoughts_token_countis non-zero but Cloud Trace spans only showgen_ai.usage.input_tokensandgen_ai.usage.output_tokens.After fix —
gen_ai.usage.experimental.reasoning_tokensappears alongside the existing token attributes in all three tracing functions.